1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-23 14:00:56 +01:00
Commit graph

15728 commits

Author SHA1 Message Date
Valerio Bozzolan
80484b76a5 Remarkup Code-block: parse language specifier in markdown
Summary:
We add support to code blocks with the language expressed as GitLab/GitHub/StackOverflow/...
"flavored markdown".

So we support this syntax: (to avoid confusion see it online on the Diff)

    lang=text
    ```php
    $asd = 1;
    ```

Before this change, this was the only supposed syntax in Remarkup, with an explicit "lang=":

    lang=text
    ```lang=php
    $asd = 1;
    ```

This change introduces a minor risk to eat legitimate Remarkup content, since Remarkup allows
to do a multi-line in this way:

    lang=text
    ```$asd = 1;
    $asd = 2;```

The above example still works, but, there is a chance that hardcore Remarkup people
have a problem when doing a code block to mention programming languages.

In short, this can be problematic since "cpp" will be eaten from this list:

    COUNTEREXAMPLE
    ```cpp
    php
    python
    ```

Using the above example is not socially nice because it is not usable in GitLab, GitHub and Stack Overflow.

If your first line is eaten:

Just *add* a newline on the top to reach a valid raw Markdown list (suggested, valid in Remarkup + Markdown):

    lang=text
    ```
    cpp
    php
    python
    ```

Or, just add "text" to specify that as language (suggested, valid in Remarkup + Markdown):

    lang=text
    ```text
    cpp
    php
    python
    ```

Or, just *remove* a newline from the bottom to reach a valid raw Remarkup list (Remarkup-only):

    lang=text
    ```cpp
    php
    python```

Or, just specify that you are writing in the language "text" (Remarkup-only):

    lang=text
    ```lang=text
    cpp
    php
    python```

To reduce impact and help you, the logic of this strict implementation is:

- must have backticks
- must not have any valid remarkup option, like lang=, counterexample, etc.
- must not have content in the same line of the last backticks
- must have a known language in our proposed subset

If everything is OK, we remove that language from the content since it would be otherwise displayed.

Interestingly, this could improve performance when rendering README files or snippets from external
websites, since - in case - we do not need to guess the language using our deep dark magic.

Closes T15481

Test Plan:
We added some nice unit tests. Ensure that this test passes:

    PhutilRemarkupEngineTestCase::testEngine

Optionally, take vision of these, before and after:

https://we.phorge.it/P16

Change the test plan slightly every time, to make sure it is not in your cache.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15481

Differential Revision: https://we.phorge.it/D25299
2023-07-04 18:23:14 +02:00
Aviv Eyal
603cf474ee Fix int fields for now
Summary: Fix T15516

Test Plan: Viewed an "int" custom field, no boom.

Reviewers: speck, valerio.bozzolan, O1 Blessed Committers

Reviewed By: valerio.bozzolan, O1 Blessed Committers

Subscribers: tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15516

Differential Revision: https://we.phorge.it/D25325
2023-07-04 02:26:46 -07:00
Aviv Eyal
d725ffaa77 Fix "Register with Approval flow" for php 8
Summary: Ref T15064.

Test Plan:
- Register as a new user
- as admin, view user's page and approve it
- As new user, hit "wait patiently" to go back to Home

No crashes in this flow.

Reviewers: O1 Blessed Committers, chris

Reviewed By: O1 Blessed Committers, chris

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15064

Differential Revision: https://we.phorge.it/D25317
2023-07-02 08:49:12 -07:00
Aviv Eyal
9aafbb42ef fix strlen in PhabricatorMailManagementShowOutboundWorkflow
Summary: Ref T15064

Test Plan: run `./bin/mail show-outbound --id 1` on a non-html email with php 8.1, no crash.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Tags: #php_8_support

Maniphest Tasks: T15064

Differential Revision: https://we.phorge.it/D25275
2023-07-02 08:49:02 -07:00
MacFan4000
aebd7a2f72 Update a couple of lingering instances of secure.phabricator.com to we.phorge.it
Summary: We were still pointint to secure.phabricator.com in a couple places. This fixes that.

Test Plan: No testing needed

Reviewers: O1 Blessed Committers, speck, Cigaryno

Reviewed By: O1 Blessed Committers, speck, Cigaryno

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25316
2023-06-30 12:01:54 -04:00
Steve Campbell
73253489bf Make src/infrastructure/javelin/markup.php phabricator_form PHP 8.1 compliant
Summary:
src/infrastructure/javelin/markup.php phabricator_form() performs a strcasecmp() operation on an optional attribute withoiut checking to see if it exists first. This causes an 'Passing null to parameter #1 ($string1) of type string is deprecated' error.

Because we subsequenly check to see that the value equals /POST/i, all we need to do is check that the value has some 'true' value before doing the strcasecmp(). If it's not true, then it won't be POST!

Fixes T15509

Test Plan: View a diff. Eg: https://my.phorge.site/D1234

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15509

Differential Revision: https://we.phorge.it/D25319
2023-06-30 15:13:20 +01:00
Steve Campbell
101ce834b8 Fix PHP 8.1 strlen() error when database replication Seconds_Behind_Master is NULL
Summary:
When MySQL database replication is configured, but not running, the slave status Seconds_Behind_Master is NULL.
When this value is passed to strlen() an error is thrown.

Fixes T15507

Test Plan:
* Configure database replication such that Seconds_Behind_Master is NULL
* Restart php-fpm
* Go to Phorge web site.

Reviewers: O1 Blessed Committers, chris

Reviewed By: O1 Blessed Committers, chris

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15507

Differential Revision: https://we.phorge.it/D25318
2023-06-30 13:49:32 +01:00
Taavi Väänänen
0a17287e08
Generalize references to PHP 5.x Debian/Ubuntu packages
Summary:
Both Debian and Ubuntu have been shipping PHP 7.x (or now 8.x) for a while.
This patch replaces the versioned php5-foo package references with more general
php-foo package names.

Test Plan: Navigated to the documentation pages that this patch updates.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25315
2023-06-29 15:58:54 +03:00
Valerio Bozzolan
8ffa604462 Diviner: mention how to flag Arcanist and Phorge as "safe" git repos
Summary:
This tip is useful if you are affected by the following error in git or similar:

    fatal: detected dubious ownership in repository at '/var/www/phorge'

Just follow the new tip from Diviner to fix at system level.

Ref T15282

Test Plan:
- see if you like the documentation tip
- try to execute the command, and verify that your computer does not explode
- eventually visit the /config/ page with and without that change (with and without a /etc/gitconfig)

Reviewers: O1 Blessed Committers, avivey, Cigaryno

Reviewed By: O1 Blessed Committers, avivey, Cigaryno

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15282

Differential Revision: https://we.phorge.it/D25236
2023-06-28 23:14:31 +02:00
Valerio Bozzolan
1005e38160 Fix notification message when Aphlict is not configured
Summary:
Before this change the footer in the notification bar was a bit
weird on the message "Notification server not enabled":

| Before    | After     |
|-----------|-----------|
| {F315024} | {F315001} |

Also a bit wrong on the "Connecting..." message:

| Before    | After     |
|-----------|-----------|
| {F315061} | {F315072} |

The fix involves a refactor in the HTML structure in order to
match the graphic rendered when Aphlict is configured and running.

In short, we pass from this:

    COUNTEREXAMPLE
    <span class="connection-status-text aphlict-connection-status-notenabled">
      <span class="visual-only phui-icon-view phui-font-fa fa-circle-o grey"></span>
      Notification server not enabled
    </span>

To this:

    <span class="visual-only phui-icon-view phui-font-fa fa-circle-o grey"></span>
    <span class="connection-status-text aphlict-connection-status-notenabled">Notification server not enabled</span>

See the Task for additional details.

Closes T15415

Test Plan:
Follow the official Notification documentation about the config `notification.servers`,
open the Notification top-bar, and check the message in these conditions:

- without the config
- with the config
- with/without Aphlict running

https://we.phorge.it/book/phorge/article/notifications/

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15415

Differential Revision: https://we.phorge.it/D25312
2023-06-27 13:30:11 +02:00
Valerio Bozzolan
659e13fa6d Fix UX regression in Dashboard Tabs Panel
Summary:
Before this change, it seems that the Dashboard Tabs Panel could be empty
as default. This was the regression:

cbc0e66154

The variable $is_selected was defined inside a foreach loop, but that loop
was terminated when it was then read again. This redefines that variable in
the right scope.

| Before    | After     |
|-----------|-----------|
| {F312797} | {F312798} |

Closes T15474

Test Plan: - see that now you have something visibile in your Dashboard Tabs Panel

Reviewers: speck, O1 Blessed Committers

Reviewed By: speck, O1 Blessed Committers

Subscribers: tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15474, T15146

Differential Revision: https://we.phorge.it/D25291
2023-06-27 08:15:16 +02:00
Andre Klapper
e4fe54edf0 Fix typo "recipeint" in metamta.one-mail-per-recipient description
Test Plan: Go to `/config/settings/all` and look at the description of `metamta.one-mail-per-recipient`

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25311
2023-06-26 14:28:01 +02:00
Valerio Bozzolan
a25af8ccef Diviner: add a minimal systemd example
Summary:
After this change the section "Managing Daemons with phd"
mentions a minimally viable systemd configuration file
to make systemd-users happier and systemd-haters saddest.

Closes T15467

Test Plan: - read the page, see systemd, don't cry

Reviewers: O1 Blessed Committers, 20after4

Reviewed By: O1 Blessed Committers, 20after4

Subscribers: 20after4, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15467

Differential Revision: https://we.phorge.it/D25293
2023-06-23 08:50:16 +02:00
Andre Klapper
7c1487e4dc Fix Pholio RuntimeException: Undefined variable $dictionary (when adding an empty Inline Comment)
Summary:
Trying to create an empty inline comment in a Pholio mock, `$dictionary` does not get set as both `strlen($v_content)` and `$inline->getID()` are not true.

Thus show a more explanatory error message ('Comment cannot be empty.') to users instead of exposing internal variable names.

```
EXCEPTION: (RuntimeException) Undefined variable $dictionary at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=b325304b6e52), phorge(head=master, ref.master=980293b707a0)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/pholio/controller/PholioInlineController.php:117]
```

Closes T15456

Test Plan: After applying this change, try to add an empty Inline Comment in a Pholio mock. See that the error message is now "Comment cannot be empty." instead of "Undefined variable $dictionary".

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15456

Differential Revision: https://we.phorge.it/D25281
2023-06-19 14:13:36 +02:00
Andre Klapper
1c59b65421 Dashboards: add capability who can create Dashboards
Summary:
Add a `dashboard.create` capability to allow limiting dashboard creation (and creation of related panels).
This can reduce spam in open Phorge installations in which anyone can create an account while you still want anyone to be able to view existing dashboards.

Closes T15438

Test Plan:
1. As an admin, go to `/applications/view/PhabricatorDashboardApplication/`
2. See only two options "Can Use Application: All Users" and "Can Configure Application: Administrators"
3. Apply patch
4. As an admin, go to `/applications/view/PhabricatorDashboardApplication/` and see the new option "Can Create Dashboards" set to "All Users"
5. As an average user, go to `/dashboard/` and successfully create a new Dashboard
6. As an admin, go to `/applications/edit/PhabricatorDashboardApplication/` and change "Can Create Dashboards" from "All Users" to "Administrators", select "Save Policies" button
7. As an average user, go to `/dashboard/` and see that "Create Dashboard" is disabled
8. As an average user, go to an existing dashboard not created by this user and see that selecting "Edit Dashboard" shows "You do not have permission to edit this object."
9. As an average user, go to the existing dashboard that you created yourself and see that the Edit Policy is set to this user.
10. As an average user, go to the existing dashboard that you created yourself and see that you can still create or add panels.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15438

Differential Revision: https://we.phorge.it/D25270
2023-06-19 12:12:22 +02:00
Mukunda Modell
327ac921c2 Conduit column.search: add status, sequence and isDefault to API results
Summary:
This seems like a fairly obvious oversight with the column.search API.

Knowing:

1. isHidden - boolean to indicate Active vs Archived
2. isDefaultColumn - the one that Tasks get dropped in by default, usually called "Backlog"
3. sequence - numerical order on the Workboard

are all necessary for a lot of things that very sensible real-world API clients need to do when working with columns.

Partial cherry-pick from:

https://phabricator.wikimedia.org/rPHABebfe30890b52784d222ec4ed36c05462b2a57f92

Closes T15484

Test Plan:
Tested on phabricator.wikimedia.org over many months and used by real client apps.

To do additional tests, visit this page:

/conduit/method/project.column.search/

Check that the new fields are returned correctly and nothing explodes.

Reviewers: #blessed_committers, O1 Blessed Committers, 0, avivey, valerio.bozzolan

Reviewed By: #blessed_committers, O1 Blessed Committers, 0, valerio.bozzolan

Subscribers: Cigaryno, 0, speck, tobiaswiese, valerio.bozzolan, Matthew

Maniphest Tasks: T15081, T15484

Differential Revision: https://we.phorge.it/D25038
2023-06-19 08:58:52 +02:00
Andre Klapper
478012cd46 Add serious business string for defrocking a user (remove admin rights)
Summary:
Allow humans to understand what happened when `phabricator.serious-business` mode is enabled: The administrator role was removed from a user account.

Closes T15480

Test Plan: Enable `phabricator.serious-business` mode. Enable and disable an account as admin. Check that account's feed and the general feed.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15480

Differential Revision: https://we.phorge.it/D25298
2023-06-16 13:58:28 +02:00
Valerio Bozzolan
7fe622e5d3 Auto-subscription: little less verbose
Summary:
When the user "Mrs. Kitten" adds or remove "Mrs. Kitten" as Subscriber,
usually these messages were generated:

    Mrs. Kitten added a subscriber: Mrs. Kitten.
    Mrs. Kitten removed a subscriber: Mrs. Kitten.

This was a bit like the Spiderman meme self-pointing Spiderman.

After this change, these beautiful messages are generated instead:

    Mrs. Kitten subscribed.
    Mrs. Kitten unsubscribed.

| Before    | After     |
| {F286215} | {F286216} |

Closes T15347

Test Plan:
- subscribe on something
- unsubscribe from something
- all other cases remain as-is

Reviewers: O1 Blessed Committers, bfs, speck

Reviewed By: O1 Blessed Committers, bfs, speck

Subscribers: bfs, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15347

Differential Revision: https://we.phorge.it/D25191
2023-06-15 11:52:02 +02:00
Andre Klapper
1c098c273d Fix PHP 8.1 "strlen(null)" exception calling Conduit's user.whoami
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=97e163187418), phorge(head=master, ref.master=7bbd657c19c4)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/conduit/controller/PhabricatorConduitAPIController.php:686]
```

Closes T15469

Test Plan: After applying this change, `/api/user.whoami` correctly renders the "Method Call Result"

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15469

Differential Revision: https://we.phorge.it/D25292
2023-06-13 07:31:52 +02:00
Andre Klapper
7bbd657c19 Fix PHP 8.1 "strlen(null)" exception when Diffusion repository has no tags
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=diffusionEmptyRepoPatternSearch, ref.master=97e163187418, ref.diffusionEmptyRepoPatternSearch=97e163187418), phorge(head=master, ref.master=108cbcd09bd3)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/controller/DiffusionTagListController.php:28]
```

Closes T15461

Test Plan: After applying this line, get the expected `CommandException: Command failed with error #128!` due to ` fatal: detected dubious ownership in repository at '/var/repo/1'` (plus a `RuntimeException: file_exists(): Passing null to parameter #1 ($filename) of type string is deprecated` as a bonus), instead of the previous RuntimeException about `strlen()`.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15461

Differential Revision: https://we.phorge.it/D25285
2023-06-12 21:50:35 +02:00
Andre Klapper
54ee51d7ab Work around exception in Herald transcript of old tasks: Call to a member function getAppliedTransactionPHIDs() on bool
Summary:
In some cases, older Herald transcripts which got garbage collected display an exception.
For unknown reasons, calling `getObjectTranscript()` on the HeraldTranscript object `$xscript` can return a bool with the value `false`.
Afterwards, calling `getAppliedTransactionPHIDs()` on that bool throws an exception.
Thus as a workaround, return an empty array instead in this case, like the code already does when $xscript->getObjectTranscript()->getAppliedTransactionPHIDs() === null.

Note that it remains unclear whether/when the bool may have the value `true` hence this situation is not covered by this code change (the code would still throw an exception in this situation).

Closes T15343

Test Plan:
1. Change system time to one year back (or optionally, use `./phorge/bin/garbage set-policy --collector herald.transcripts --days 1`, drop steps 9 and 10, and wait a day before performing step 11)
2. As an admin, go to http://phorge.localhost/herald/create/
3. Select "Maniphest Tasks"
4. On http://phorge.localhost/herald/create/?adapter=HeraldManiphestTaskAdapter , select "Global Rule"
5. Under "Conditions", select `any of` and set the three conditions `Description | contains | Internet Archive`, `Description | contains | archive.org`, `Description | contains | Wayback Machine`.
6. Under "Action", select `Add projects | someProject`
7. Select "Save Rule"
8. Trigger the rule by creating a new task.
9. Change system time back to today
10. On the shell, run `./phorge/bin/phd restart`
11. Visit created task and select "View Herald Transcript"
12. Transcript page displays "Old Transcript - Details of this transcript have been garbage collected." and shows no exception anymore

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15343

Differential Revision: https://we.phorge.it/D25246
2023-06-12 21:49:14 +02:00
Andre Klapper
3f8846d80c Fix PHP 8.1 "strlen(null)" exception on Diffusion repository History page after setting Callsign
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=97e163187418), phorge(head=diffusionCallsignHistory, ref.master=9b99123ff931, ref.diffusionCallsignHistory=9b99123ff931)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/xaction/PhabricatorRepositoryCallsignTransaction.php:28]
```

Closes T15465

Test Plan: After applying this change, `/diffusion/XYZ/manage/history/` renders correctly and shows `user set the callsign for this repository to XYZ.`

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15465

Differential Revision: https://we.phorge.it/D25289
2023-06-12 21:47:45 +02:00
Andre Klapper
1060736e89 Fix some icons not displayed on UIExamples' "Icons and Images" (PHUIIconExample) page
Summary:
Fix three typos in icon names

Closes T15454

Test Plan: Go to `/uiexample/view/PHUIIconExample/` before and after applying the change; compare icons displayed for `fa-codiepie`, `fa-user-o`, `fa-thermometer-0`

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15454

Differential Revision: https://we.phorge.it/D25280
2023-06-12 17:29:20 +02:00
Andre Klapper
efbb809d42 Fix PHP 8.1 "strlen(null)" exception which blocks rendering commit page in Diffusion
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=97e163187418), phorge(head=diffusionCallsignHistory, ref.master=9b99123ff931, ref.diffusionCallsignHistory=d5fb6702a49a)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/controller/DiffusionCommitController.php:31]
```

Closes T15466

Test Plan: After applying this change, going to an existing commit URL like `/diffusion/XYZ/commit/999a2c7d1ff8` shows the exception covered by T15464 instead, as expected.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15466

Differential Revision: https://we.phorge.it/D25290
2023-06-12 11:21:33 +02:00
Andre Klapper
32b2d3871f Fix PHP 8.1 "strlen(null)" exceptions trying to browse Diffusion repository code
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=97e163187418), phorge(head=diffusionBrowseCode, ref.master=108cbcd09bd3, ref.diffusionBrowseCode=108cbcd09bd3)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/controller/DiffusionBrowseController.php:25]
```

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=97e163187418), phorge(head=diffusionBrowseCode, ref.master=108cbcd09bd3, ref.diffusionBrowseCode=108cbcd09bd3)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php:40]
```
Closes T15462

Test Plan: After applying these two changes, get next expected exceptions about `RuntimeException: file_exists(): Passing null to parameter #1 ($filename) of type string is deprecated` and `CommandException: Command failed with error #128!  fatal: detected dubious ownership in repository at '/var/repo/1'`, but no RuntimeException about `strlen()` anymore.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15462

Differential Revision: https://we.phorge.it/D25286
2023-06-12 03:50:12 +02:00
Andre Klapper
db19b23dce Fix PHP 8.1 "strlen(null)" exceptions on History page of Diffusion repo after changing text encoding
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=97e163187418), phorge(head=master, ref.master=108cbcd09bd3)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/xaction/PhabricatorRepositoryEncodingTransaction.php:20]
```

Closes T15460

Test Plan: After applying these two changes, changing the text encoding of a Diffusion repository to a valid encoding and going to `/diffusion/1/manage/history/`, page renders correctly and shows no exception.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15460

Differential Revision: https://we.phorge.it/D25284
2023-06-12 03:49:34 +02:00
Andre Klapper
331b9cca42 Fix PHP 8.1 "strlen(null)" exception rendering a commit in Diffusion when bugtraq.url = null
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=97e163187418), phorge(head=diffusionBrowseCommit, ref.master=9b99123ff931, ref.diffusionBrowseCommit=9b99123ff931)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/controller/DiffusionCommitController.php:925]
```

Closes T15464

Test Plan: After applying this change, selecting the last commit on http://phorge.localhost/diffusion/query/all/ correctly renders the commit page (e.g. `/R1:1234567890abcdef1234567890abcdef12345678`)

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15464

Differential Revision: https://we.phorge.it/D25288
2023-06-12 03:37:14 +02:00
Andre Klapper
9b99123ff9 Fix PHP 8.1 "strlen(null)" exceptions trying to browse Diffusion repository history
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=97e163187418), phorge(head=diffusionBrowseCode, ref.master=108cbcd09bd3, ref.diffusionBrowseCode=015ffef14b0c)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php:50]
```

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=97e163187418), phorge(head=diffusionBrowseCode, ref.master=108cbcd09bd3, ref.diffusionBrowseCode=015ffef14b0c)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php:50]
```

Closes T15463

Test Plan: After applying these two changes, get next expected exceptions about `RuntimeException: file_exists(): Passing null to parameter #1 ($filename) of type string is deprecated` and `CommandException: Command failed with error #128!  fatal: detected dubious ownership in repository at '/var/repo/1'`, but no RuntimeException about `strlen()` anymore.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15463

Differential Revision: https://we.phorge.it/D25287
2023-06-12 02:51:20 +02:00
Andre Klapper
108cbcd09b Fix PHP 8.1 "base64_decode(null)" exception calling Conduit's file.upload with no data passed
Summary:
Since PHP 8.1, `base64_decode()` does not accept passing null as a parameter. Thus first check that `data !== null` before calling `decodeBase64($data)` (which then calls PHP's `base64_decode()`), and throw an exception if it is.

```
EXCEPTION: (RuntimeException) base64_decode(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=18554ea76ceb), phorge(head=conduitDashboardPanelEdit, ref.master=0d81da590923, ref.conduitDashboardPanelEdit=ab4391b30465)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<arcanist>/src/error/PhutilErrorHandler.php:261]
  #1 <#2> base64_decode(NULL, boolean) called at [<phorge>/src/applications/files/conduit/FileConduitAPIMethod.php:84]
```

Closes T15426

Test Plan: Applied this change; afterwards `/api/file.upload` under "Method Result", "error_info" shows "Unable to decode base64 data!" instead of "base64_decode(): Passing null to parameter #1 ($string) of type string is deprecated" which is more descriptive.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15426

Differential Revision: https://we.phorge.it/D25258
2023-06-11 16:34:16 +02:00
Andre Klapper
7b57ba2b98 Fix PHP 8.1 "strlen(null)" exception initializing Diffusion repository without path access permissions
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

Closes T15459

Test Plan: After applying this change, going to `/diffusion/17/manage/`, the actual error (`cannot mkdir /var/repo/17: Permission denied`) is propagated and displayed in the "Working Copy Status" section, instead of being hidden by a `strlen()` exception. See T15459 for details.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15459

Differential Revision: https://we.phorge.it/D25283
2023-06-10 22:59:57 +02:00
Andre Klapper
c553692d45 Fix PHP 8.1 "strlen(null)" exception which blocks rendering errors on Create Blueprint page
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=e4fd31ec024e), phorge(head=D25240, ref.master=b1edfea09bad, ref.D25240=b1edfea09bad)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php:392]
```
Closes T15413

Test Plan:
Applied this change on top of D25239. Afterwards, the "Create Blueprint" page on `/drydock/blueprint/edit/form/default/` after pressing the "Create Blueprint" button the page correctly renders the expected error that some fields cannot be empty.

# Enter a commit message.
#
# Changes:
#
#     src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php

Also, try to create extra numeric fields (like in the User application) and try to send zero or 1 using Conduit `user.edit`.

Reviewers: O1 Blessed Committers, valerio.bozzolan, speck

Reviewed By: O1 Blessed Committers, valerio.bozzolan, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15413

Differential Revision: https://we.phorge.it/D25240
2023-06-10 22:58:28 +02:00
Andre Klapper
b4cfe56f03 Fix PHP 8.1 "ltrim(null)" exception which blocks rendering first Meme comment
Summary:
Since PHP 8.1, passing a null string to `ltrim(string $string)` is deprecated.

Thus first check if After and Below text are not null before trimming.

Closes T15379

Test Plan:
Applied this change; afterwards several times created a new Pholio mock and added a Meme comment. I could not reproduce the problem anymore (first meme comment in a mock always rendered correctly instead of showing an exception as comment). However, I could not reliably reproduce the problem anyway.

- Create a meme without any above/below text
- Create a meme with just spaces as above/below text
- Create a meme with just above text as "asd", or "0" or a lizard
- Create a meme with just below text as "asd", or "0" or a lizard
- Create a meme with both above and below texts with "asd", "0" and a lizard and more stuff (doing all 64 combinations)

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15379

Differential Revision: https://we.phorge.it/D25212
2023-06-10 18:41:00 +02:00
Andre Klapper
76ef9db8a5 Fix PHP 8.1 "strlen(null)" exception about Staging URI on Diffusion repo History page
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
Passing null to strlen() is deprecated since PHP 8.1. Thus first check that the string is not null.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=b325304b6e52), phorge(head=master, ref.master=83edbffd521d)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/xaction/PhabricatorRepositoryStagingURITransaction.php:20]
```
This is quite similar to D25277.

Closes T15458

Test Plan: After applying this change, editing the Staging URI by saving its already empty value, and going to the repo History page, the page `/diffusion/1/manage/history/` renders without an exception, showing `R1` being Inactive and `user set as the staging area for this repository.` [sic!]

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15458

Differential Revision: https://we.phorge.it/D25282
2023-06-10 18:34:42 +02:00
Andre Klapper
bab9970740 Fix PHP 8.1 "trim(null)" exception which blocks rendering Conduit's harbormaster.sendmessage page
Summary:
Since PHP 8.1, passing a null string to `trim()` is deprecated.
Thus first check that `$content` is not null before trimming it.

Also since trim() returns a string and never null, we can also simplify the non-empty check,
in a more readable and efficient way, avoiding strlen() that was usually used for other "more wild" cases.

```
EXCEPTION: (RuntimeException) trim(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=18554ea76ceb), phorge(head=master, ref.master=0d81da590923)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<arcanist>/src/error/PhutilErrorHandler.php:261]
  #1 <#2> trim(NULL) called at [<phorge>/src/infrastructure/markup/blockrule/PhutilRemarkupTableBlockRule.php:117]
```

Closes T15427

Test Plan: Applied this change, afterwards `/conduit/method/harbormaster.sendmessage/` correctly rendered in web browser.

Reviewers: O1 Blessed Committers, speck, valerio.bozzolan

Reviewed By: O1 Blessed Committers, speck, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15427

Differential Revision: https://we.phorge.it/D25259
2023-06-10 16:26:49 +02:00
Valerio Bozzolan
83edbffd52 Config page: add lovely git-related error messages in standard error log
Summary:
Premise: the Config page runs git commands. Spoiler: they can fail.

Before this change errors were just suppressed and ignored.

After this change you get at least a log line. Also, you get a tip for a very specific well-known error affecting recent git.

Probably suppressing stuff was fine in the moment git worked here. But nowadays
git doesn't work so easily here, since it introduced very weird additional
configurations in order for a repository to be indicated as "safe" or not.

Error suppression was a problem there, because understanding the error with
"future objects" is not trivial for most users. Really.

After this change, these errors are beautifully mentioned in the standard log
of your webserver, to the best of our communication ability.

This is a cute example of a new log line:

    Cannot identify the version of the phorge repository because the webserver does not trust it (more info on Task https://we.phorge.it/T15282).
    Try this system resolution:
    sudo git config --system --add safe.directory /var/www/phorge

Another:

    Cannot identify the version of the phorge repository because the webserver does not trust it (more info on Task https://we.phorge.it/T15282).
    Try this system resolution:
    sudo git config --system --add safe.directory /var/www/arcanist

Incidentally, these specific errors probably afflict your Phorge/Phabricator, and now
you have some useful resolution tips. You are welcome!

You can also join T15282 to discuss your specific case.

Closes T15243

Test Plan:
- visit the `/config/` page: does it worked before? it still works now
- visit the `/config/` page without `/etc/gitconfig`: you may still see "Unknown" as version - but, you finally get something in the log (instead of nothing)
- visit the `/config/` page after following your log messages: now you should see the library versions! yeeh

Additional tests:

- manually sabotage the command "git log" replacing with "gitfooolog" and visit /config page: see the unexpected 'gitfooolog command not found' log line
- manually sabotage the command "git remove" replacing with "gitremotelog" and visit /config/ page: see the unexpected 'gitremotelog command not found' log line

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, deadalnix, aklapper, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15243

Differential Revision: https://we.phorge.it/D25148
2023-06-10 13:21:41 +02:00
Andre Klapper
7af82abcb9 PHP 8.1 "strlen(null)" exception on SVN History page after changing "Import Only"
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=b325304b6e52), phorge(head=master, ref.master=cb938d869c3f)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/xaction/PhabricatorRepositorySVNSubpathTransaction.php:25]
```

Closes T15445

Test Plan: After applying this change, and after setting some "Import Only" value for a new Subversion repository, the page `/diffusion/1/manage/history/` renders showing `R1` being Inactive and `user set the repository "Import Only" path to main`.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15445

Differential Revision: https://we.phorge.it/D25277
2023-06-08 14:53:44 +02:00
Andre Klapper
b09471d557 Fix PHP 8.1 "strlen(null)" exceptions creating a Diffusion Identity without entering assignee
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=b325304b6e52), phorge(head=diffusionCreateIdentity, ref.master=cbc0e661544a, ref.diffusionCreateIdentity=cbc0e661544a)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/xaction/PhabricatorRepositoryIdentityAssignTransaction.php:55]
```

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=b325304b6e52), phorge(head=diffusionCreateIdentity, ref.master=cbc0e661544a, ref.diffusionCreateIdentity=cbc0e661544a)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php:298]
```

```
EXCEPTION: (RuntimeException) mb_detect_encoding(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=b325304b6e52), phorge(head=diffusionCreateIdentity, ref.master=cbc0e661544a, ref.diffusionCreateIdentity=cbc0e661544a)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<arcanist>/src/error/PhutilErrorHandler.php:261]
  #1 <#2> mb_detect_encoding(NULL, array) called at [<phorge>/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php:313]
```

Closes T15440

Test Plan: After applying these three changes, going to `/diffusion/identity/edit/form/default/` and selecting the "Create Identity" button will only show a AphrontQueryException unrelated to PHP 8 and covered in T15439 instead.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15440

Differential Revision: https://we.phorge.it/D25271
2023-06-08 14:52:26 +02:00
Andre Klapper
9aa88ae0d7 Fix PHP 8.1 "strlen(null)" exception on History page of Diffusion repo after renaming default branch
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=arcpatch-D25250, ref.master=18554ea76ceb, ref.arcpatch-D25250=fade4603a799), phorge(head=master, ref.master=e11c5486c92b)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/xaction/PhabricatorRepositoryDefaultBranchTransaction.php:25]
```

Closes T15434

Test Plan: After applying this change, after changing the name of the (previously empty) default branch of a Diffusion repository, and then going to the repository History at `/diffusion/<someID>/manage/history/`, the page renders correctly in the web browser instead of showing an exception.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15434

Differential Revision: https://we.phorge.it/D25265
2023-06-08 14:50:43 +02:00
Andre Klapper
25c4f6224d Fix some PHP 8.1 "strlen(null)" exceptions on Differential Revision page
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=18554ea76ceb), phorge(head=diff3, ref.master=e11c5486c92b, ref.diff3=e11c5486c92b)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/differential/customfield/DifferentialBranchField.php:42]
```

Closes T15432

Test Plan: After applying these three changes (on top of D25262 and D25263), there is no `strlen()` related exception displayed on `/D1/` anymore, however there are further exceptions to be sorted out in other tasks.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15432

Differential Revision: https://we.phorge.it/D25264
2023-06-08 14:49:28 +02:00
Andre Klapper
44a8a1c408 Fix PHP 8.1 "preg_match(null)" exception on "Create Revision" page in Differential when not entering data
Summary:
`preg_match()` does not accept passing null as the `$subject` string parameter in PHP 8.1.

Thus add a `phutil_nonempty_string()` check if the `$subject` parameter is a non-empty string.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=18554ea76ceb), phorge(head=diffAttach, ref.master=e11c5486c92b, ref.diffAttach=e11c5486c92b)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<arcanist>/src/error/PhutilErrorHandler.php:261]
  #1 <#2> preg_match(string, NULL, NULL) called at [<phorge>/src/applications/differential/editor/DifferentialTransactionEditor.php:221]
```

Closes T15431

Test Plan: After applying this change on top of D25262, going to `/differential/diff/create/`, creating a diff,  and entering nothing on the "Create Revision" page on `/differential/revision/attach/1/to/` correctly shows the errors "You must provide a test plan. Describe the actions you performed to verify the behavior of this change. Revisions must have a title." in web browser instead of an exception.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15431

Differential Revision: https://we.phorge.it/D25263
2023-06-08 14:48:45 +02:00
Andre Klapper
b3894bc2c6 Fix PHP 8.1 "strlen(null)" exception viewing branches of non-existing Hg repo
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=b325304b6e52), phorge(head=HgViewBranches, ref.master=cb938d869c3f, ref.HgViewBranches=cb938d869c3f)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php:83]
```

Closes T15447

Test Plan: After applying this change, going to `/diffusion/1/branches/default/` shows an expected FilesystemException instead of the previous RuntimeException.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15447

Differential Revision: https://we.phorge.it/D25279
2023-06-08 10:32:23 +02:00
Andre Klapper
66dd1c35d8 Fix PHP 8.1 "strlen(null)" exception on SVN repo page with no branches
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=b325304b6e52), phorge(head=SvnRepoMain, ref.master=cb938d869c3f, ref.SvnRepoMain=cb938d869c3f)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/controller/DiffusionController.php:156]
```

Closes T15446

Test Plan: After applying this change, going to a newly created SVN repository  at `/diffusion/1/`, the page renders, showing the repository as Inactive and also "Empty Repository - This repository does not have any commits yet." as expected.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15446

Differential Revision: https://we.phorge.it/D25278
2023-06-08 10:31:20 +02:00
Andre Klapper
1028eb3ee9 Fix PHP 8.1 "strlen(null)" exception setting a Diffusion repo URI when diffusion.ssh-host is null
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=b325304b6e52), phorge(head=diffusionCreateRepoURI, ref.master=d8d65f3f87ed, ref.diffusionCreateRepoURI=d8d65f3f87ed)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/storage/PhabricatorRepositoryURI.php:755]
```

Closes T15444

Test Plan: After applying this change and setting a repo URI following the steps in T15444, the page "R10: URI 58" on `/diffusion/10/uri/view/58/` is correctly rendered without an exception.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15444

Differential Revision: https://we.phorge.it/D25273
2023-06-08 10:30:08 +02:00
Valerio Bozzolan
71e4eee275 Update Figlet implementation to be PHP8 compatible
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.

Imported from:

https://secure.phabricator.com/rPd5c63c86e7e4e87d5f72b35b1bdb1e888aea49bc

https://secure.phabricator.com/rPbc6f4786a2e36441d17b765fde8e8e047840bc58

Closes T15289

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: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15064, T15289

Differential Revision: https://we.phorge.it/D25142
2023-06-05 09:16:57 -04:00
Aviv Eyal
cb938d869c Teach ./bin/celerity about sprite maps
Summary:
Add `./bin/celerity sprites`, to replace script `./scripts/celerity/generate_sprites.php`.
Also make new workflow run `./bin/celerity map` at the same time.

Fixes T15437.

Test Plan: Changes a file that goes in the sprites, run new command

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15437

Differential Revision: https://we.phorge.it/D25274
2023-06-03 09:57:05 -07:00
Andre Klapper
d8d65f3f87 Fix PHP 8.1 "strlen(null)" exception on UIExamples' Bars page
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=b325304b6e52), phorge(head=uiExamples, ref.master=dd24c94b0741, ref.uiExamples=dd24c94b0741)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/view/phui/PHUISegmentBarSegmentView.php:58]
```

Closes T15441

Test Plan:
After applying this change, going to `/uiexample/view/PhabricatorAphrontBarUIExample/`,"Bars (PhabricatorAphrontBarUIExample)" renders with beautiful colors.

To see that page, enable prototypes with:

    ./bin/config set phabricator.show-prototypes true

Also try to create a Milestone on a Project, with the Config `maniphest.points` enabled, and try to create some Tasks with different points. Visit its Workboard to see the bar that still works with a useful tooltip.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15441

Differential Revision: https://we.phorge.it/D25272
2023-06-03 11:50:52 +02:00
Andre Klapper
dd24c94b07 Fix PHP 8.1 "strlen(null)" exceptions block creating a diff in Differential web interface
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=18554ea76ceb), phorge(head=diffDiff, ref.master=e11c5486c92b, ref.diffDiff=e11c5486c92b)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php:160]
```

Closes T15430

Test Plan:
After applying these two changes, going to `/differential/diff/create/`, pasting the content of a diff file into the "Raw Diff" field, and selecting the "Create Diff" button, `/differential/diff/1/` rendered correctly in web browser.

Also, install xdebug and try again with coverage mode enabled in your php.ini of your PHP cli.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15430

Differential Revision: https://we.phorge.it/D25262
2023-06-02 20:40:42 +02:00
Andre Klapper
0cbcb110b0 Fix PHP 8.1 "strlen(null)" exceptions trying to View Branches of a Diffusion repository
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=e4fd31ec024e), phorge(head=D25241, ref.master=b1edfea09bad, ref.D25241=b1edfea09bad)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/controller/DiffusionBranchTableController.php:29]
```

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=e4fd31ec024e), phorge(head=D25241, ref.master=b1edfea09bad, ref.D25241=b1edfea09bad)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php:33]
```

Closes T15414

Test Plan: Applied these two changes and at least got to `(FilesystemException) Filesystem path "/var/repo/1/" does not exist.` as an error message, instead of being stuck at `(RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated`.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15414

Differential Revision: https://we.phorge.it/D25241
2023-06-02 20:05:14 +02:00
dylsss
cbc0e66154 Change erroneous loose comparison to strict comparison in tab panels
Summary:
Fix a loose comparison causing a bug when comparing the selected tab (0) with a tab which has an alphanumeric ID which doesn't begin with an integer.

E.g., `(0 == 'kq3p37awi2n5')` is true in PHP 7.4 and below, this can sometimes cause multiple tabs to be displayed when a tab panel is first loaded onto a page:

{F278735}

Test Plan: Create a tab panel with at least 3 tabs, add a couple more and ensure that multiple tabs aren't loaded.

Reviewers: O1 Blessed Committers, valerio.bozzolan, Cigaryno, avivey

Reviewed By: O1 Blessed Committers, valerio.bozzolan, Cigaryno, avivey

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15146

Differential Revision: https://we.phorge.it/D25067
2023-06-02 15:57:09 +02:00
Andre Klapper
6b8ec50148 Fix PHP 8.1 "preg_match(null)" exception when cloning a repository with no URI set
Summary:
`preg_match()` does not accept passing null as the `$subject` string parameter in PHP 8.1.

Thus add a `phutil_nonempty_string()` check if the `$subject` parameter is a non-empty string.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

```
EXCEPTION: (RuntimeException) preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=arcpatch-D25250, ref.master=18554ea76ceb, ref.arcpatch-D25250=fade4603a799), phorge(head=diffusionEmptyBranchHistory, ref.master=e11c5486c92b, ref.diffusionEmptyBranchHistory=76f042e4b969)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<arcanist>/src/error/PhutilErrorHandler.php:261]
  #1 <#2> preg_match(string, NULL) called at [<phorge>/src/applications/diffusion/request/DiffusionGitRequest.php:6]
```

Closes T15435

Test Plan: After applying this change, try to "Clone" a Repository with no URI set shows the overlay dialog `Clone Repository - Repository has no URIs set.` as expected.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15435

Differential Revision: https://we.phorge.it/D25266
2023-05-31 09:40:22 +02:00