1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-02-28 06:29:27 +01:00
Commit graph

2164 commits

Author SHA1 Message Date
epriestley
6f09edeb91 Fix an issue where the "%%%" parser could match too many lines in unterminated blocks
Summary: Fixes T13530. The block parser could match too many lines in an unterminated "%%%" literal block. Adjust the logic to stop doing this (and hopefully be a little easier to read).

Test Plan: Added a failing test, made it pass.

Maniphest Tasks: T13530

Differential Revision: https://secure.phabricator.com/D21208
2020-05-03 09:25:41 -07:00
epriestley
186a12ef7f Replace nonexistent "withPHIDs()" in ChangesetQuery with "withIDs()"
Summary:
Ref T13519. See <https://discourse.phabricator-community.org/t/error-call-to-undefined-method-differentialchangesetquery-withphids/3816/>.

Changesets do not have PHIDs, and the Query has no "withPHIDs()" method. The keys in the viewstate storage are (usually) IDs.

Test Plan:
  - On a revision with Diff 1 and Diff 2 affecting the same file:
    - Viewed Diff 1.
    - Hid file A.
    - Viewed Diff 2.
  - Before patch: exception about call to "withPHIDs()", which does not exist for ChangesetQuery.
  - After patch: no exception. Also, file actually unhid, which is the correct behavior!

Maniphest Tasks: T13519

Differential Revision: https://secure.phabricator.com/D21189
2020-04-29 14:47:47 -07:00
epriestley
f21f1d8ab9 Update the diff table of contents to use hierarchical views and edit distance renames
Summary:
Ref T13520. Generally, make the table of contents look and more like the paths panel:

  - Show a hierarchy, with compression for single-sibling children.
  - Use the same icons, instead of "M D" and "(img)" stuff.
  - Use EditDistanceMatrix to do a piece-by-piece diff of paths changes.
  - Show path changes within the path list.

I'm not entirely sold on this, but it was complicated to write and I've never heard the term "sunk cost fallacy". I think this is mostly a net improvement, but may need some adjustments and followup.

Test Plan: Viewed various changes in Differential and Diffusion, saw a more usable table of contents.

Maniphest Tasks: T13520

Differential Revision: https://secure.phabricator.com/D21183
2020-04-28 12:27:37 -07:00
epriestley
5eaa0f24e7 Use "@" to silence "GC list" warnings from "apc_store()" and "apcu_store()"
Summary:
Fixes T13525. Since D21044, the intermittent GC list warnings are treated more severely and can become user-visible errors.

Silence them, since this seems to be the only realistic response in most versions of APC/APCu.

Test Plan: Will deploy.

Maniphest Tasks: T13525

Differential Revision: https://secure.phabricator.com/D21179
2020-04-28 04:13:37 -07:00
epriestley
60de1506fe Make "hidden" changesets sticky, and show hidden state in the filetree
Summary:
Ref T13455. Make "hidden" a changeset property similar to other changeset properties.

We don't need to render this on the server, so we make a request (to update the setting) and just discard the response.

Test Plan: {F7375468}

Maniphest Tasks: T13455

Differential Revision: https://secure.phabricator.com/D21158
2020-04-22 16:12:42 -07:00
epriestley
ef69c7969f Restore editor behavior to Diffusion and support "\" shortcut
Summary:
Ref T13515. This restores the "Open in Editor" behavior to Diffusion, and makes "\" work there.

The URI pattern is now sent as a structured template to the client, so the code will work properly if a file path contains "%l".

Test Plan:
  - Clicked "Open in Editor" and pressed "\" in Diffusion when viewing a file.
  - Clicked a line, hit "\", got the file opened to that line.

Maniphest Tasks: T13515

Differential Revision: https://secure.phabricator.com/D21149
2020-04-19 09:41:37 -07:00
epriestley
f02024615a Add "short name", "id", and "phid" variables for external editor URIs
Summary: Ref T13515. External editor URIs currently depend on repositories having callsigns, but callsigns are no longer required. Add some variables to support configuring this feature for repositories that do not have callsigns.

Test Plan: Changed settings to use new variables, saw links generate appropriately.

Maniphest Tasks: T13515

Differential Revision: https://secure.phabricator.com/D21147
2020-04-19 09:37:53 -07:00
epriestley
c79094d7a8 Add static errors, supported protocols, and a dynamic function listing to external editor settings page
Summary:
Ref T13515.

  - Previously valid editor URIs may become invalid without being changed (if an administrator removes a protocol from the list, for example), but this isn't explained very well. Show an error on the settings page if the current value isn't usable.
  - Generate a list of functions from an authority in the parser.
  - Generate a list of protocols from configuration.

Test Plan: {F7370872}

Maniphest Tasks: T13515

Differential Revision: https://secure.phabricator.com/D21146
2020-04-19 09:37:31 -07:00
epriestley
3984c14260 Tokenize external editor links so they can be safely materialized on the client
Summary:
Ref T13515. Currently, opening a file to a particular line in an external editor relies on replacing "%l" with "%l" (which is escaped as "%25l") on the server, and then replacing "%25l" with the line number on the client. This will fail if the file path (or any other variable) contains "%l" in its unencoded form.

The parser also can't identify invalid variables.

Pull the parser out, formalize it, and make it generate an intermediate representation which can be sent to the client and reconstituted.

(This temporarily breaks Diffusion and permanently removes the weird, ancient integration in Dark Console.)

Test Plan:
  - Added a bunch of tests for the actual parser.
  - Used "Open in Editor" in Differential.

Maniphest Tasks: T13515

Differential Revision: https://secure.phabricator.com/D21143
2020-04-19 09:02:49 -07:00
epriestley
c3c55d82ae Make "renderer", "engine", and "encoding" sticky across reloads in Differential and Diffusion
Summary:
Ref T13455. Update the other "view state" properties to work like "highlight" now works.

Some complexity here arises from these concerns:

  - In "View Standalone", we render the changeset inline. This is useful for debugging/development, and desirable to retain.
  - In all other cases, we render the changeset with AJAX.

So the client needs to be able to learn about the "state" properties of the changeset on two different flows. Prior to this change, each pathway had a fair amount of unique code.

Then, some bookkeeping issues:

  - At inital rendering time, we may not know which renderer will be selected: it may be based on the client viewport dimensions.
  - Prior to this change, the client didn't separate "value of the property for the changeset as rendered" and "desired value of the property".

Test Plan:
  - Viewed changes in Differential, Diffusion, and in standalone mode.
  - Toggled renderer, character sets, and document engine (this one isn't terribly useful). Reloaded, saw them stick.
  - Started typing a comment, cancelled it, hit the undo UI.

Maniphest Tasks: T13455

Differential Revision: https://secure.phabricator.com/D21138
2020-04-19 08:59:09 -07:00
epriestley
8aac55cc57 Make "Highlight As..." sticky across reloads in Diffusion and Differential
Summary:
Ref T13455. Add container-level storage for persistent view state, and persist "Highlight As..." inside it.

The storage generates a "PhabricatorChangesetViewState" configuration object as an output.

When preferences are expressed on a diff and that diff is later attached to a revision, we attempt to copy the preferences.

The internal storage tracks per-changeset settings, but currently always uses "last update wins" to apply the settings in the UI.

Test Plan:
  - Viewed revisions, changed highlighting, reloaded. Saw highlighting stick in revision view and standalone view.
  - Viewed commits, changed highlighting, reloaded. Saw highlighting stick.
  - Created a diff, changed highlighting, turned it into a revision, saw highlighting persist.

Subscribers: jmeador, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13455

Differential Revision: https://secure.phabricator.com/D21137
2020-04-19 08:58:39 -07:00
epriestley
2748f83e12 Modularize Ferret fulltext functions
Summary: Ref T13511. Currently, Ferret fulltext field functions (like "title:") are hard-coded. Modularize them so extensions may define new ones.

Test Plan: Added a new custom field which emits data for the indexer, searched for "animal-noises:moo", "animal-noises:-", etc., in global search and application search.

Maniphest Tasks: T13511

Differential Revision: https://secure.phabricator.com/D21131
2020-04-16 13:41:13 -07:00
epriestley
9bdf477f2f Combine the two different ngram-splitting algorithms into a single engine
Summary:
Ref T13501. Depends on D21127. With the "prefix" behavior removed in D21127, we now have two virtually identical copies of the same code.

The newer one in Ferret is better: it slices utf8 correctly and is slightly more efficient on large inputs. Pull it out and make all callers call into it.

Test Plan:
  - Grepped for all affected symbols.
  - Ran `bin/search index --force ...` to reindex various objects (tasks, files).
  - Searched for things in the UI.

Maniphest Tasks: T13501

Differential Revision: https://secure.phabricator.com/D21128
2020-04-16 09:45:00 -07:00
epriestley
fb3f423279 Remove broken and unfixable "prefix" ngram behavior
Summary:
Ref T13501. The older ngram code has some "prefix" behavior that tries to handle cases where a user issues a very short (one or two character) query.

This code doesn't work, presumably never worked, and can not be made to work (or, at least, I don't see a way, and am fairly sure one does not exist).

If the user searches for "xy", we can find trigrams in the form "xy*" using the index, but not in the form "*xy". The code makes a misguided effort to look for " xy", but this will only find "xy" in words that begin with "xy", like "xylophone".

For example, searching Files for "om" does not currently find "random.txt".

Remove this behavior. Without engaging the trigram index, these queries fall back to an unidexed "LIKE" table scan, but that's about the best we can do.

Test Plan: Searched for "om", hit "random.txt".

Maniphest Tasks: T13501

Differential Revision: https://secure.phabricator.com/D21127
2020-04-16 09:44:37 -07:00
epriestley
0511b2a012 Implement the "present" and "absent" operators in the Ferret execution engine
Summary:
Ref T13509. Now that the compiler can parse these queries, actually implement them.

These are fairly easy to implement:

  - For present, just "JOIN". If it works, the field is present.
  - For absent, we "LEFT JOIN" and then "WHERE any_column IS NULL".

Test Plan: Searched for various documents with and without fields present, got sensible results in Maniphest. For example, "body:-" finds tasks with no body, "body:- duck" finds tasks with no body and "duck" elsewhere in the content, and so on.

Maniphest Tasks: T13509

Differential Revision: https://secure.phabricator.com/D21110
2020-04-14 10:55:30 -07:00
epriestley
8fa8d0e648 Make Ferret query functions sticky only if their values are not quoted
Summary:
Ref T13509. Currently, functions are "sticky", but this stickness is in the query execution layer.

Instead:

  - move stickiness to the query compiler; and
  - make it so that functions are not sticky if their arguments are quoted.

For example:

  - `title:x y` previously meant `title:x title:y` (and still does). The "title:" is sticky.
  - `title:"x" y` previously meant `title:x title:y`. It now means `title:x all:y`. The "title:" is not sticky because the argument is quoted.

Test Plan: Added unit tests, ran unit tests.

Maniphest Tasks: T13509

Differential Revision: https://secure.phabricator.com/D21108
2020-04-14 10:47:51 -07:00
epriestley
1a59cae743 Update some Phabricator behaviors for changes to Futures
Summary:
Depends on D21053. Ref T11968. Three things have changed:

  - Overseers can no longer use FutureIterator to continue execution of an arbitrary list of futures from any state. Use FuturePool instead.
  - Same with repository daemons.
  - Probably (?) fix an API change in the Harbormaster exec future.

Test Plan:
  - Ran "bin/phd debug task" and "bin/phd debug pull", no longer saw Future-management related errors.
  - The Harbormaster future is easiest to test by just seeing if production works once this change is deployed there.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21054
2020-04-03 12:28:16 -07:00
epriestley
0872051bfa Make AuthProvider, ExternalAccount, and ExternalAccountIdentifier all Destructible
Summary: Depends on D21014. Ref T13493. Make these objects all use destructible interfaces and destroy sub-objects appropriately.

Test Plan:
  - Used `bin/remove destroy --trace ...` to destroy a provider, a user, and an external account.
  - Observed destruction of sub-objects, including external account identifiers.

Maniphest Tasks: T13493

Differential Revision: https://secure.phabricator.com/D21015
2020-02-22 17:46:29 -08:00
epriestley
35a18146a2 Merge a small amount of remaining "libphutil/" code with Phabricator, break libphutil dependency
Summary: Ref T13395. Moves a small amount of remaining "libphutil/" code into "phabricator/" and stops us from loading "libphutil/".

Test Plan: Browsed around; there are likely remaining issues.

Maniphest Tasks: T13395

Differential Revision: https://secure.phabricator.com/D20981
2020-02-12 15:17:36 -08:00
epriestley
f9b3e3360b Continue moving classes with no callers in libphutil or Arcanist to Phabricator
Summary: Ref T13395. Move cache classes, syntax highlighters, other markup classes, and sprite sheets to Phabricator.

Test Plan: Attempted to find any callers for any of this stuff in libphutil or Arcanist and couldn't.

Maniphest Tasks: T13395

Differential Revision: https://secure.phabricator.com/D20977
2020-02-12 13:14:04 -08:00
epriestley
2327578adc Respect linebreaks in full HTML tables in Remarkup
Summary:
Fixes T5427. See PHI1630. See also T13160 and D20568.

In the full HTML table syntax with "<table>", respect linebreaks as literals inside "<td>" cells.

Test Plan: Previewed some full-HTML tables with and without linebreaks, saw what seemed like sensible rendering behavior.

Maniphest Tasks: T5427

Differential Revision: https://secure.phabricator.com/D20971
2020-02-06 15:01:16 -08:00
epriestley
fdbe9ba149 Improve Remarkup parsing performance for certain large input blocks
Summary: Fixes T13487. In PHI1628, an install has a 4MB remarkup corpus which takes a long time to render. This is broadly expected, but a few reasonable improvements fell out of running it through the profiler.

Test Plan:
  - Saw local cold-cache end-to-end rendering time drop from 12s to 4s for the highly secret input corpus.
  - Verified output has the same hashes before/after.
  - Ran all remarkup unit tests.

Maniphest Tasks: T13487

Differential Revision: https://secure.phabricator.com/D20968
2020-02-04 15:07:00 -08:00
epriestley
0e82bd024a Use the new "CurtainObjectRefList" UI element for subscribers
Summary:
Depends on D20966. Ref T13486. Curtains currently render subscribers in a plain text list, but the new ref list element is a good fit for this.

Also, improve the sorting and ordering behavior.

This makes the subscriber list take up a bit more space, but it should make it a lot easier to read at a glance.

Test Plan: Viewed object subscriber lists at varying limits and subscriber counts, saw sensible subscriber lists.

Maniphest Tasks: T13486

Differential Revision: https://secure.phabricator.com/D20967
2020-02-04 12:38:41 -08:00
epriestley
84fd5cd5bb Fix an issue where intracontent empty lines were incorrectly trimmed in quoted blocks
Summary: Fixes T13335. When processing quoted blocks, we remove leading empty lines. This logic incorrectly continued after encountering a nonempty line.

Test Plan: Added a test, made it pass. Previewed blocks in web UI.

Maniphest Tasks: T13335

Differential Revision: https://secure.phabricator.com/D20965
2020-02-04 08:09:50 -08:00
epriestley
c42c5983aa Fix an issue where loading a mangled project graph could fail too abruptly
Summary:
Ref T13484. If you load a subproject S which has a mangled/invalid `parentPath`, the query currently tries to execute an empty edge query and fatals.

Instead, we want to deny-by-default in the policy layer but not fail the query. The subproject should become restricted but not fatal anything related to it.

See T13484 for a future refinement where we could identify "broken / data integrity issue" objects explicilty.

Test Plan:
  - Modified the `projectPath` of some subproject in the database to `QQQQ...`.
  - Loaded that project page.
  - Before patch: fatal after issuing bad edge query.
  - After patch: "functionally correct" policy layer failure, although an explicit "data integrity issue" failure would be better.

Maniphest Tasks: T13484

Differential Revision: https://secure.phabricator.com/D20963
2020-02-03 08:54:04 -08:00
epriestley
54bcbdaba9 Fix an XSS issue with certain high-priority remarkup rules embedded inside lower-priority link rules
Summary:
See <https://hackerone.com/reports/758002>. The link rules don't test that their parameters are flat text before using them in unsafe contexts.

Since almost all rules are lower-priority than these link rules, this behavior isn't obvious. However, two rules have broadly higher priority (monospaced text, and one variation of link rules has higher priority than the other), and the latter can be used to perform an XSS attack with input in the general form `()[ [[ ... | ... ]] ]` so that the inner link rule is evaluated first, then the outer link rule uses non-flat text in an unsafe way.

Test Plan:
Tested examples in HackerOne report. A simple example of broken (but not unsafe) behavior is:

```
[[ `x` | `y` ]]
```

Differential Revision: https://secure.phabricator.com/D20937
2019-12-13 10:37:50 -08:00
epriestley
63d84e0b44 Improve use of keys when iterating over commits in "bin/audit delete" and "bin/repository rebuild-identities"
Summary:
Fixes T13457. Ref T13444. When we iterate over commits in a particular repository, the default iteration strategy can't effectively use the keys on the table.

Tweak the ordering so the "<repositoryID, epoch, [id]>" key can be used.

Test Plan:
  - Ran `bin/audit delete --repository X` and `bin/repository rebuild-identities --repository X` before and after changes.
    - With just the key changes, performance was slightly better. My local data isn't large enough to really emphasize the key changes.
    - With the page size changes, performance was a bit better (~30%, but on 1-3 second run durations).
  - Used `--trace` and ran `EXPLAIN ...` on the new queries, saw them select the "<repositoryID, epoch, [id]>" key and report a bare "Using index condition" in the "Extra" column.

Maniphest Tasks: T13457, T13444

Differential Revision: https://secure.phabricator.com/D20921
2019-11-19 10:18:55 -08:00
epriestley
18da346972 Add additional flags to "bin/repository rebuild-identities" to improve flexibility
Summary:
Ref T13444. Repository identities have, at a minimum, some bugs where they do not update relationships properly after many types of email address changes.

It is currently very difficult to fix this once the damage is done since there's no good way to inspect or rebuild them.

Take some steps toward improving observability and providing repair tools: allow `bin/repository rebuild-identities` to effect more repairs and operate on identities more surgically.

Test Plan: Ran `bin/repository rebuild-identities` with all new flags, saw what looked like reasonable rebuilds occur.

Maniphest Tasks: T13444

Differential Revision: https://secure.phabricator.com/D20911
2019-11-19 09:39:48 -08:00
epriestley
6afbb6102d Remove "PhabricatorEventType::TYPE_DIFFUSION_LOOKUPUSER" event
Summary: Ref T13444. This is an ancient event and part of the old event system. It is not likely to be in use anymore, and repository identities should generally replace it nowadays anyway.

Test Plan: Grepped for constant and related methods, no longer found any hits.

Maniphest Tasks: T13444

Differential Revision: https://secure.phabricator.com/D20909
2019-11-19 09:38:03 -08:00
epriestley
76d9912932 Force unified abstract block diffs into roughly usable shape
Summary: Depends on D20845. Ref T13425. In unified mode, render blocks diffs less-unreasonably.

Test Plan: {F6910436}

Maniphest Tasks: T13425

Differential Revision: https://secure.phabricator.com/D20848
2019-09-30 10:44:07 -07:00
epriestley
f004b76465 Add a "Subtype" tag to the task graph view in Maniphest
Summary: See PHI1466. When an install defines task subtypes, show them on the task graph.

Test Plan:
  - On desktop with subtypes defined, column is visible.
  - On desktop with subtypes not defined, column is hidden.
  - On mobile, column is hidden.

{F6896845}

Differential Revision: https://secure.phabricator.com/D20842
2019-09-27 10:58:55 -07:00
epriestley
884cd74cc4 In prose diffs, use hash-and-diff for coarse "level 0" diffing to scale better
Summary: Depends on D20838. Fixes T13414. Instead of doing coarse diffing with "PhutilEditDistanceMatrix", use hash-and-diff with "DocumentEngine".

Test Plan:
  - On a large document (~3K top level blocks), saw a more sensible diff, instead of the whole thing falling back to "everything changed" mode.
  - On a small document, still saw a sensible granular diff.

{F6888249}

Maniphest Tasks: T13414

Differential Revision: https://secure.phabricator.com/D20839
2019-09-25 16:50:49 -07:00
epriestley
9d884f144f Add "PhutilProseDiff" classes to "phabricator/"
Summary: Depends on D20836. Ref T13414. Ref T13425. Ref T13395. Move these to "phabricator/" before trying to improve the high-level diff engine in prose diffs.

Test Plan: Ran "arc liberate", looked at a prose diff (no behavioral change).

Maniphest Tasks: T13425, T13414, T13395

Differential Revision: https://secure.phabricator.com/D20838
2019-09-25 16:49:54 -07:00
epriestley
b1d4d5c00c Add an "{anchor #xyz}" rule to Remarkup
Summary: Ref T13410. Fixes T4280. Allows you to put a named anchor into a document explicitly.

Test Plan: Used `{anchor ...}` in Remarkup, used location bar to jump to anchors.

Maniphest Tasks: T13410, T4280

Differential Revision: https://secure.phabricator.com/D20825
2019-09-24 11:04:19 -07:00
epriestley
bff72ce3b5 Generate more friendly anchor names for header sections in Remarkup
Summary:
Depends on D20820. Ref T13410. We currently cut anchor names in the middle, don't support emoji in anchors, and generate relatively short anchors.

Generate slightly longer anchors, allow more unicode, and try not to cut things in the middle.

Test Plan: Created a document with a variety of different anchors and saw them generate more usable names.

Maniphest Tasks: T13410

Differential Revision: https://secure.phabricator.com/D20821
2019-09-24 11:03:03 -07:00
epriestley
adc2002d28 Make it easier to parse "X-Forwarded-For" with one or more load balancers
Summary:
Fixes T13392. If you have 17 load balancers in sequence, Phabricator will receive requests with at least 17 "X-Forwarded-For" components in the header.

We want to select the 17th-from-last element, since prior elements are not trustworthy.

This currently isn't very easy/obvious, and you have to add a kind of sketchy piece of custom code to `preamble.php` to do any "X-Forwarded-For" parsing. Make handling this correctly easier.

Test Plan:
  - Ran unit tests.
  - Configured my local `preamble.php` to call `preamble_trust_x_forwarded_for_header(4)`, then made `/debug/` dump the header and the final value of `REMOTE_ADDR`.

```
$ curl http://local.phacility.com/debug/
<pre>

HTTP_X_FORWARDED_FOR =
   FINAL REMOTE_ADDR = 127.0.0.1
</pre>
```

```
$ curl -H 'X-Forwarded-For: 1.1.1.1, 2.2.2.2, 3.3.3.3, 4.4.4.4, 5.5.5.5, 6.6.6.6' http://local.phacility.com/debug/
<pre>

HTTP_X_FORWARDED_FOR = 1.1.1.1, 2.2.2.2, 3.3.3.3, 4.4.4.4, 5.5.5.5, 6.6.6.6
   FINAL REMOTE_ADDR = 3.3.3.3
</pre>
```

```
$ curl -H 'X-Forwarded-For: 5.5.5.5, 6.6.6.6' http://local.phacility.com/debug/
<pre>

HTTP_X_FORWARDED_FOR = 5.5.5.5, 6.6.6.6
   FINAL REMOTE_ADDR = 5.5.5.5
</pre>
```

Maniphest Tasks: T13392

Differential Revision: https://secure.phabricator.com/D20785
2019-09-05 04:30:13 -07:00
epriestley
764db4869c Make "bin/storage destroy" target individual hosts in database cluster mode
Summary:
Ref T13336. Currently, "bin/storage destroy" destroys every master. This is wonderfully destructive, but if replication fails it's useful to be able to destroy only a replica.

Operate on a single host, and require "--host" to target the operation in cluster mode, so `bin/storage destroy --host dbreplica001` is a useful operation.

Test Plan: Ran `bin/storage destroy` with various flags locally. Will destroy `secure002` and refresh replication.

Maniphest Tasks: T13336

Differential Revision: https://secure.phabricator.com/D20784
2019-09-04 10:11:08 -07:00
epriestley
e0d6994adb Use the "@" operator to silence connection retry messages if initializing the stack with database config optional
Summary:
Depends on D20780. Ref T13403. During initial setup, it's routine to run "bin/config" with a bad database config. We start the stack in "config optional" mode to anticipate this.

However, even in this mode, we may emit warnings if the connection fails in certain ways. These warnings aren't useful; suppress them with "@".

(Possibly this message should move from "phlog()" to "--trace" at some point, but it has a certain amount of context/history around it.)

Test Plan:
  - Configured MySQL to fail with a retryable error, e.g. good host but bad port.
  - Ran `bin/config set ...`.
  - Before: saw retry warnings on stderr.
  - After: no retry warnings on stderr.
  - (Turned off suppression code artificially and verified warnings still appear under normal startup.)

Maniphest Tasks: T13403

Differential Revision: https://secure.phabricator.com/D20781
2019-09-03 12:54:17 -07:00
epriestley
f8eec38c94 When "mysqli->real_connect()" fails without setting an error code, recover more gracefully
Summary: Depends on D20779. Ref T13403. Bad parameters may cause this call to fail without setting an error code; if it does, catch the issue and go down the normal connection error pathway.

Test Plan:
  - With "mysql.port" set to "quack", ran `bin/storage probe`.
  - Before: wild mess of warnings as the code continued below and failed when trying to interact with the connection.
  - After: clean connection failure with a useful error message.

Maniphest Tasks: T13403

Differential Revision: https://secure.phabricator.com/D20780
2019-09-03 12:51:20 -07:00
epriestley
9316cbf7fd Move web application classes into "phabricator/"
Summary: Ref T13395. Companion change to D20773.

Test Plan: See D20773.

Maniphest Tasks: T13395

Differential Revision: https://secure.phabricator.com/D20774
2019-09-02 07:58:59 -07:00
epriestley
c6642213d5 Straighten out replication/cache behavior in "bin/storage dump"
Summary:
Fixes T13336.

  - Prevent `--no-indexes` from being combined with `--for-replica`, since combining these options can only lead to heartbreak.
  - In `--for-replica` mode, dump caches too. See discussion in T13336. It is probably "safe" to not dump these today, but fragile and not correct.
  - Mark the "MarkupCache" table as having "Cache" persistence, not "Data" persistence (no need to back it up, since it can be fully regenerated from other datasources).

Test Plan: Ran `bin/storage dump` with various combinations of flags.

Maniphest Tasks: T13336

Differential Revision: https://secure.phabricator.com/D20743
2019-08-28 08:25:40 -07:00
epriestley
7198bd7db7 When "utf8mb4" is available, use it as the default client charset when invoking standalone "mysql" commands
Summary:
Fixes T13390. We have some old code which doesn't dynamically select between "utf8mb4" and "utf8". This can lead to dumping utf8mb4 data over a utf8 connection in `bin/storage dump`, which possibly corrupts some emoji/whales.

Instead, prefer "utf8mb4" if it's available.

Test Plan: Ran `bin/storage dump` and `bin/storage shell`, saw sub-commands select utf8mb4 as the client charset.

Maniphest Tasks: T13390

Differential Revision: https://secure.phabricator.com/D20742
2019-08-27 16:36:48 -07:00
epriestley
f1b054a20f Correct the interaction between overheating and offset-based paging
Summary:
Ref T13386. If you issue `differential.query` with a large offset (like 3000), it can overheat regardless of policy filtering and fail with a nonsensical error message.

This is because the overheating limit is based only on the query limit, not on the offset.

For example, querying for "limit = 100" will never examine more than 1,100 rows, so a query with "limit = 100, offset = 3000" will always fail (provided there are at least that many revisions).

Not all numbers work like you might expect them to becuase there's also a 1024-row fetch window, but basically small limits plus big offsets always fail.

Test Plan: Artificially reduced the internal window size from 1024 to 5, then ran `differential.query` with `offset=50` and `limit=3`. Before: overheated with weird error message. After: clean result.

Maniphest Tasks: T13386

Differential Revision: https://secure.phabricator.com/D20728
2019-08-21 20:27:35 -07:00
epriestley
c439931373 Respect "disabled" custom field status granted by "subtype" configuration in form validation
Summary:
Fixes T13384. Currently, the subtype "disabled" configuration is not respected when selecting fields for `ROLE_EDIT`.

The only meaningful caller for `ROLE_EDIT` is transaction validation, but transaction validation should respect fields being disabled by subtype configuration.

Test Plan:
  - Added a "required" Maniphest custom field "F", then "disabled" it in a subtype "S".
  - Created a task of subtype "S".
    - Before: Form submission fails with error "F is required", even though the field is not actually visible on the form and can not be set.
    - After: Form submits cleanly and creates the task.

Maniphest Tasks: T13384

Differential Revision: https://secure.phabricator.com/D20726
2019-08-21 18:03:20 -07:00
epriestley
006cb659cb Make the success message from "bin/config" more clear
Summary:
Ref T13373. When you "bin/config set x ..." a value, the success message ("Set x ...") is somewhat ambiguous and can be interpreted as "First, you need to set x..." rather than "Success, wrote x...".

Make the messaging more explicit. Also make this string more translatable.

Test Plan: Ran `bin/config set ...` with various combinations of flags, saw more clear messaging.

Maniphest Tasks: T13373

Differential Revision: https://secure.phabricator.com/D20711
2019-08-12 12:50:03 -07:00
epriestley
8e263a2f64 Support "date" custom fields in "*.edit" endpoints
Summary: Fixes T13355. This didn't appear to be a ton of extra work, we just didn't get it for free in the original implementation in D14635.

Test Plan:
  - Saw "date" custom fields appear in Conduit API documentation for "maniphest.edit".
  - Set custom "date" field to null and non-null values via the API.

{F6666582}

Maniphest Tasks: T13355

Differential Revision: https://secure.phabricator.com/D20690
2019-07-31 13:10:14 -07:00
epriestley
f55aac49f4 Rename "pastebin" database to "paste"
Summary:
See D20650. Long ago, this got added as "pastebin", but that's the name of another product/company, not a generic term for paste storage.

Rename the database to `phabricator_paste`.

(An alternate version of this patch would rename `phabricator_search` to `phabricator_bing`, `phabricator_countdown` to `phabricator_spacex`, `phabricator_pholio` to `phabricator_adobe_photoshop`, etc.)

Test Plan:
  - Grepped for `pastebin`, now only found references in old patches.
  - Applied patches.
  - Browsed around Paste in the UI without encountering issues.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D20661
2019-07-19 05:53:26 -07:00
epriestley
cb4add3116 In Ferret, allow documents with no title to match query terms by using LEFT JOIN on the "title" ranking field
Summary:
Fixes T13345. See D20650. Currently, `PhabricatorCursorPagedPolicyAwareQuery` does a JOIN against the "title" field so it can apply additional ranking/ordering conditions to the query.

This means that documents with no title (which don't have this field) are always excluded from the result set.

We'd prefer to include them, just not give them any bonus ranking/relevance boost. Use a LEFT JOIN so they get included.

Test Plan:
  - Applied D20650 (diff 1), made it use raw `getTitle()` as the document title, indexed a paste with no title.
  - Searched for a term in the paste body.
  - Before change: no results.
  - After change: found result.

{F6601159}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13345

Differential Revision: https://secure.phabricator.com/D20660
2019-07-18 10:37:36 -07:00
epriestley
9ab5f59ca2 Export "date" and "remarkup" custom fields to Excel + "zip" extension check
Summary:
Fixes T13342. This does a few different things, although all of them seem small enough that I didn't bother splitting it up:

  - Support export of "remarkup" custom fields as text. There's some argument here to export them in some kind of structure if the target is JSON, but it's hard for me to really imagine we'll live in a world some day where we really regret just exporting them as text.
  - Support export of "date" custom fields as dates. This is easy except that I added `null` support.
  - If you built PHP from source without "--enable-zip", as I did, you can hit the TODO in Excel exports about "ZipArchive". Since I had a reproduction case, test for "ZipArchive" and give the user a better error if it's missing.
  - Add a setup check for the "zip" extension to try to avoid getting there in the first place. This is normally part of PHP so I believe users generally won't hit it, I just hit it because I built from source. See also T13232.

Test Plan:
  - Added a custom "date" field. On tasks A and B, set it to null and some non-null value. Exported both tasks to Excel/JSON/text, saw null and a date, respectively.
  - Added a custom "remarkup" field, exported some values, saw the values in Excel.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13342

Differential Revision: https://secure.phabricator.com/D20658
2019-07-18 09:59:20 -07:00
epriestley
987e104610 Bump the remarkup cache version after JIRA/Asana rule changes
Summary: See PHI1319. Ref T13291. Bump the remarkup cache version, since the old JIRA / Asana rules may exist in the partial cached representation of remarkup blocks from older versions.

Test Plan: Typed some comments with various formatting, saw remarkup work fine.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13291

Differential Revision: https://secure.phabricator.com/D20619
2019-06-25 14:56:29 -07:00