Summary:
See D21862. Ref T13700. D21862 affected notification server URI generation behavior when a notification server client is configured with:
1. no "path" argument; and
2. "cluster.instance" not set.
Condition (1) is not true default, and condition (2) is not true in my local environment, so it was easy for this to slip through the cracks. Apply the change suggested in D21862.
Also fix a couple other string-null issues I caught locally.
Test Plan: Generated notification server URIs under various simulated local conditions (no instance, path set) and everything seemed to be working correctly.
Maniphest Tasks: T13700
Differential Revision: https://secure.phabricator.com/D21875
Summary:
More perusing through Phabricator to find and address incompatibilities.
Refs T13588
Test Plan:
- Creating macros, changing their text.
- Upload to use existing SSH key instead of generate one.
- Modify alt text of an uploaded file.
- Create phame post
- Delete phriction document
- Move tasks around on workboard, assign story points
- Generate Diviner docs
- Bulk modify tasks
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T13588
Differential Revision: https://secure.phabricator.com/D21869
Summary:
Navigate through all the conduit pages and address PHP8 incompatibilities. Additionally test out empty form submissions on many of the "create" or "update" APIs and return more contextual errors rather than PHP8 errors.
Refs T13588
Test Plan:
- Clicked through all the conduit APIs.
- Clicked submit on empty forms, or minimally filled-in forms.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley, PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13588
Differential Revision: https://secure.phabricator.com/D21872
Summary:
Using dashboard/panels and addressing PHP8 incompatibilities.
Refs T13588
Test Plan: Create a dashboard with multiple panels, including a tabbed panel. Update the tabbed panel to have multiple tabs of content. Try to have a tabbed panel contain itself.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T13588
Differential Revision: https://secure.phabricator.com/D21871
Summary: Browsing through all other non-prototype applications and using them a bit to track down PHP8 incompatibilities.
Test Plan:
I tried basic usage of the following applications which found & fixed:
- Almanac, creating resources, networks, services, etc.
- Drydock, creating blue prints
- Pholio mocks
- Setting up TOTP MFA
- Phame blog and post
- Creating default Space
- Creating and signing a legalpad document
- Creating a conpherence room
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D21868
Summary:
While testing https://secure.phabricator.com/D21864 I ran into some issues getting mercurial HTTP access working. Using wireshark I confirmed that my local mercurial 6.4 was not including command arguments as HTTP headers but in the querystring.
I didn't dig too deep into understanding when/why this started happening. The protocol documents this in [[ https://repo.mercurial-scm.org/hg/file/tip/mercurial/helptext/internals/wireprotocol.txt | wireprotocol.txt ]].
>Command arguments can be sent multiple ways. The simplest is part of the URL query string using ``x-www-form-urlencoded`` encoding (see Python's ``urllib.urlencode()``. However, many servers impose length limitations on the URL. So this mechanism is typically only used if the server doesn't support other mechanisms.
Based on that either the mercurial on the server is really old (it's 6.1.1 tho) or maybe some other parsing/info passing in Phab's handling of the wire protocol is causing the client to downgrade the wire protocol support.
Test Plan: Host mercurial repo using HTTP, test push/pull.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D21867
Summary:
Updating compatibility for PHP 8.2 for remarkup-related functionality.
This also resolves an issue introduced by https://secure.phabricator.com/D21860. Of all the flags when opening the zip the one I chose was not introduced in 5.2 but in 7.4
Test Plan: I rendered comments using figlet, cowsay, images, object references.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D21866
Summary: Running through setting up and using Diffusion repositories and addressing PHP8 issues that come up.
Test Plan:
I set up a hosted mercurial repository and pushed commits to it over ssh.
I set up a hosted git repository and pushed commits to it over ssh.
I set up a mirrored mercurial repository over ssh.
I set up a mirrored git repository over ssh.
I created a diff on a git repository and landed it.
I created a diff on a mercurial repository and landed it.
I cloned and pushed a commit to a mercurial repo over http.
I cloned and pushed a commit to a git repo over http.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D21864
Summary:
Updates to all of the `ProfileMenuItem` classes to be compatible with PHP 8.
These changes were moved from D21862.
Refs T13588
Test Plan:
I loaded the profile page for a user and saw all the items I expected to see.
I navigated through the different profile nav items.
I loaded the settings page for a user and saw all the items I expected to see.
I navigated through the differnt setting nave items.
I added Cat Facts to my navigation.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T13588
Differential Revision: https://secure.phabricator.com/D21863
Summary:
Starting with a new instance running PHP 8.2, address all exceptions that come up through some basic browsing/usage.
For `strlen(null)` issues I generally tried to resolve if the value should be non-null at the point of issue, and attempt to address at earlier call-site. There were not many of these that I could determine. In the rest of those cases I would replace with a null-and-strlen check, or use `phutil_nonempty_string` if I was certain the value was a string and it was more convenient.
Hitting all code-paths is challenging, so I would search for `strlen` within radius of files I was modifying and evaluate to address those uses in the same manner.
Notes:
- `AphrontRequest::getStr` only ever returns a string, and is safe to use `phutil_nonempty_string`.
- `PhabricatorEnv::getEnvConfig` can return non-string things so any values coming from there should never use `phutil_nonempty_string`.
- `AphrontRequest::getHTTPHeader` indicates it could return wild so `phutil_nonempty_string` should not be used.
- `AphrontRequest::getURIData` isn't clear if it could return non-string data, so never use `phutil_nonempty_string`.
Refs T13588
Test Plan: I'm running an instance on 8.2 and went through the basic setup/installation, startup and usage, including setup issues and configurations/settings.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: aklapper, Korvin, epriestley
Maniphest Tasks: T13588
Differential Revision: https://secure.phabricator.com/D21862
Summary: The change from https://secure.phabricator.com/D21860 introduced a PHP error due to an invalid variable reference (`zip` instead of `$zip`). This fixes that issue as well as confirms that pk-zip compressed figlet font files can continue to be used/loaded.
Test Plan:
1. I ensured I had numerous figlet font files installed in `resources/figlet/custom` and used `file` to verify that `univers.flf` and `puffy.flf` are pk-zip archives.
2. With this change applied I added a comment with `figlet(font=univers){{{hello}}}` and verified that the comment used the univers font to render `hello`, and like-wise with the `puffy` font.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D21861
Summary:
As of PHP ~v8 the `zip_open` and associated functions have been deprecated and
removed. The replacement is the `ZipArchive` API. This updates the figlet
implementation to use this API which has been present in PHP since 5.2.
Additionally in PHP 8 the use of squiggly brackets for indexing into arrays is
also deprecated. This updates to remove two uses of squiggly brackets and
replace with square brackets.
These two deprecations would result in being unable to load differential
revisions in which someone had commented using figlet remarkup.
Refs T13588
Test Plan:
Applied these changes to an install and loaded a revision that had comments
where someone utilized figlet remarkup. The revision loaded properly and the
figlet comment rendered properly.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T13588
Differential Revision: https://secure.phabricator.com/D21860
Summary: Ref T13682. After some change prior change likely connected to T13682, file PHIDs fail to extract properly in Pholio.
Test Plan:
- Created a new Pholio mock with at least one image.
- Before: fatal when extracting data from this datastructure.
- After: clean mock creation.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21858
Since we want usernames to depend on the email provided, hack are way
through it. An extension can not handle this level of intrusiveness
I'm sorry oh coding gods
Summary:
Ref T13685. "RemarkupEditField" uses a "RemarkupValue" internally, and it currently attempts to serialize into the database unsuccessfully.
Instead, flatten the value before returning it for storage.
Test Plan:
- Edited the default description of a task in a custom form.
Maniphest Tasks: T13685
Differential Revision: https://secure.phabricator.com/D21854
Summary:
Ref T13588. See PHI2198. Some pathways represent SVN commit identifiers (which are numeric) with integers; these raise an exception in `phutil_nonempty_string()`, which is strict.
(Ideally, commit identifiers wouldn't have mixed types, but that's a dramatically less practical change.)
Test Plan: Will deploy.
Maniphest Tasks: T13588
Differential Revision: https://secure.phabricator.com/D21852
Summary: See <https://hackerone.com/reports/1573143>. The pathway for disabling global builtin queries is missing a policy check. Add it.
Test Plan:
- Accessed the "/search/delete/id/.../" URI for a global builtin query as a non-administrator.
- Before patch: could improperly disable queries.
-After patch: proper policy exception.
Differential Revision: https://secure.phabricator.com/D21851
Summary: See <https://hackerone.com/reports/1584409>. This obsolete API method is technically policy-violating: Owners packages have had policies since D15275. See also T10360.
Test Plan: Viewed Conduit API console, no longer saw API method available.
Differential Revision: https://secure.phabricator.com/D21850
Summary:
Ref T13682. Versioned drafts may have missing or unexpected metadata:
- versioned drafts from an older version of Phabricator may be missing metadata;
- versioned drafts created by an older UI against a newer version of Phabricator may have `null` metadata.
Generally, make these workflows robust to metadata in unexpected formats, so database debris doesn't break the UI.
Test Plan: Simulated debris, interacted with UI.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21849
Summary: Ref T13682. This prepares for modernizing Slowvote and exposing a more usable API.
Test Plan: Ran migrations, opened and closed polls.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21848
Summary: Ref T13682. Prepares for use of API-friendly string constants rather than opaque integers.
Test Plan: Created and edited polls, opening and closing them. Grepped for affected methods.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21847
Summary: Ref T13682. Use API-friendly string constants instead of opaque integers in Slowvote voting methods.
Test Plan: Created, edited, and voted in polls with various voting methods. Examined database after migrations.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21846
Summary: Ref T13682. Extract Slowvote vote types to a separate object, to prepare for turning them into API-friendly string constants.
Test Plan: Created, edited, and voted in Slowvote polls. Grepped for affected constants.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21845
Summary: Ref T13682. Migrate and update Slowvote to use API-friendly string constants for response visibility, not opaque integers.
Test Plan: Created and edited slowvotes, changing response visibility.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21844
Summary: Ref T13682. This change supports modifying these constants to be sensible strings instead of opaque integers.
Test Plan: Created and edited polls.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21843
Summary:
Ref T13682. Ref T13065. Migrate and update "SlowvotePoll" to use more modern mailkey storage (in the mail application, rather than onboard).
Also make other minor modernization changes.
Test Plan: Created a new poll. Ran migration, verified mail key copied properly in database.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13682, T13065
Differential Revision: https://secure.phabricator.com/D21842
Summary: Ref T13682. Use a more modern PHID key name for this key.
Test Plan: Ran `bin/storage adjust`, got a modern key.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21841
Summary: Ref T13682. Make the "Attached" list in Files a bit more detailed, and add a "Detach" button.
Test Plan: Tried to detach unrelated, referenced, and attached files. Saw attached files detach.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21840
Summary:
Ref T13682. When editing a comment, the edit pathway may invoke file PHID extraction without first formalizing an object on the Editor.
Pass the "object" explicitly to generally improve the robustness of this pathway.
Test Plan: Edited a comment.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21839
Summary: Ref T13682. Many subclasses of "CursorPagedPolicyAwareQuery" have the same implementation of "loadPage()", and this is a sensible default behavior.
Test Plan: Examined changes to verify that all removed methods have the same behavior.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21838
Summary: Ref T13682. Allow users to manually attach files which are referenced (but not attached) via the UI.
Test Plan: Reference files via `{F...}`, then attached them via the UI workflow.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21837
Summary: Ref T13682. Provide a basic UI element for showing file attached to an object.
Test Plan: Viewed objects with attached files in the UI.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21836
Summary: Ref T13682. This supports an "Attached Files" curtain UI element.
Test Plan: See next change.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21835
Summary:
Ref T13682. This is a marginal case, but if you quote someone else's text and it has a file reference embedded in it, that isn't good enough to generate "attachment intent".
Since you need both a reference and an explicit attachment, this should never actually affect any user-visible behavior today, but makes the ruleset more thorough.
Test Plan: Dragged and dropped a file, referencing it in either a quoted or unquoted block. Saw it attach only for a quoted block.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21833
Summary:
Ref T13682. When a user uploads a file, then changes their mind and deletes the reference to the file, we don't actually want to attach the file.
When choosing which files to attach, only attach files which are both referenced in Remarkup and explicitly attached in remarkup metadata.
Test Plan:
- Dropped a file into a comment, submitted it, saw it attach normally.
- Dropped a file into a comment, deleted the reference, submitted it, saw no attachment.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21832
Summary: Ref T13682. This property may not exist, and should defualt to an empty array if not present.
Test Plan: Created a new object.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21834
Summary:
Ref T13682. This method originated in D4781 and no longer has any callers; remove it.
(I believe this was from an era where macros didn't have to be on their own line to trigger.)
Test Plan: Grepped for method, found no callers.
Maniphest Tasks: T13682
Differential Revision: https://secure.phabricator.com/D21831
Summary:
Ref T13603. On common edit pathways, extract explicit file attachments from Remarkup. These pathways are affected:
- Objects that use EditEngine and expose a remarkup area via "RemarkupEditField".
- Objects that use EditEngine to generate a comment area.
This is the vast majority of pathways, but not entirely exhaustive.
Test Plan: Created and commented on a task, explicitly attaching images. Saw images attach properly.
Maniphest Tasks: T13603
Differential Revision: https://secure.phabricator.com/D21830
Summary:
Ref T13603. This removes the file attachment edge types, which no longer have readers or writers.
It does not delete the underlying data.
This indirectly removes API access to this edge. As far as I know, this was only used by one customer (see D21480) who has migrated away from Phabricator.
Test Plan: Grepped for edge constants.
Maniphest Tasks: T13603
Differential Revision: https://secure.phabricator.com/D21823
