Summary:
With this patch, the underlying exception described in T15372#8537 still remains. However, with this patch, the bug is more contained as it is not triggered when not calling an interpreter (`cowsay`, `figlet`), so Phorge does not crash rendering `noValidInterpreter {{{foo}}} bar` lines but renders them as is (for whatever reasons such lines may exist).
See T15372
Test Plan:
Enter strings into a comment:
* `invalid {{{saysay}}} foo` now renders as plain text instead of crashing
* `invalid (invalid) {{{saysay}}} foo` now renders as plain text instead of crashing
* `cowsay (invalid) {{{saysay}}} foo` will still crash as before
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25415
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_scalar()` as a replacement when both string and integers could be passed as a value like here.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_scalar() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
Also fix similar warning for `ctype_digit(): Argument of type null will be interpreted as string in the future` by checking for `null` first.
```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=customOAuthUrlencodeNull, ref.master=788098096e11, ref.customOAuthUrlencodeNull=4f0f2043b7e9), phorge(head=master, ref.master=bcfcd9acfc12)
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php:27]
```
```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=customOAuthUrlencodeNull, ref.master=788098096e11, ref.customOAuthUrlencodeNull=4f0f2043b7e9), phorge(head=customFieldDate, ref.master=bcfcd9acfc12, ref.customFieldDate=bcfcd9acfc12)
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php:35]
```
```
EXCEPTION: (RuntimeException) ctype_digit(): Argument of type null will be interpreted as string in the future at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=customOAuthUrlencodeNull, ref.master=788098096e11, ref.customOAuthUrlencodeNull=4f0f2043b7e9), phorge(head=customFieldDate, ref.master=bcfcd9acfc12, ref.customFieldDate=bcfcd9acfc12)
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<arcanist>/src/error/PhutilErrorHandler.php:261]
#1 <#2> ctype_digit(NULL) called at [<phorge>/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php:77]
```
Closes T15601
Test Plan: After applying these three changes and creating a custom field with `"type": "date"` under `/config/edit/maniphest.custom-field-definitions/`, the website `/transactions/editengine/maniphest.task/view/5/` renders correctly in the browser, showing "This is a preview of the current form configuration."
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15601
Differential Revision: https://we.phorge.it/D25389
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=customOAuthUrlencodeNull, ref.master=788098096e11, ref.customOAuthUrlencodeNull=4f0f2043b7e9), phorge(head=master, ref.master=226f3150933d)
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php:271]
```
Closes T15594
Test Plan: Add a build step in Harbormaster, select "Interacting with External Build Systems > Build with Buildkite" at `/harbormaster/step/new/1/HarbormasterBuildkiteBuildStepImplementation/`, select the "Create Build Step" button. Page now shows several expected error messages (API Token is required; Organization Name is required; etc) instead of a PHP error.
Reviewers: O1 Blessed Committers, Matthew
Reviewed By: O1 Blessed Committers, Matthew
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15594
Differential Revision: https://we.phorge.it/D25379
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, however use `phutil_nonempty_scalar()` as the value might not necessarily be a string object.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_scalar() 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=788098096e11), phorge(head=arcpatch-D25239, ref.master=840a7fab2bc8, ref.arcpatch-D25239=870d62ce0ed9)
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php:36]
```
Closes T15581
Test Plan:
Page `Create Blueprint` at `/drydock/blueprint/edit/form/default/` renders as expected in web browser.
Try also creating a custom integer field and put some fuzzy data.
https://we.phorge.it/book/phorge/article/custom_fields/
Test these values:
- 1 (stays as-is)
- 0 (stays as-is)
- 123.45 (becomes 123)
- a "lizard" (becomes zero)
- empty ("") (becomes empty)
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15581
Differential Revision: https://we.phorge.it/D25371
Summary:
Under PHP 8.1, PhabricatorEditorURIEngine::newForViewer() is throwing a trim(NULL) error when trying to view a diff.
This is because it tries to apply string operations to a user setting which will be null by default.
Fixes T15518
Test Plan:
Unit test added -
arc unit
Or just view a diff. Eg:
https://my.phorge.site/D1234
Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey
Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15518
Differential Revision: https://we.phorge.it/D25324
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
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
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
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
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
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
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
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
Summary:
Removes the -khtml, -moz and -ms prefix, since most Browsers are natively supporting the user select directive.
The -webkit prefix is still kept or added for Safari, wich does not support user-select.
Ref: see https://we.phorge.it/D25024#815 for context
Test Plan: Removing the CSS should change nothing in modern browsers.
Reviewers: O1 Blessed Committers, speck, valerio.bozzolan, Matthew
Reviewed By: O1 Blessed Committers, speck, valerio.bozzolan
Subscribers: Cigaryno, Matthew, speck, tobiaswiese, valerio.bozzolan
Differential Revision: https://we.phorge.it/D25025
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. So, we adopt an explicit cast to string
in order to answer the question "Is this a non-empty string?" without relying on implicit
cast and deprecated things.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_string_cast() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
Closes T15409
Test Plan:
Applied this change and afterwards, selecting "Working Copy" on `/drydock/blueprint/edit/form/default/` does not show a RuntimeException on strlen() anymore. Instead, it shows a PhutilAggregateException covered in T15408.
Try also creating an integer "Custom Field" in some applications like Maniphest (maniphest.custom-field-definitions)
or in the People component (user.custom-field-definitions) etc., populating the related forms with your usual fuzzy tests.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15409
Differential Revision: https://we.phorge.it/D25235
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.
Similarly, `preg_match()` does not accept passing `null` as `$subject` parameter since PHP 8.1.
Closes T15381
Test Plan: Applied these two changes (on top of D25179 and D25213), and on the task creation page, after clicking the magnifier icon in the "Subscribers" field, the "Browse Subscribers" overlay dialog got rendered.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15381
Differential Revision: https://we.phorge.it/D25214
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 T15348
Test Plan:
Applied this change (on top of D25157) and creation form at `/herald/edit/?content_type=HeraldPreCommitRefAdapter&rule_type=personal` rendered in web browser.
Same applies for other types of personal and global Herald rules.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15348
Differential Revision: https://we.phorge.it/D25190
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 second half of T15331
Test Plan: Applied these two changes (one in Arcanist, one in Phorge). Project with empty Description field was created and `/project/view/projectid/` rendered in web browser.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15331
Differential Revision: https://we.phorge.it/D25177
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 T15319
Test Plan:
Applied this change (on top of D25147, D25153, D25167) and
`/people/editprofile/userid/` finally rendered in web browser.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15319
Differential Revision: https://we.phorge.it/D25168
Summary:
See Q59. Fixes rP935d7120ee32.
Call sites should be happy to use PhutilURI when possible.
Test Plan: visit any Repository page. Before - exception, now - data.
Reviewers: O1 Blessed Committers!, valerio.bozzolan
Reviewed By: valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25139
Summary:
Accessing a project's workboard URL of a non-existing workboard shows RunTimeException in PHP 8.1:
preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated
Closes T15262
Test Plan: Page whether to create a workboard for a project was displayed after this change
Reviewers: O1 Blessed Committers, avivey, valerio.bozzolan
Reviewed By: O1 Blessed Committers, avivey, valerio.bozzolan
Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15262
Differential Revision: https://we.phorge.it/D25131
Summary:
This change avoids some unnecessary uses of the strlen() function,
actually fixing some deprecation warnings in PHP 8.2.
In short, this is the suggested universal replace:
-if(strlen($v))
+if(phutil_nonempty_string($v))
And, if you know PHP, this is also another adoptable replace, but
only for cases where you are sure that the string "0" is not useful:
-if(strlen($v))
+if($v))
As usual the optimal solution depends on the contest.
Other similar patches will probably follow.
Closes T15222
Ref T15190
Test Plan:
- for the first time in my life, with this change, the unit tests are passed in PHP 8.2
- check with your big eyes that there are no obvious typos
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15199, T15190, T15222
Differential Revision: https://we.phorge.it/D25104
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. 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. 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 T13588. This allows "arc unit --everything" to more or less run cleanly on PHP 8.1.
Test Plan: Ran "arc unit --everything".
Maniphest Tasks: T13588
Differential Revision: https://secure.phabricator.com/D21821
Summary: Ref T13680. Conpherence may pass values with an integer type to this layer of the stack. These are "supposed" to be strings, but just be accepting.
Test Plan:
- Wrote 100+ messages to a Conpherence room.
- Clicked "Show Older Messages".
- Before: exception, int passed to "phutil_nonempty_string()".
- After: older messages loaded.
Maniphest Tasks: T13680
Differential Revision: https://secure.phabricator.com/D21824
Summary: Ref T13678. When remarkup content embeds other remarkup content, detect and degrade if the references have nesting depth greater than 1. This is a coarse cycle detector, since rendering shallow (but technically non-cycling) trees doesn't seem valuable.
Test Plan: Created various objects with self-references, saw everything degrade properly (after one level of embedding) when embedded in itself and in other contexts. See attached screenshot.
Maniphest Tasks: T13678
Differential Revision: https://secure.phabricator.com/D21810
Summary: Ref T13676. Ref T13588. Fix some issues that prevent "bin/phd" and "bin/drydock" from executing under PHP 8.1, broadly because `null` is being passed to `strlen()`.
Test Plan: Ran `bin/phd debug task` and `bin/drydock ...` under PHP 8.1.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13676, T13588
Differential Revision: https://secure.phabricator.com/D21795
Summary:
Ref T5479. Ref T13658. This was a contributed application from the early days of Phabricator which never had customers or users in the wild. The contributor moved on from the project many years ago.
Any capabilities in this general role would look different today. It also has one or two product name literal strings, so this is as good a time as any to remove it.
This change does not remove storage; I'll issue upgrade guidance and do that separately after some time.
Test Plan: Grepped for "phragment", got no relevant hits.
Subscribers: hach-que, PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13658, T5479
Differential Revision: https://secure.phabricator.com/D21793
Summary:
Ref T9530. Ref T13658. The "Releeph" application was never useful outside of Facebook and any application providing release support would not resemble it much.
It has some product name literal strings, so now is as good a time as any to get rid of it.
This application never left prototype and I'm not aware of any install in the wild that uses it (or has ever used it).
I did not destroy the database itself. I'll issue upgrade guidance and destroy the database in some future release, just in case.
Test Plan: Grepped for "releeph", found no relevant/removable hits.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13658, T9530
Differential Revision: https://secure.phabricator.com/D21792
Summary: Ref T13658.
Test Plan:
This test plan is non-exhaustive.
- Ran `bin/storage databases`.
- Viewed Badges UI exmaples page.
- Used eval rule for `strings.platform.server.name`, got "Phabricator".
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21773
Summary: Ref T13658.
Test Plan:
This test plan is non-exhaustive.
- Ran `bin/mail`.
- Uninstalled and reinstalled an application.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21770
Summary:
Ref T13667. The Herald "content added" rule (and other similar rules) do not correctly extract content from empty commits.
When we load an empty raw diff, return an empty changed content map.
Ref T13588. Also fix some PHP8.1 null/string stuff
Test Plan:
- Ran "bin/repository reparse --publish <commit>", with an empty commit hash and a nonempty commit hash.
- Reviewed Herald transcripts for general sanity.
Maniphest Tasks: T13667, T13588
Differential Revision: https://secure.phabricator.com/D21761
Summary:
Ref T13667. When a revision's diff has no changesets (usually because Diffusion performed an automatic update with an empty commit), the UI currently tries to "min()" an empty array and fatals.
Handle this case properly.
Test Plan:
- Created a revision with a diff with no changesets ("git commit --allow-empty" + copy-paste into web UI).
- Viewed revision.
- Before: "min()" fatal.
- After: UI isn't perfect, but works without fataling.
Subscribers: cspeckmim
Maniphest Tasks: T13667
Differential Revision: https://secure.phabricator.com/D21760
Summary: Ref T13671. Allow "bin/storage dump" to dump a subset of databases, primarily to support merging previously-partitioned databases.
Test Plan: Ran `bin/storage dump` with and without `--database ...` flags. Ran `--database invalid`, `--database a --database a` to hit error cases.
Maniphest Tasks: T13671
Differential Revision: https://secure.phabricator.com/D21745
Summary:
Ref T13588. This configuration value may not be set.
Also fix an issue in `bin/storage` and whatever else I hit between now and this diff actually uploading.
Also fix a MySQLi report mode difference, beginning in PHP 8.1.
Also update a bunch of "static" property usage in Lisk.
Test Plan: Ran `bin/files ...` locally under PHP 8.1.
Maniphest Tasks: T13588
Differential Revision: https://secure.phabricator.com/D21744
Summary: With this change, links to documentation will point to we.phorge.it rather then secure.phabricator,com
Test Plan: No testing needed - simple URL update
Reviewers: O1 Blessed Committers, deadalnix
Reviewed By: O1 Blessed Committers, deadalnix
Subscribers: taavi, speck, tobiaswiese
Differential Revision: https://we.phorge.it/D25014
Summary:
Ref T13658. This adds a simple expression evaluator to Remarkup and supports platform name expressions. The syntax is:
```
${{{strings.platform.server.name}}}
```
Note that this won't work inside code blocks (or literal blocks, or other block-level literal elements) right now, although it could be made to selectively (the ".path" expressions might be useful in documentation codeblocks).
Test Plan: {F9391006}
Reviewers: cspeckmim
Reviewed By: cspeckmim
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21713
Summary: Ref T13072. Make large Conduit doc pages a bit more navigable. This prepares for updating "harbormaster.sendmessage" to support sending messages to builds.
Test Plan: Viewed various Conduit API documentation pages, clicked links.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13072
Differential Revision: https://secure.phabricator.com/D21696
Summary: Ref T13559. Substantially correct the client logic for "Save" and "Cancel" actions to handle unusual cases.
Test Plan:
Quoting behavior:
- Quoted a comment.
- Cancelled the quoted comment without modifying anything.
- Reloaded page.
- Before changes: quoted comment still exists.
- After changes: quoted comment is deleted.
- Looked at comment count in header, saw consistent behavior (before: weird behavior).
Empty suggestion behavior:
- Created a new comment on a suggestable file.
- Clicked "Suggest Edit" to enable suggestions.
- Without making any text or suggestion changes, clicked "Save".
- Before changes: comment saves, but is empty.
- After changes: comment deletes itself without undo.
General behavior:
- Created and saved an empty comment (deletes itself).
- Created and saved a nonempty comment (saves as draft).
- Created and saved an empty comment with an edit suggestion (saves).
- Created and saved an empty comment with a suggestion to entirely delete lines -- that is, no suggestion text (saves).
- Edited a comment, saved without changes (save).
- Edited a comment, save deleting all text (saves -- note that this is intentionally without undo, since this is a lot of steps to do by accident).
- Cancel editing an unchanged comment (cancels without undo).
- Cancel editing a changed comment (cancels with undo).
- Undo'd, got text back.
- Cancel new comment with no text (deletes without undo).
- Cancel new comment with text (deletes with undo).
- Undo'd, got text back.
- Saved a quoted comment with no changes (saves -- note that this is intentionally not a "delete", since just quoting someone seems fine if you click "Save" -- maybe you want to come back to it later).
Maniphest Tasks: T13559
Differential Revision: https://secure.phabricator.com/D21654
Summary:
Ref T13559. Various client decisions depend on the "initial" or "committed" states of inline comments. Previously, these were informally constructed from "mostly similar" available values, or glossed over in some cases.
On the server, save the initial state when creating a comment. Save the committed state when applying a "save" operation. Send all three states to the client.
On the client, load and track all three states explicitly.
Test Plan: Created inlines, etc. See followups.
Maniphest Tasks: T13559
Differential Revision: https://secure.phabricator.com/D21653
Summary:
Ref T13559. Currently, the default text for inline comment side-loads in a bizarre way. Instead, when a user creates an inline comment, load the inline context and set it as part of the initial content state.
This allows the side channel (and the code that puts the text in place at the last second on the client) to be removed.
Test Plan: Created inlines, clicked "Suggest Edit". See followup changes.
Maniphest Tasks: T13559
Differential Revision: https://secure.phabricator.com/D21652
