1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-27 09:12:41 +01:00
Commit graph

12683 commits

Author SHA1 Message Date
Chad Little
0d4902a0e5 Add ability to set a header image per Phame blog
Summary: This is the backend half of uploading an image as a header for Phame Blogs. Allows you to upload image, or delete it. Ref T10901

Test Plan:
Go to Manage Blog, visit Edit Header Image, Upload snarky file. See snarky file on Manage page. Edit Header Image, click delete, save, see file goes away.

{F1690966}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10901

Differential Revision: https://secure.phabricator.com/D16140
2016-06-17 11:23:21 -07:00
epriestley
95d95e3fb4 Try to get to the bottom of the sporradic test failures in testColumnExtendedPolicies()
Summary:
This test has been failing occasionally in a way that does not reproduce, and only when no one is looking at it.

Try to add some extra assertions to maybe get more information.

Test Plan: `arc unit`

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D16137
2016-06-17 07:58:31 -07:00
epriestley
28eb562899 Ignore unrecognized refs in "refs/remotes/"
Summary: Ref T9028. When selecting refs, pretend refs in "refs/remotes/" that we don't otherwise recognize don't exist, since it looks like these are probably remotes //of the remote// we're observing, and who knows what state they're in.

Test Plan: Used `bin/repository discover --verbose` to verify that these named refs no longer appear in the list.

Reviewers: chad, joshuaspence

Reviewed By: joshuaspence

Maniphest Tasks: T9028

Differential Revision: https://secure.phabricator.com/D16136
2016-06-16 16:03:36 -07:00
epriestley
8032a14223 Mark unreachable commits handles as "closed"
Summary:
Ref T9028. Mostly, this gives them a strikethru style.

(I think this is probably the right definition of "closed" for commits. Another definition might be "audited", but I don't think completing audits really "closes" a commit.)

Test Plan: {F1689662}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9028

Differential Revision: https://secure.phabricator.com/D16135
2016-06-16 13:01:09 -07:00
epriestley
7c8f9d7ba2 Don't track "phabricator/" staging area tags
Summary: Ref T9028. Ref T6878. This rule should probably be refined in the long term, but for now just ignore "phabricator/diff/12424" and similar staging area tags.

Test Plan: Ran `bin/repository discover --verbose` on a repository with staging area refs, saw Phabricator ignore those refs as untracked.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6878, T9028

Differential Revision: https://secure.phabricator.com/D16134
2016-06-16 11:22:02 -07:00
epriestley
1c63ac6a3a When a ref is moved or deleted, put it on a list; later, check for reachability
Summary:
Ref T9028. This allows us to detect when commits are unreachable:

  - When a ref (tag, branch, etc) is moved or deleted, store the old thing it pointed at in a list.
  - After discovery, go through the list and check if all the stuff on it is still reachable.
  - If something isn't, try to follow its ancestors back until we find something that is reachable.
  - Then, mark everything we found as unreachable.
  - Finally, rebuild the repository summary table to correct the commit count.

Test Plan:
  - Deleted a ref, ran `pull` + `refs`, saw oldref in database.
  - Ran `discover`, saw it process the oldref, mark the unreachable commit, and update the summary table.
  - Visited commit page, saw it properly marked.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9028

Differential Revision: https://secure.phabricator.com/D16133
2016-06-16 11:21:38 -07:00
epriestley
02d7bb8604 Add "bin/repository mark-reachable" for fixing commit reachability flags
Summary:
Ref T9028. This corrects the reachability of existing commits in a repository.

In particular, it can be used to mark deleted commits as unreachable.

Test Plan:
  - Ran it on a bad repository, with bad args, etc.
  - Ran it on a clean repo, got no changes.
  - Marked a reachable commit as unreachable, ran script, got it marked reachable.
  - Started deleting tags and branches from the local working copy while running the script, saw greater parts of the repository get marked unreachable.
  - Pulled repository again, everything automatically revived.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9028

Differential Revision: https://secure.phabricator.com/D16132
2016-06-16 11:21:17 -07:00
epriestley
77ee518d88 Make daemons ignore "Unreachable" commits and avoid duplicate work
Summary:
Ref T9028. This improves the daemon behavior for unreachable commits. There is still no way for commits to become marked unreachable on their own.

  - When a daemon encounters an unreachable commit, fail permanently.
  - When we revive a commit, queue new daemons to process it (since some of the daemons might have failed permanently the first time around).
  - Before doing a step on a commit, check if the step has already been done and skip it if it has. This can't happen normally, but will soon be possible if a commit is repeatedly deleted and revived very quickly.
  - Steps queued with `bin/repository reparse ...` still execute normally.

Test Plan:
  - Used `bin/repository reparse` to run every step, verified they all mark the commit with the proper flag.
  - Faked the `reparse` exception in the "skip step" code, used `repository reparse` to skip every step.
  - Marked a commit as unreachable, ran `discover`, saw daemons queue for it.
  - Ran daemons with `bin/worker execute --id ...`, saw them all skip + queue the next step.
  - Marked a commit as unreachable, ran `bin/repository reparse` on it, got permanent failures immediately for each step.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9028

Differential Revision: https://secure.phabricator.com/D16131
2016-06-16 11:20:56 -07:00
epriestley
ec89c7d63e Add an "Unreachable" flag for commits and revive them during discovery
Summary:
Ref T9028. This is the easy part of dealing with deleted commits:

  - Add a flag for unreachable commits (nothing sets this flag yet).
  - Ignore unreachable commits when querying for known commits during discovery, so we pretend they do not exist.
  - When recording a commit, try just reviving an existing unreachable commit first. If that works, bail out.

Test Plan:
  - Artificially marked a commit as unreachable with raw SQL.
  - Verified it said "deleted: unreachable" in the UI.
  - Ran `repository discover --trace --verbose`.
  - Saw the discovery process ignore the commit when filling the cache.
  - Saw the discovery process revive the commit instead of trying to record it again.
  - Web UI now shows the commit as normal.
  - Running `repository discover` again doesn't make any further changes.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9028

Differential Revision: https://secure.phabricator.com/D16130
2016-06-16 11:20:37 -07:00
epriestley
2949905c04 Fetch and discover all Git ref types, not just branches
Summary:
Ref T9028. Fixes T6878. Currently, we only fetch and discover branches. This is fine 99% of the time but sometimes commits are pushed to just a tag, e.g.:

```
git checkout <some hash>
nano file.c
git commit -am '...'
git tag wild-wild-west
git push origin wild-wild-west
```

Through a similar process, commits can also be pushed to some arbitrary named ref (we do this for staging areas).

With the current rules, we don't fetch tag refs and won't discover these commits.

Change the rules so:

  - we fetch all refs; and
  - we discover ancestors of all refs.

Autoclose rules for tags and arbitrary refs are just hard-coded for now. We might make these more flexible in the future, or we might do forks instead, or maybe we'll have to do both.

Test Plan:
Pushed a commit to a tag ONLY (`vegetable1`).

<cf508b8de6>

On `master`, prior to the change:

  - Used `update` + `refs` + `discover`.
  - Verified tag was not fetched with `git for-each-ref` in local working copy and the web UI.
  - Verified commit was not discovered using the web UI.

With this patch applied:

  - Used `update`, saw a `refs/*` fetch instead of a `refs/heads/*` fetch.
  - Used `git for-each-ref` to verify that tag fetched.
  - Used `repository refs`.
  - Saw new tag appear in the tags list in the web UI.
  - Saw new refcursor appear in refcursor table.
  - Used `repository discover --verbose` and examine refs for sanity.
  - Saw commit row appear in database.
  - Saw commit skeleton appear in web UI.
  - Ran `bin/phd debug task`.
  - Saw commit fully parse.

{F1689319}

Reviewers: chad

Reviewed By: chad

Subscribers: avivey

Maniphest Tasks: T6878, T9028

Differential Revision: https://secure.phabricator.com/D16129
2016-06-16 11:20:05 -07:00
epriestley
67084a6953 Support AES256 at-rest encryption in Files
Summary:
Ref T11140. This makes encryption actually work:

  - Provide a new configuation option, `keyring`, for specifying encryption keys.
  - One key may be marked as `default`. This activates AES256 encryption for Files.
  - Add `bin/files generate-key`. This is helps when generating valid encryption keys.
  - Add `bin/files encode`. This changes the storage encoding of a file, and helps test encodings and migrate existing data.
  - Add `bin/files cycle`. This re-encodes the block key with a new master key, if your master key leaks or you're just paraonid.
  - Document all these options and behaviors.

Test Plan:
  - Configured a bad `keyring`, hit a bunch of different errors.
  - Used `bin/files generate-key` to try to generate bad keys, got appropriate errors ("raw doesn't support keys", etc).
  - Used `bin/files generate-key` to generate an AES256 key.
  - Put the new AES256 key into the `keyring`, without `default`.
  - Uploaded a new file, verified it still uploaded as raw data (no `default` key yet).
  - Used `bin/files encode` to change a file to ROT13 and back to raw. Verified old data got deleted and new data got stored properly.
  - Used `bin/files encode --key ...` to explicitly convert a file to AES256 with my non-default key.
  - Forced a re-encode of an AES256 file, verified the old data was deleted and a new key and IV were generated.
  - Used `bin/files cycle` to try to cycle raw/rot13 files, got errors.
  - Used `bin/files cycle` to cycle AES256 files. Verified metadata changed but file data did not. Verified file data was still decryptable with metadata.
  - Ran `bin/files cycle --all`.
  - Ran `encode` and `cycle` on chunked files, saw commands fail properly. These commands operate on the underlying data blocks, not the chunk metadata.
  - Set key to `default`, uploaded a file, saw it stored as AES256.
  - Read documentation.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11140

Differential Revision: https://secure.phabricator.com/D16127
2016-06-16 08:08:56 -07:00
epriestley
39afc0f97c Add an AES256 storage format for at-rest encryption
Summary:
Ref T11140. This doesn't do anything yet since there's no way to enable it and no way to store master keys.

Those are slightly tougher problems and I'm not totally satisfied that I have an approach I really like for either problem, so I may wait for a bit before tackling them. Once they're solved, this does the mechanical encrypt/decrypt stuff, though.

This design is substantially similar to the AWS S3 server-side encryption design, and intended as an analog for it. The decisions AWS has made in design generally seem reasonable to me.

Each block of file data is encrypted with a unique key and a unique IV, and then that key and IV are encrypted with the master key (and a distinct, unique IV). This is better than just encrypting with the master key directly because:

  - You can rotate the master key later and only need to re-encrypt a small amount of key data (about 48 bytes per file chunk), instead of re-encrypting all of the actual file data (up to 4MB per file chunk).
  - Instead of putting the master key on every server, you can put it on some dedicated keyserver which accepts encrypted keys, decrypts them, and returns plaintext keys, and can send it 32-byte keys for decryption instead of 4MB blocks of file data.
  - You have to compromise the master key, the database, AND the file store to get the file data. This is probably not much of a barrier realistically, but it does make attacks very slightly harder.

The "KeyRing" thing may change once I figure out how I want users to store master keys, but it was the simplest approach to get the unit tests working.

Test Plan:
  - Ran unit tests.
  - Dumped raw data, saw encrypted blob.
  - No way to actually use this in the real application yet so it can't be tested too extensively.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11140

Differential Revision: https://secure.phabricator.com/D16124
2016-06-16 08:05:57 -07:00
epriestley
f9e3108938 Fix an issue with AuthProviderConfig handles
Summary: Fixes T11156. These were never correct, but also never actually used until I made timelines load object handles unconditionally in D16111.

Test Plan: Viewed an auth provider with transactions, no more fatal.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11156

Differential Revision: https://secure.phabricator.com/D16128
2016-06-16 06:04:43 -07:00
Chad Little
4e14b3f3a5 Use modern layout in Phame Blog management
Summary: Uses PHUITwoColumnView in Blog Manage and Blog Picture. Ref T9897

Test Plan: Use each page.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T9897

Differential Revision: https://secure.phabricator.com/D16126
2016-06-15 16:07:28 -07:00
Chad Little
b94b16df49 Allow Phame Posts to be ordered by datePublished
Summary: Adds some ordering options to PhamePost queries. Works on search, PhameHome, BlogHome

Test Plan: Try searching with Order By set to Date Published in application search, get correct order. Check a blog home page, check PhameHome.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T9897

Differential Revision: https://secure.phabricator.com/D16125
2016-06-15 12:48:56 -07:00
epriestley
1049feb0ed Add support to Files for file storage formats, to support encryption-at-rest
Summary:
Ref T11140. When reading and writing files, we optionally apply a "storage format" to them.

The default format is "raw", which means we just store the raw data.

This change modularizes formats and adds a "rot13" format, which proves formatting works and is testable. In the future, I'll add real encryption formats.

Test Plan:
  - Added unit tests.
  - Viewed files in web UI.
  - Changed a file's format to rot13, saw the data get rotated on display.
  - Set default format to rot13:
    - Uploaded a small file, verified data was stored as rot13.
    - Uploaded a large file, verified metadata was stored as "raw" (just a type, no actual data) and blob data was stored as rot13.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11140

Differential Revision: https://secure.phabricator.com/D16122
2016-06-15 11:17:53 -07:00
epriestley
f9a58fafba Add "video/quicktime" as a default Video MIME type
Summary: Ref T11142. H264 video in a Quicktime container works in Safari and Firefox for me (although not Chrome), so include it in the default video mime types.

Test Plan: Uploaded video file from T11142 locally, saw it render with `<video />` properly in Safari.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11142

Differential Revision: https://secure.phabricator.com/D16121
2016-06-14 14:44:18 -07:00
Chad Little
695f0b09b2 Add supportsSearch to Phame Blog/Post
Summary: Flips the bits from true to false in transaction editor.

Test Plan: update a post, search for new term

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T9897

Differential Revision: https://secure.phabricator.com/D16120
2016-06-14 13:35:49 -07:00
Chad Little
cfa73eb544 Make PhameBlog full text searchable
Summary: Ref T9897, makes blogs searchable

Test Plan: Make a blog, index it, search for it.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T9897

Differential Revision: https://secure.phabricator.com/D16119
2016-06-14 13:13:22 -07:00
epriestley
2e45021250 Fix several issues with email-related global preferences
Summary:
Ref T11098. Mixture of issues here:

  - Similar problem to D16112, where users with no settings at all could fail to fall back to the global defaults.
    - I made `UserPreferencesQuery` responsible for building defaults instead to simplify this, since we have 4 or 5 callsites which need to do it and they aren't easily reducible.
  - Handle cases where `metamta.one-mail-per-recipient` is off (and thus users can not have any custom settings) more explicitly.
  - When `metamta.one-mail-per-recipient` is off, remove the "Email Format" panel for users only -- administrators can still access it in global preferences.

Test Plan:
  - Deleted a user's preferences, changed globals, purged cache, made sure defaults reflected global defaults.
  - Changed global mail tags, sent mail to the user, verified it was dropped in accordinace with global settings.
  - Changed user's settings to get the mail instead, verified mail was sent.
  - Toggled user's Re / Vary settings, verified mail subject lines reflected user settings.
  - Disabled `metamta.one-mail-per-recipient`, verified user "Email Format" panel vanished.
  - Edited "Email Format" in single-mail-mode in global prefs as an administrator.
  - Sent more mail, verified mail respected new global settings.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11098

Differential Revision: https://secure.phabricator.com/D16118
2016-06-14 12:35:31 -07:00
epriestley
cb7560d301 Remove "re prefix" and "vary subjects" config
Summary: Ref T11098. There is no reason to maintain these as separate values now that they can be configured in global settings.

Test Plan:
  - Hit and read setup issue.
  - Fiddled with settings.
  - I'll vet this more throughly in the next diff since I need to fix an issue with global defaults in mail and can explicitly test this at the same time.

Reviewers: chad

Reviewed By: chad

Subscribers: eadler

Maniphest Tasks: T11098

Differential Revision: https://secure.phabricator.com/D16117
2016-06-14 12:35:12 -07:00
Chad Little
bce44c8b02 Add PhamePost to full text search
Summary: Adds PhamePost object to fulltextsearch index. Some issue searching just "Open" though? Also "closed" objects search fine but don't display as disabled.

Test Plan:
bin/search index --type POST

{F1687043}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T9897

Differential Revision: https://secure.phabricator.com/D16116
2016-06-14 12:18:58 -07:00
epriestley
e44d92babc Have modular transactions fall back correctly when selecting feed titles
Summary: Ref T9789. Falling back to `parent::` is better, and fixes older-style feed stories for Pastes, like "added a comment".

Test Plan: Viewed a comment feed story about a paste.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9789

Differential Revision: https://secure.phabricator.com/D16114
2016-06-14 11:07:42 -07:00
Luka Kladaric
fc1c633bae fix Vary Subjects option names
Summary:
The option names for `Vary Subjects` are copypasta from the `Add "Re:" Prefix` option. Fix their names to refer to `Vary Subjects` instead.

Fixes T11148

Test Plan: Verify option names for `Vary Subjects` refer to `Add "Re:" Prefix` before apply. Verify they no longer do after apply.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Maniphest Tasks: T11148

Differential Revision: https://secure.phabricator.com/D16113
2016-06-14 17:13:12 +00:00
epriestley
33ec855449 Modularize application transactions in Paste, mostly
Summary:
Ref T9789. `Transaction` and `Editor` classes are the last major pieces of infrastructure that haven't been fully modularized.

Some of the specific issues are:

  - `Editor` classes rely on a bunch of `instanceof` stuff in the base class to pick up transaction types like "subscribe", "projects", etc. Instead, applications should be adding these, and third-party applications should be able to add them.
  - Code is spread across `Transaction` and `Editor` classes somewhat oddly. For example, generating old/new values would probably make more sense at the `Transaction` level, but it currently exists at the `Editor` level.
  - Both types of classes have a lot of functions based on `switch()` statements, which require a ton of boilerplate and are just generally kind of hard to work with.

This creates classes for each type of transaction, and moves almost all of the logic to them. These classes are simpler and more focused than the old stuff was, and can organize related code better.

This starts inching toward defining `CoreTransactions` for features shared across applications. It only defines the "Create" transaction so far, but at some point I plan to move all the other shared transactions to Core and let them control which objects they're available for.

Test Plan:
  - Created pastes with web UI and API.
  - Edited all paste properites.
  - Archived/activated.
  - Verified files got reasonable names.
  - Reviewed timeline and feed.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9789

Differential Revision: https://secure.phabricator.com/D16111
2016-06-14 06:13:28 -07:00
epriestley
d68b2cc0e4 Fix construction of default settings for users with no settings at all
Summary:
Ref T11098. Users with at least one setting set correctly fall back to the defaults, but users with no settings at all currently do not.

Make them fall back to global defaults properly.

Test Plan:
  - Set global defaults to some non-default setting.
  - Completely delete a user's settings.
  - `bin/cache purge --purge-all` or `--purge-user`.
  - View settings as the user.
    - Before change: showed hard-coded defaults instead of global defaults until you save anything.
    - After change: properly shows global defaults from the start.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11098

Differential Revision: https://secure.phabricator.com/D16112
2016-06-13 15:50:12 -07:00
epriestley
65634781b4 Don't re-mention users for comment edits
Summary:
Ref T11035. This only fixes half of the issue: comment editing has been fixed, but normal transactions which edit things like descriptions haven't yet.

The normal edits aren't fixed because the "oldValues" are populated too late. The code should start working once they get populated sooner, but I don't want to jump the gun on that since it'll probably have some spooky effects. I have some other transaction changes coming down the pipe which should provide a better context for testing "oldValue" population order.

Test Plan:
  - Mentioned `@dog` in a comment.
  - Removed `@dog` as a subscriber.
  - Edited the comment, adding some unrelated text at the end (e.g., fixing a typo).
    - Before change: `@dog` re-added as subscriber.
    - After change: no re-add.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11035

Differential Revision: https://secure.phabricator.com/D16108
2016-06-13 13:57:59 -07:00
Chad Little
74682d46ae Add edit-pencil to ApplicationSearch for PhamePosts
Summary: Adds a quick edit link to PhamePosts in ApplicationSearch

Test Plan: Review a few searches, click on pencil.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16109
2016-06-13 12:05:22 -07:00
Chad Little
63d413e201 Don't truncate blogs or posts in Phame sidebar
Summary: Removes the CSS truncation, so it's easier to track drafts. Ref T9897

Test Plan: Really long draft titles.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T9897

Differential Revision: https://secure.phabricator.com/D16107
2016-06-13 11:21:40 -07:00
Shijie Feng
aaf3698666 Add datasources to allow search revisions by project.
Summary:
When having lots of repos, seeing "all revisions in this project" is hard, and we ended up adding herald rules to basically copy project tags to the revisions on a per-project basis. Adding a "tagged: project" function to the Repositories search field allows users to find differentials within a project.

Fix T10850.

Test Plan: search differentials by tagging project and repository in the Repository field

Reviewers: avivey, epriestley, #blessed_reviewers

Reviewed By: avivey, epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T10850

Differential Revision: https://secure.phabricator.com/D16096
2016-06-13 18:08:44 +00:00
Chad Little
e78488f6eb Clean up some PhamePostEditor archive cases
Summary: Forgot to save this file locally. Adds isArchived to same hidden features as isDraft

Test Plan: test mail on archived posts

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16106
2016-06-13 10:54:30 -07:00
Chad Little
72c57d36a3 Ability to archive Phame Posts
Summary: Ref T9897. Adds ability to Archive a Phame Post (only visible under ApplicationSearch).

Test Plan: Archive a post, re-publish it, search for it, archive it again. View Home, Blog, Live pages.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T9897

Differential Revision: https://secure.phabricator.com/D16104
2016-06-13 10:47:53 -07:00
epriestley
a5e29f3ffa Fix an ancient ad-hoc string truncation
Summary: Fixes T11139. We missed this years ago when we moved to PhutilUTF8StringTruncator.

Test Plan: {F1686072}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11139

Differential Revision: https://secure.phabricator.com/D16105
2016-06-13 10:16:25 -07:00
epriestley
f56f1b05c0 Install a SIGTERM handler in ssh-connect
Summary:
Ref T10547. This has been around for a while but I was never able to reproduce it. I caught a repro case in the cluster recently and I think this is the right fix.

We tell Subversion to run `ssh-connect` instead of `ssh` so we can provide options and credentials, by using `SVN_SSH` in the environment. Subversion will sometimes kill the SSH tunnel subprocess aggressively with SIGTERM -- as of writing, you can search for `SIGTERM` in `make_tunnel()` here:

http://svn.apache.org/repos/asf/subversion/trunk/subversion/libsvn_ra_svn/client.c

By default, when a PHP process gets SIGTERM it just exits immediately, without running destructors or shutdown functions. Since destructors/shutdown functions don't run, `TempFile` doesn't get a chance to remove the file.

I don't have a clear picture of //when// Subversion sends SIGTERM to the child process. I can't really get this to trigger locally via `svn`, although I was able to get it to trigger explicitly. So I'm only about 95% sure this fixes it, but it seems likely.

Test Plan:
Locally, I couldn't get this to reproduce "normally" even knowing the cause (maybe Subversion doesn't do the SIGTERM stuff on OSX?) but I was able to get it to reproduce reliabily by adding `posix_kill(getmypid(), SIGTERM);` to the body of the script.

With that added, running the script with `PHABRICATOR_CREDENTIAL=PHID-CDTL-...` in the environment reliably left straggler temporary files.

Adding `declare()` and a signal handler fixed this: the script now runs the `TempFile` destructor and longer leaves the stragglers around.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10547

Differential Revision: https://secure.phabricator.com/D16102
2016-06-13 10:05:46 -07:00
epriestley
bba53205de Remove all uses of PhutilGitURI in Phabricator
Summary:
Ref T11137. This class is removed in D16099. Depends on D16099.

`PhutilURI` now attempts to "just work" with Git-style URIs, so at least in theory we can just delete all of this code and pretend it does not exist.

(I've left "Display URI" and "Effective URI" as distinct, at least for now, because I think the distinction may be relevant in the future even though it isn't right now, and to keep this diff small, although I may go remove one after I think about this for a bit.)

Test Plan:
  - Created a new Git repository with a Git URI.
  - Pulled/updated it, which now works correctly and should resolve the original issue in T11137.
  - Verified that daemons now align the origin to a Git-style URI with a relative path, which should resolve the original issue in T11004.
  - Grepped for `PhutilGitURI`.
  - Also grepped in `arcanist/`, but found no matches, so no patch for that.
  - Checked display/conduit URIs.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11137

Differential Revision: https://secure.phabricator.com/D16100
2016-06-13 07:20:58 -07:00
epriestley
fb156af480 Render prose diffs in email as summaries
Summary:
Ref T7643. When a large block of prose text is edited (like a wiki page), summarize the diff when sending mail.

For now, I'm still showing the whole thing in the web UI, since it's a bit more manageable there.

Also try to fix newlines in Airmail.

Test Plan:
This web diff:

{F1682591}

..became this mail diff:

{F1682592}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7643

Differential Revision: https://secure.phabricator.com/D16098
2016-06-10 09:39:58 -07:00
epriestley
ad0562e15e Improve some typeahead matching behaviors
Summary:
Ref T8510. Sort prefix matches above non-prefix matches, so that "Ape Discovery" does not match "discovery" better than "Discovery".

Sort functions last.

Rename function internal strings so they don't get over-promoted the prefix-match rules.

Add kind of a hack to get "Project X" sorting above all the "Project X (Milestone 1)" results.

Test Plan:
Created "Ape Discovery", "Baboon Discovery", "Chimpanzee Discovery", etc.

Main project now sorts above milestones:

{F1681773}

Prefix matches now sort above other matches:

{F1681774}

Function results (rarely used) are now less prominent:

{F1681775}

Better function results here:

{F1681776}

More function results:

{F1681777}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8510

Differential Revision: https://secure.phabricator.com/D16094
2016-06-09 13:22:34 -07:00
epriestley
c694bd651b Make base64 the default SMTP encoding
Summary: Ref T11120. If this works, I'll just remove this option completely.

Test Plan: ¯\_(ツ)_/¯

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11120

Differential Revision: https://secure.phabricator.com/D16095
2016-06-09 13:22:17 -07:00
epriestley
e6e4ddd03a Change simple template dashboard to "Open Tasks" instead of "All Tasks"
Summary: "All Tasks" is bad in the long run and not clearly better for new installs.

Test Plan: Created a new smiple template, saw open tasks only.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D16093
2016-06-09 12:27:50 -07:00
epriestley
55a698a28a Use HTTPEngineExtension proxy for git HTTP operations
Summary: Ref T10227. When we perform `git` http operations (fetch, mirror) check if we should use a proxy; if we should, set `http_proxy` or `https_proxy` in the environment to make `git` have `curl` use it.

Test Plan:
  - Configured a proxy extension to run stuff through a local instance of Charles.
  - Ran `repository pull` and `repository mirror`.
  - Saw `git` HTTP requests route through the proxy.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10227

Differential Revision: https://secure.phabricator.com/D16092
2016-06-09 12:17:10 -07:00
epriestley
a88329fc38 Implement basic differential.revision.search
Summary: Ref T11123. This implements a very basic skeleton for modern revision search.

Test Plan: Viewed and executed Conduit API method.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11123

Differential Revision: https://secure.phabricator.com/D16089
2016-06-09 09:06:58 -07:00
epriestley
4d12c58dcf Allow Pholio mocks to be created and edited without drag-and-drop
Summary: Ref T6523. Allows you to click stuff instead of using drag-and-drop.

Test Plan: On iOS simulator, created and updated a mock.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6523

Differential Revision: https://secure.phabricator.com/D16088
2016-06-09 08:43:38 -07:00
epriestley
3b1c6073a5 Don't include other users' unpublished drafts when counting Pholio inlines
Summary: Fixes T10886. This should get more formal some day, but just fix it for now.

Test Plan: Reloaded mock with other unpublished draft inlines, saw accurate count.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10886

Differential Revision: https://secure.phabricator.com/D16087
2016-06-09 08:43:12 -07:00
Chad Little
e808963eae Add Ponder Question mail create receiver
Summary: Fixes T11115, but unclear how to test this. I think I've asked this in the past.

Test Plan:
 - Visit Applications -> Ponder
 - Configure external email
 - Test External Email
 - See new Question

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11115

Differential Revision: https://secure.phabricator.com/D16084
2016-06-08 14:48:20 -07:00
epriestley
72d554aa9b Fix parsing of anchors in Phriction document link syntax
Summary: Ref T4280. At some point (probably D15732) we started getting anchor parsing wrong. Just pop the anchor off before doing all the logic, then put it back on at the end.

Test Plan:
Tested various forms like:

```
[[ x ]]
[[ x | z ]]
[[ x#y | z ]]
[[ ./x#y | z ]]
```

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4280

Differential Revision: https://secure.phabricator.com/D16083
2016-06-08 13:14:33 -07:00
Chad Little
78fab485b4 Break-word on all propertly list values
Summary: Fixes T10827. Adds break-word on all values.

Test Plan: Really long text.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10827

Differential Revision: https://secure.phabricator.com/D16081
2016-06-08 10:09:41 -07:00
Chad Little
3453507230 Add print styles for Phortune carts
Summary: Fixes T10899, adds styles specifically for printing Phortune carts.

Test Plan: Print a Phortune cart

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10899

Differential Revision: https://secure.phabricator.com/D16079
2016-06-08 07:52:18 -07:00
epriestley
a653d4d9b3 Give ConpherenceThreadQuery a primary table alias
Summary: Fixes T11113. On the 2nd+ page, we could end up with an ambiguous `id` WHERE clause because we don't define a primary table alias on this query. Define one.

Test Plan:
Changed SearchEngine to return pages of size 5, searched for my threads, toggled to second page, no exception.

Used DarkConsole to examine that second-page query, saw that it had `thread.id` explicitly instead of `id` implicitly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11113

Differential Revision: https://secure.phabricator.com/D16080
2016-06-08 07:39:45 -07:00
Chad Little
c71f92a1eb Fix new Countdown link
Summary: Correct the url for edit engine. Fixes T11046

Test Plan: new install -> clicky green button

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T11046

Differential Revision: https://secure.phabricator.com/D16078
2016-06-07 20:13:08 -07:00
epriestley
ffb50ef45d Require login for "Must Verify Email" controller
Summary:
Via HackerOne. This page fatals if accessed directly while logged out.

The "shouldRequireLogin()" check is wrong; this is a logged-in page.

Test Plan:
Viewed the page while logged out, no more fatal.

Faked my way through the actual verification flow.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D16077
2016-06-07 16:37:12 -07:00