Summary:
Ref T9132. This isn't perfect, but doesn't break any existing functionality. This stuff works:
- Editing values.
- Reordering fields.
- All builtin field tyepes.
This stuff may not work yet:
- Assigning custom field defaults.
- Some conduit stuff.
- Fully custom fields?
- Locking/hiding fields? Didn't actually test this one.
I'll keep chipping away at that stuff. In some cases, it may be easier to convert all the CustomField apps first, although Differential might be a fair bit of work.
Test Plan:
Created a bunch of custom fields of every avialable type and edited them.
{F1008789}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14617
Summary: Fixes T9715. Adds a MySQL-based lock to ensure that schema migrations are not applied on multiple hosts simultaneously.
Test Plan: Ran `./bin/storage upgrade` concurrently. One invocation was successful whilst the other hit a `PhutilLockException`.
Reviewers: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T9715
Differential Revision: https://secure.phabricator.com/D14463
Summary: This logic is flipped.
Test Plan:
- Before change: ran `bin/phd debug task`, saw queries to the config table every second.
- After change: ran `bin/phd debug task`, saw queries to the config table every 10 seconds.
Reviewers: chad, joshuaspence
Reviewed By: chad, joshuaspence
Differential Revision: https://secure.phabricator.com/D14542
Test Plan:
I didn't put any skill points in spelling since I need
combat skills to survive in a nuclear wasteland, but spell check says
this is better.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D14522
Summary:
Fixes T9808.
An instance imported a very large repository, generating approximately 4 million tasks over the course of a few days. A week later, these tasks started expiring and became candidates for garbage collection.
The GC works by deleting 100 rows at at time over and over again. It finds the rows it's going to delete by querying for old rows.
Currently, this query generates a `WHERE dateCreated < X ORDER BY id DESC` query. This query can not efficiently execute using a single key, because it relies on `dateCreated` order to find the rows, then on `id` order to sort them. With a table with 4M rows, this is slow.
This would still be OK, except that the query has to execute a lot of times since it only deletes 100 rows each time. Particularly, it needs to execute a total of ~40K times.
Instead, generate `WHERE dateCreated < X ORDER BY dateCreated DESC, id DESC`. This should have the same effect in general and the GC definitely doesn't care about the difference, but it should be more efficient at large scales.
Test Plan:
I had to `TRUNCATE` the problem table so I don't have a perfect repro to completely convincingly test this anymore. Both queries behave fine at small scales, which is why we haven't seen this before.
I was able to run the newer query in production before I nuked the table and have it complete in a reasonable amount of time, while the old query hung longer than I wanted to wait (several minutes?). The query plan for the new query was also a good one, while the query plan for the old query was terrible.
I loaded the daemon console and ran `bin/garbage collect --collector worker.tasks --trace`. I verified the queries looked reasonable and produced reasonable results in production.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9808
Differential Revision: https://secure.phabricator.com/D14505
Summary: Some linter messages, such as those produced by `ArcanistPHPCompatibilityXHPASTLinterRule`, contain backticks but are currently rendered as Remarkup literals. I think that it is generally desirable to allow lint messages to be rendered as Remarkup, although we should ideally have a way to render Remarkup for use on the command line (I actually think that this already exists, but I don't think that `arc lint` does this when rendering linter messages).
Test Plan: Resubmitted D14481 to my dev install and saw Remarkuped lint messages.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14485
Summary: Rename the XHPAST database from `{$NAMESPACE}_xpastview` to `{$NAMESPACE}_xhpast`.
Test Plan: Ran `./bin/storage --namespace test upgrade --no-quickstart`.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14442
Summary: Ref T7053. Remove the `envHash` and `envInfo` fields, which are no longer used now that the daemons restart automagically. Depends on D14458.
Test Plan: Saw no more setup issues.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: tycho.tatitscheff, epriestley
Maniphest Tasks: T7053
Differential Revision: https://secure.phabricator.com/D14446
Summary: Fixes T7053. Depends on D14452.
Test Plan:
Created a custom daemon which dumps out the config hash (by querying `PhabricatorEnv::calculateEnvironmentHash()`). Ran this daemon with `./bin/phd debug PhabricatorDebugDaemon` and saw the config hash update within 30 seconds.
{P1886}
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T7053
Differential Revision: https://secure.phabricator.com/D14458
Summary: Adds some basic style to new !!Remarkup Highlighter!! Ref T5560
Test Plan: Wait for next diff.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T5560
Differential Revision: https://secure.phabricator.com/D14383
Summary:
Fixes T9519. Right now, build steps go straight from the build to the edit screen.
This means that there's no way to see their edit history or review details without edit permission. In particular, this makes it a bit harder to catch the Drydock Blueprint authorization warnings from T9519.
- Add a standard view screen.
- Add a little warning callout to blueprint authorizations.
This also does a bit of a touchup on the weird dropshadow element from T9586. Maybe not totally design-approved now but it's less ugly, at least.
Test Plan:
{F906695}
{F906696}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9519
Differential Revision: https://secure.phabricator.com/D14330
Summary:
It's hard for us to predict how long patches and migrations will take in the general case since it varies a lot from install to install, but we can give installs some kind of rough heads up about longer patches. I'm planning to just put a sort of hint for things in the changelog, something like this:
{F905579}
To make this easier, start storing how long stuff took. I'll write a little script to dump this into a table for the changelog.
Test Plan:
Ran `bin/storage status`:
{F905580}
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14320
Summary:
Fixes T9273. Remarkup has reasonably good fundamentals but the API is a giant pain to work with.
Provide a `PHUIRemarkupView` to make it easier. This object is way simpler to use by default.
It's not currently as powerful, but we can expand the power level later by adding more setters.
Eventually I'd expect to replace `PhabricatorRemarkupInterface` and `PhabricatorMarkupOneOff` with this, but no rush on those.
I converted a few callsites as a sanity check that it works OK.
Test Plan:
- Viewed remarkup in Passphrase.
- Viewed remarkup in Badges.
- Viewed a Conduit method.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9273
Differential Revision: https://secure.phabricator.com/D14289
Summary: Fixes T9573. This incorrectly affected Phriction. I could restore it for only projects, but you didn't like the rule very much anyway and I don't feel strongly about it.
Test Plan: Unit tests.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9573
Differential Revision: https://secure.phabricator.com/D14287
Summary:
Ref T182. This allows you to assign blueprints that a repository can use to perform working copy operations. Eventually, this will support "merge this" in Differential, etc.
This is just UI for now, with no material effects.
Most of this diff is just taking logic that was in the existing "Blueprints" CustomField and putting it in more general places so Diffusion (which does not use CustomFields) can also access it.
Test Plan:
- Configured repository automation for a repository.
- Removed repository automation for a repository.
Reviewers: chad
Reviewed By: chad
Subscribers: avivey
Maniphest Tasks: T182
Differential Revision: https://secure.phabricator.com/D14259
Summary:
Ref T9551. We currently use the same logic for generating project hashtags and Phriction slugs, but should be a little more conservative with project hashtags.
Stop them from generating with stuff that won't parse in a "Reviewers:" field or generally in commments (commas, colons, etc).
Test Plan:
Created a bunch of projects with nonsense in them and saw them generate pretty reasonable hashtags.
{F873456}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9551
Differential Revision: https://secure.phabricator.com/D14261
Summary:
Ref T9551. To set things up:
- Name a project `aa bb`. This will have the tag `aa_bb`.
- Try to visit `/tag/aa%20bb`.
Here's what happens now:
- You get an Aphront redirect error as it tries to add the trailing `/`. Add `phutil_escape_uri()` so that works again.
- Then, you 404, even though this tag is reasonably equivalent to the real project tag and could be redirected. Add a fallback to lookup, resolve, and redirect if we can find a hit for the tag.
This also fixes stuff like `/tag/AA_BB/`.
Test Plan: Visited URIs like `/tag/aa%20bb`, `/tag/aa%20bb/`, `/tag/Aa_bB/`, etc. None of them worked before and now they all do.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9551
Differential Revision: https://secure.phabricator.com/D14260
Summary:
Ref T9519. This is like 80% of the way there and doesn't fully work yet, but roughly shows the shape of things to come. Here's how it works:
First, there's a new custom field type for blueprints which works like a normal typeahead but has some extra logic. It's implemented this way to make it easy to add to Blueprints in Drydock and Build Plans in Harbormaster. Here, I've added a "Use Blueprints" field to the "WorkingCopy" blueprint, so you can control which hosts the working copies are permitted to allocate on:
{F869865}
This control has a bit of custom rendering logic. Instead of rendering a normal list of PHIDs, it renders an annotated list with icons:
{F869866}
These icons show whether the blueprint on the other size of the authorization has approved this object. Once you have a green checkmark, you're good to go.
On the blueprint side, things look like this:
{F869867}
This table shows all the objects which have asked for access to this blueprint. In this case it's showing that one object is approved to use the blueprint since I already approved it, but by default new requests come in here as "Authorization Requested" and someone has to go approve them.
You approve them from within the authorization detail screen:
{F869868}
You can use the "Approve" or "Decline" buttons to allow or prevent use of the blueprint.
This doesn't actually do anything yet -- objects don't need to be authorized in order to use blueprints quite yet. That will come in the next diff, I just wanted to get the UI in reasonable shape first.
The authorization also has a second piece of state, which is whether the request from the object is active or inactive. We use this to keep track of the authorization if the blueprint is (maybe temporarily) deleted.
For example, you might have a Build Plan that uses Blueprints A and B. For a couple days, you only want to use A, so you remove B from the "Use Blueprints: ..." field. Later, you can add B back and it will connect to its old authorization again, so you don't need to go re-approve things (and if you're declined, you stay declined instead of being able to request authorization over and over again). This should make working with authorizations a little easier and less labor intensive.
Stuff not in this diff:
- Actually preventing any allocations (next diff).
- Probably should have transactions for approve/decline, at least, at some point, so there's a log of who did approvals and when.
- Maybe should have a more clear/loud error state when no blueprints are approved?
- Should probably restrict the typeahead to specific blueprint types.
Test Plan:
- Added the field.
- Typed some stuff into it.
- Saw the UI update properly.
- Approved an authorization.
- Declined an authorization.
- Saw active authorizations on a blueprint page.
- Didn't see any inactive authroizations there.
- Clicked "View All Authorizations", saw all authorizations.
Reviewers: chad, hach-que
Reviewed By: chad
Maniphest Tasks: T9519
Differential Revision: https://secure.phabricator.com/D14251
Summary: Fixes T9538. Ref T9408. `cowsay` and `figlet` Remarkup rules are being mangled in HTML mail right now. Put them in <pre> to unmangle them.
Test Plan:
Sent myself a cow + figlet in mail.
Used `bin/mail show-outbound --id ... --dump-html > dump.html` + open that HTML file in Safari to preview HTML mail.
Saw linebreaks and monospaced formatting.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9538, T9408
Differential Revision: https://secure.phabricator.com/D14248
Summary: Ref T9514. I missed these when I swapped out the console stuff recently.
Test Plan: Ran `bin/storage probe`, saw bold instead of escape sequences.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9514
Differential Revision: https://secure.phabricator.com/D14240
Summary:
Ref T9252. Currently, `queueTask()` accepts `$priority` as its third argument. Allow it to take a full range of `$options` instead. This API just never got updated after we expanded avialable options.
Arguably this whole API should be some kind of "TaskQueueRequest" object but I'll leave that for another day.
Test Plan:
- Grepped for `queueTask()` and verified no other callsites are affected by this API change.
- Ran some daemons.
- See also next diff.
Reviewers: hach-que, chad
Reviewed By: hach-que, chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14235
Summary:
Ref T9252. Currently, Harbormaster and Drydock work like this in some cases:
# Queue a lease for activation.
# Then, a little later, save the lease PHID somewhere.
# When the target/resource is destroyed, destroy the lease.
However, something can happen between (1) and (2). In Drydock this window is very short and the "something" would have to be a lighting strike or something similar, but in Harbormaster we wait until the resource activates to do (2) so the window can be many minutes long. In particular, a user can use "Abort Build" during those many minutes.
If they do, the target is destroyed but it doesn't yet have a record of the artifact, so the artifact isn't cleaned up.
Make these things work like this instead:
# Create a new lease and pre-generate a PHID for it.
# Save that PHID as something that needs to be cleaned up.
# Queue the lease for activation.
# When the target/resource is destroyed, destroy the lease if it exists.
This makes sure there's no step in the process where we might lose track of a lease/resource.
Also, clean up and standardize some other stuff I hit.
Test Plan:
- Stopped daemons.
- Restarted a build in Harbormaster.
- Stepped through the build one stage at a time using `bin/worker execute ...`.
- After the lease was queued, but before it activated, aborted the build.
- Processed the Harbormaster side of things only.
- Saw the lease get destroyed properly.
Reviewers: chad, hach-que
Reviewed By: hach-que
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14234
Summary:
Fixes T9494. This:
- Removes all the random GC.x.y.z config.
- Puts it all in one place that's locked and which you use `bin/garbage set-policy ...` to adjust.
- Makes every TTL-based GC configurable.
- Simplifies the code in the actual GCs.
Test Plan:
- Ran `bin/garbage collect` to collect some garbage, until it stopped collecting.
- Ran `bin/garbage set-policy ...` to shorten policy. Saw change in web UI. Ran `bin/garbage collect` again and saw it collect more garbage.
- Set policy to indefinite and saw it not collect garabge.
- Set policy to default and saw it reflected in web UI / `collect`.
- Ran `bin/phd debug trigger` and saw all GCs fire with reasonable looking queries.
- Read new docs.
{F857928}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9494
Differential Revision: https://secure.phabricator.com/D14219
Summary: Ref T9494. Depends on D14216. Remove 10 copies of this code.
Test Plan: Ran `arc unit --everything`, browsed Config > Modules, clicked around Herald / etc.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9494
Differential Revision: https://secure.phabricator.com/D14217
Summary:
Ref T9252. This primarily allows Harbormaster to request (and Drydock to fulfill) working copies with a patch from a staging area. Doing this means we can do builds on in-review changes from `arc diff`.
This is a little cobbled-together but should basically work.
Also fix some other issues:
- Yielded, awakend workers are fine to update but could complain.
- We can't log slot lock failures to resources if we don't end up saving them.
- Killing the transaction would wipe out the log.
- Fix some TODOs, etc.
Test Plan: Ran Harbormaster builds on a local revision.
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14214
Summary:
Ref T9252. This is the same as D14201, but for lease stuff instead of resource stuff.
This one is a little heavier but still feels pretty reasonable to me at the end of the day (worker is <1K lines and has a ton of comment stuff).
Also fixes a few random bugs I hit in the task queue.
Test Plan:
- Restarted some Harbormaster builds, saw them go through cleanly.
- Released pre-activation resources/leases.
- Probably still kinda buggy but I'll iron the details out over time.
Logs are starting to look somewhat plausible:
{F855747}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14202
Summary:
Fixes T9479. Currently, `@aaaaaaaa` may try to match as a commit hash, and `@C123456` may try to match as a Countdown reference. These should only match as user mentions.
Prevent object mention rules from matching after `@`. We already prevent them after `-` and `#`, and already prevented the username rule after `@` (i.e., preventing `@@user`).
Test Plan:
Created some "interesting" users locally and `@mentioned` them:
{F850779}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9479
Differential Revision: https://secure.phabricator.com/D14186
Summary:
Ref T9123. The handling in D14183 didn't deal with new field values properly.
Make all this handling more consistent.
Test Plan: Created a new WorkignCopy build plan with some repos.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9123
Differential Revision: https://secure.phabricator.com/D14184
Summary:
Ref T9123. To run upstream builds in Harbormaster/Drydock, we need to be able to check out `libphutil`, `arcanist` and `phabricator` next to one another.
This adds an "Also Clone: ..." field to Harbormaster working copy build steps so I can type all three repos into it and get a proper clone with everything we need.
This is somewhat upstream-centric and a bit narrow, but I don't think it's totally unreasonable, and most of the underlying stuff is relatively general.
This adds some more typechecking and improves data/type handling for custom fields, too. In particular, it prevents users from entering an invalid/restricted value in a field (for example, you can't "Also Clone" a repository you don't have permission to see).
Test Plan: Restarted build, got a Drydock resource with multiple repositories in it.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9123
Differential Revision: https://secure.phabricator.com/D14183
Summary:
Ref T9252. Currently, Harbormaster does this when trying to acquire a working copy:
- Ask for a working copy.
- Yield for 15 seconds.
- Check if we have a working copy yet.
That's OK, but Drydock takes ~1s to acquire a working copy lease if a resource is already available, so we end up doing this:
- T+0: Ask for a working copy.
- T+0: Yield for 15 seconds.
- T+1: Working copy lease activates.
- T+15: Working copy lease is used.
- T+16: Build finishes.
So we end up spending about 2 seconds doing work and 14 seconds sleeping.
One way to fix this would be to fiddle with the yield duration, so we yield for 1, 2, 4, ... seconds or something. This probably isn't a bad idea for longer leases (i.e., wait for 15, 30, 45 ... seconds or similar) but it implies a lot of churn for short leases.
Instead, let tasks "awaken" other tasks when they complete. The "awaken" operation means: if a task is in a yielded state (no failures, no owner, explicitly yielded, future expires time), pretend it only yielded until right now instead of whenever it really yielded to.
Basically, this rewrites history so that even though Harbormaster did a `yield(15)`, we pretend it did a `yield(4)` after we activate the lease if lease activation took 4 seconds.
If this misses, it's fine: we fall back to the normal yield behavior and things move forward normally a few seconds later.
If it hits, we get a more nimble process pretty cleanly.
Test Plan:
- Restarted a build plan (lease working copy + run `ls`) with this patch no-op'd, took about 16 seconds.
- Restarted a build plan with this patch active, took about 1 second.
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14178
Summary:
Fixes T6569. This implements an expiry mechanism for Drydock resources which parallels the mechanism for leases.
A few things are missing that we'll probably need in the future:
- An "EXPIRES" command to update the expiration time. This would let resources be permanent while leased, then expire after, say, 24 hours without any leases.
- A callback like `shouldActuallyExpireRightNow()` for resources and leases that lets them decide not to expire at the last second.
- A callback like `didAcquireLease()` for resource blueprints, to parallel `didReleaseLease()`, letting them clear or extend their timer.
However, this stuff would mostly just let us tune behaviors, not really open up new capabilities.
Test Plan: Changed host resources to expire after 60 seconds, leased one, saw it vanish 60 seconds later.
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T6569
Differential Revision: https://secure.phabricator.com/D14176
Summary: Fixes T9260. That task has a good description of the issue.
Test Plan: Followed steps in T9260 to reproduce the issue. Applied patch; issue went away.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9260
Differential Revision: https://secure.phabricator.com/D14169
Summary: Ref T6569. If a lease is activated with an expiration date, schedule a task to try to clean it up after that time.
Test Plan:
- Used `bin/drydock lease ... --until ...` to activate a lease in the near future.
- Waited for a bit.
- Saw it expire and get destroyed at the scheduled time.
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T6569
Differential Revision: https://secure.phabricator.com/D14148
Summary:
Ref T9252. This simplifies some Drydock code.
Most of this code relates to the old notion of Drydock being able to enumerate all the tasks it needs to complete in order to acquire a lease. The code has stepped back from this, since it's unnecessary, the queue is more powerful than it used to be, and it would be a lot of work to keep track of.
The ~only thing that should ever wait for leases in modern code is `bin/drydock lease`, and it's fine for it to just sit there sleeping, so this just does that.
This reduces the granularity of logging, but I'll address that separately in future logging-focused changes.
Test Plan: Used `bin/drydock lease` to acquire a lease, saw it acquire cleanly.
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14147
Summary:
Ref T9253. For resources and leases that need to do something which takes a lot of time or requires waiting, allow them to allocate/acquire first and then activate later.
When we allocate a resource or acquire a lease, the blueprint can either activate it immediately (if all the work can happen quickly/inline) or activate it later. If the blueprint activates it later, we queue a worker to handle activating it.
Rebuild the "working copy" blueprint to work with this model: it allocates/acquires and activates in a separate step, once it is able to acquire a host.
Test Plan: With some power of imagination, brought up a bunch of working copies with `bin/drydock lease --type working-copy ...`
Reviewers: hach-que, chad
Reviewed By: hach-que, chad
Maniphest Tasks: T9253
Differential Revision: https://secure.phabricator.com/D14127
Summary:
Ref T9253. See discussion in D13843.
I want to let Drydock blueprints for Almanac services choose those services from a typeahead, but only list appropriate services in the typeahead. To do this:
- Provide a StandardCustomField for an arbitrary datasource.
- Adjust the AlmanacServiceDatasource to allow filtering by service class.
This implementation is substantially the same as the one in D13843, with some adjustments:
- I lifted most of the code in the `Users` standard custom field into a new `Tokenizer` standard custom field.
- The `Users` and `Datasource` custom fields now extend the `Tokenizer` custom field and can share most of the code it uses.
- I exposed this field fully as a configurable field. I don't think anyone will ever use it, but this generality costs us nearly nothing and improves consistency.
- The code in D13843 didn't actually pass the parameters over the wire, since the object that responds to the request is not the same object that renders the field. Use the "parameters" mechanism in datasources to get things passed over the wire.
Test Plan:
- Created a custom "users" field in Maniphest and made sure it still wokred.
- Created a custom "almanc services" field in Maniphest and selected some services for a task.
- With additional changes from D13843, selected an appropriate Almanac service in a new Drydock blueprint.
Reviewers: hach-que, chad
Reviewed By: hach-que, chad
Maniphest Tasks: T9253
Differential Revision: https://secure.phabricator.com/D14111
Summary:
Ref T7785. Makes Figlet available without installing the `figlet` package.
The PEAR Text_Figlet code is really sketchy and includes this API, which is quite marvelous:
```
function loadFont($filename, $loadgerman = true)
```
At some point, this should probably be rewritten into a modern style, but it's not trivial since the figlet file format and rendering engine are somewhat complicated. I made some adjustments:
- Broke the dependency on the PEAR core.
- Prevented it from doing any wrong HTML escaping.
- Looked through it for any glaring security or correctness problems.
This code isn't very pretty or modern, but as far as I can tell it's safe and does render Figlet fonts in a reasonable way.
Test Plan: {F803268}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9408, T7785
Differential Revision: https://secure.phabricator.com/D14102
Summary:
Ref T7785. Convert the Cowsay Remarkup rule to use a PHP implementation so we don't have to execute an external `cowsay` binary.
I removed some of the default ".cow" files that come with Cowsay because they:
- include Perl code which we can not interpret; or
- are primarily in-jokes or standalone visual puns or artwork rather than usable actors on the grand stage of cowsay; or
- offended my delicate sensibilities.
Users can add new cows to `resources/cows/custom/` if they want to make new cows available.
I have included a majestic original artwork depicting the "Companion Cube" character from //Portal//.
Test Plan: {F802535}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9408, T7785
Differential Revision: https://secure.phabricator.com/D14100
Summary: Ref T9408. This rule is unsafe in principle, and a practical vulnerability has been found by a security researcher.
Test Plan: `grep`
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9408
Differential Revision: https://secure.phabricator.com/D14103
Summary:
Fixes T9126. In particular:
- Add "Browse" links to all history views.
- Use icons to show "Browse" and "History" links, instead of text.
- Use FontAwesome.
- Generally standardize handling of these elements.
This might need a little design attention, but I think it's an improvement overall.
Test Plan:
- Viewed repository history.
- Viewed branch history.
- Viewed file history.
- Viewed table of contents on a commit.
- Viewed merged changes on a merge commit.
- Viewed a directory containing an external.
- Viewed a deleted file.
{F788419}
{F788420}
{F788421}
{F788422}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9126
Differential Revision: https://secure.phabricator.com/D14096
Summary: Fixes T8572. Ideally we would probably just permit this, but clean up the behavior until the day arrives when inline code is actually rewritten.
Test Plan:
- Tried to launch editors in Differential and Diffusion while comments were already open.
- Verified that "Jump to inline" works in both cases.
{F788008}
{F788009}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8572
Differential Revision: https://secure.phabricator.com/D14094
Summary: See D14025. In all cases where we compare hashes, use strict, constant-time comparisons.
Test Plan: Logged in, logged out, added TOTP, ran Conduit, terminated sessions, submitted forms, changed password. Tweaked CSRF token, got rejected.
Reviewers: chad
Reviewed By: chad
Subscribers: chenxiruanhai
Differential Revision: https://secure.phabricator.com/D14026
