Summary:
It seems that, in an attempt to make ICS URI(s) editable, we broke the ability
to manually import ICS files. Whops.
The cause is, the URI needs to be put inside its dedicated import engine,
and not the general one.
Since the intention of T15137 was to be able to edit this field, we have done
that in the right way this time. So, you see the field, not just in creation mode.
Thanks to the kind aklapper for reporting.
Ref T15137
Closes T15619
Rollback 02a4f8b0c8
Test Plan:
- visit /calendar/import/ and:
- create/edit an ICS File Import (now works again)
- create/edit an ICS URI Import (still work)
- try looking at an "ICS Import page" as author (URI still visible)
- try looking at an "ICS Import page" without Edit permissions (URI still omitted correctly)
Reviewers: aklapper, O1 Blessed Committers, 20after4
Reviewed By: aklapper, O1 Blessed Committers, 20after4
Subscribers: 20after4, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15619, T15137
Differential Revision: https://we.phorge.it/D25448
Summary:
Update PhabricatorAuthCSRFEngine.php such that it doesn't fall over when provided with a null CSRF token under PHP 8.1
Fixes T15654
Test Plan: Do a POST request to phorge.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15654
Differential Revision: https://we.phorge.it/D25449
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) Undefined index: above at [<arcanist>/src/error/PhutilErrorHandler.php:251]
arcanist(), phorge()
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<phorge>/src/applications/macro/engine/PhabricatorMemeEngine.php:276]
```
Closes T15637
Test Plan:
Create a meme called "angrycat" from the /macro/ page, and try a comment like this, expecting no nuclear implosion:
{meme, src=angrycat, below=}
{meme, src=angrycat, above=}
{meme, src=angrycat, below=, above=}
{meme, src=angrycat, below= , above= }
{meme, src=angrycat, below=asd}
{meme, src=angrycat, above=asd}
{meme, src=angrycat, above=asd, below=dsa}
{meme, src=angrycat, above= asd , below= dsa }
Also carefully read the code with your big eyes, keeping in mind that strlen does not accept passing `null` in PHP 8, and looking at what we did in rPb4cfe56f03b44615ac9251aed8d74bf13b085051.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15637
Differential Revision: https://we.phorge.it/D25437
Summary:
After this change the first Tab ID is always selected, so we don't risk
to have no tabs opened by default under some corner cases.
This is vaguely better than the original logic that relied on the fact that
(0=="0") is true. Besides being a vaguely weird emoticon as well, now that
I look at it better.
Original logic:
https://we.phorge.it/source/phorge/browse/master/src/applications/dashboard/paneltype/PhabricatorDashboardTabsPanelType.php;c43618a3a8bb021936fad687f04cb1a95faa23e4$289
This fixes a regression that appeared in the cute Wikimedia Phabricator
(that now is really Wikimedia Phorge). Their homepage should be gorgeous again,
without blank space due to unopened silly tabs.
Closes T15651
Test Plan:
- Have a Tab Panel (/dashboard/panel/).
- Be sure that the first Tab is automatically selected (again?)
Reviewers: aklapper, O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15651
Differential Revision: https://we.phorge.it/D25447
Summary:
Fix the following error you may encounter in production:
Too few arguments to function DiffusionDiffQueryConduitAPIMethod::getDefaultParser(), 0 passed in /var/www/phorge/src/applications/diffusion/conduit/DiffusionDiffQueryConduitAPIMethod.php on line 156 and exactly 1 expected
It was caused by the lack of a new mandatory parameter in a method.
Closes T15649
Test Plan: Check with your big eyes that a ConduitAPIRequest is passed to getDefaultParser().
Reviewers: avivey, O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15649
Differential Revision: https://we.phorge.it/D25444
Summary:
After changing the I/O type of a Diffusion repository URI, the transaction history incorrectly shows a message which talks about changing the URI's Display type instead.
Closes T15648
Test Plan: Change the I/O type of a Diffusion repo URI and look at that URIs history.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15648
Differential Revision: https://we.phorge.it/D25443
Summary:
After this change, a new input field "Milestone Name" appears in the "Edit" menu of a Milestone:
| Before | After |
|-----------|-----------|
| {F314008} | {F314005} |
So you can quickly change the name of your Milestones, from a Workboard.
Before this change, from a Workboard, this was the way to rename a Milestone:
1. click on the Milestone name (yes, that is a link)
2. click on Manage
3. click on Edit Details
4. rename
5. Save
6. Manually visit again the Project's Workboard
After this change, from a Workboard, you just need to:
1. click on Milestone > Edit
2. click on Edit Column
3. rename
4. Save
Example usage:
{F314015}
This does not change the level of permissions needed: if you have not enough
permissions to see or edit a Milestone, you cannot access this feature indeed.
In short, this is just a frontend change, keeping current policies as-is.
Closes T15143
Test Plan:
Create a Project or use an existing editable one.
Create a Milestone called "Test Milestone". You can create Milestones visiting the Project's menu {nav icon=sitemap,name=Subprojects > icon=plus,name=Create next milestone}
Visit the Project's Workboard. Find the column "Test Milestone". Click the Edit button on a Milestone, and:
- try to save another name: it must work
- try to save an empty name: nice error message shown
- try to save both the score points and the name: it must work
- try to save "FOO" as Points: you still see the error message
Also:
- do the same for the Backlog column: it still works (name still allowed to be empty)
- do the same for a "normal" Column (not the Backlog): it still work (name still __not__ allowed to be empty)
Reviewers: O1 Blessed Committers, Cigaryno, 20after4, waldyrious
Reviewed By: O1 Blessed Committers, Cigaryno, 20after4, waldyrious
Subscribers: waldyrious, brennen, aklapper, 20after4, speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15143
Differential Revision: https://we.phorge.it/D25066
Summary:
Trying to view a Jira auth provider from https://my.phorge.site/auth/ results in strlen(): Passing null to parameter #1 ($string) of type string is deprecated
This change fixes it
Fixes T15634
Test Plan:
Go to https://my.phorge.site/auth/ and have a Jira already setup.
Click on Jira auth provider
Confirm the page is displayed and no error is generated.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15634
Differential Revision: https://we.phorge.it/D25432
Summary:
Fix issue whereby clicking on a link in Diviner (eg https://my.phorge.site/diviner/find/?name=Differential_User_Guide&type=article&jump=1) results in a strlen(null) error under PHP 8.1
Fixes T15635
Test Plan:
* Go into the Diviner differential page /book/phorge/article/differential/
* Click on the link to Differential User Guide: Inline Comments
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15635
Differential Revision: https://we.phorge.it/D25433
Summary:
In the dashboard application (https://my.phorge.site/dashboard/), when creating panels, adding panels to tab panels, and viewing query panels, we get a variety of strlen(null) errors under PHP 8.1.
This fixes all the ones seen.
Fixes T15574
Test Plan: See T15574
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15574
Differential Revision: https://we.phorge.it/D25367
Summary:
This broadens the typeahead datasource search for repos. Before this patch a repository named "Alligator Simulator" would not be found with the search string "simu...". This is patched with the ferret engine search and indexing features.
See T15583
Test Plan: Create repositories with titles with 2 or more words. Search for these repos with the global typeahead search. The search term should begin with the second/third/n-th word of the repo title.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15583
Differential Revision: https://we.phorge.it/D25430
Summary:
If the user specifies a text encoding via the "View Options" dropdown, respect this choice.
Ref Q68.
Test Plan: Play with the Encoding button in the view
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tinloaf, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25360
Summary:
`PhutilErrorHandler` already defines "DEPRECATED" error class, but it's not used.
I plan to use it to report deprecation warnings.
With this change, they will be shown in the Dark Console and in the error logs
Ref T15554
This change on its own won't do anything, but is safe to land first.
Test Plan: With D25387, pages that have deprecation warnings are now shown, and the deprecation message is available in the dark console (and in log).
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15554
Differential Revision: https://we.phorge.it/D25386
Summary: And also fix a bug in the generator script.
Test Plan: Run `bin/diviner generate`, wait till the cows come home, see 3 pages about Javelin in the Diviner app.
Reviewers: O1 Blessed Committers, chris
Reviewed By: O1 Blessed Committers, chris
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25429
Summary:
Make sure that `$epochs[]` is never empty but contains `0` so `min($epochs)` will not complain.
Closes T15623
Test Plan: Carefully read the code (as I am unaware of reproduction steps).
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15623
Differential Revision: https://we.phorge.it/D25417
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=df6c315ace5f), phorge(head=importICSCalendar, ref.master=3cc5ee6a33df, ref.importICSCalendar=3bd396120123)
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/calendar/import/PhabricatorCalendarImportEngine.php:459]
```
```
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=df6c315ace5f), phorge(head=importICSCalendar, ref.master=3cc5ee6a33df, ref.importICSCalendar=3bd396120123)
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/calendar/import/PhabricatorCalendarImportEngine.php:450]
```
Closes T15620
Test Plan:
* Revert rP02a4f8b0c8f1279fc0040ad8077942fd8b0d948b not to run into T15619
* Try to import an ICS file via `/calendar/import/edit/`
* See that page `/calendar/import/4/` renders correctly in web browser and shows `Log Messages` and `Imported Events` as expected
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15620
Differential Revision: https://we.phorge.it/D25411
Summary:
This call is preventing users to browse Subversion repositories.
Using strlen() to check string validity 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.
Fix T15608
Test Plan:
- Sign in
- Open a Diffusion SVN repository
- You should see the repository instead of getting a Runtime Exception
Reviewers: O1 Blessed Committers, Sten, avivey
Reviewed By: O1 Blessed Committers, Sten, avivey
Subscribers: Sten, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15608
Differential Revision: https://we.phorge.it/D25398
Summary:
With PHP 8.1+ it was not possible to view a commit from subversion repositories.
Indeed, if the commit user and/or email is not properly defined, strlen(null) is called, causing a deprecation warning, elevated to exception.
Using strlen() to check string validity 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.
Fix T15609
Test Plan:
- Apply D25397 and D25398
- Sign in
- Open a diffusion SVN repository
- Open a commit without user name and or email
- You should not see the same Runtime Exception (unfortunately, there is another one as described in T15610)
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15609
Differential Revision: https://we.phorge.it/D25399
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=df6c315ace5f), phorge(head=master, ref.master=7cffe557ac24)
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/phriction/conduit/PhrictionCreateConduitAPIMethod.php:28]
```
Closes T15614
Test Plan: Create a Phriction document without slug via Conduit. Get an `EXCEPTION: (Exception) No such document.` instead of a `strlen()` exception.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15614
Differential Revision: https://we.phorge.it/D25406
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=df6c315ace5f), phorge(head=thisThis, ref.master=7cffe557ac24, ref.thisThis=529790613a86)
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/paste/conduit/PasteCreateConduitAPIMethod.php:46]
```
Closes T15613
Test Plan: Create a paste in the deprecated API paste.create without content via Conduit. Shows `error_code ERR-NO-PASTE` as expected, and no exception.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15613
Differential Revision: https://we.phorge.it/D25405
Summary:
This strlen() call was preventing a new Phorge instance to be deployed/configured.
Indeed, on a fresh instance, configuration's "base-uri" key may not be defined witch lead to a Runtime Exception.
Using strlen() to check string validity 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.
Fix T15605
Test Plan:
- Checkout a fresh Phorge local copy from official 'https://we.phorge.it/source/phorge.git'
- Install/Configure local webserver/database
- Open http://phorge.domain in you browser
- Configure Phorge database (as requested by webpage)
- Create Phorge database (as requested by webpage)
- You should be able to reach administrator account page instead of getting a RuntimeException
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15605
Differential Revision: https://we.phorge.it/D25394
Summary: This commit harmonizes PhabricatorNotificationServerRef's getURI and getWebsocketURI methods as discussed in D25382.
Test Plan: This is hard to say since I didn't know how/when this function might be called.
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/D25401
Summary:
These calls were preventing notification servers configuration to be properly initialized.
Indeed, PHP 8.X is stricter concerning
This behavior is deprecated since PHP 8.1. Phorge adopts phutil_nonempty_string() as a replacement.
Fix T15598
Test Plan:
Sign in as an administrator, configure the notification server without filling admin path field,
you shouldn't get both an RuntimeException and a warning indicating that Phorge is unable to connect
to Notification Server but a message indicating that everything is fine.
Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey
Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey
Subscribers: avivey, Cigaryno, Matthew, valerio.bozzolan, tobiaswiese, speck
Maniphest Tasks: T15598
Differential Revision: https://we.phorge.it/D25382
Summary:
These calls are preventing users to browse subversion/mercurial repositories in PHP 8.1+.
Indeed, a similar bug affecting git repositories was already addressed in another commit
(rP6b8ec50148909938850b5acfd11725ae23a8e31b). This commit harmonize both DiffusionSvnRequest
and DiffusionMercurialRequest with DiffusionGitRequest
Fix T15607
Test Plan:
- Sign in
- Open a diffusion SVN/Mercurial repository
- You should not get a RuntimeException concerning this preg_match call
Reviewers: O1 Blessed Committers, Sten, valerio.bozzolan
Reviewed By: O1 Blessed Committers, Sten, valerio.bozzolan
Subscribers: Sten, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15607
Differential Revision: https://we.phorge.it/D25397
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(), phorge()
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/applications/auth/controller/PhabricatorAuthSetExternalController.php:43]
```
Closes T15606
Test Plan: Follow steps in T15606; page `/auth/external/` displays as expected the content: "Link External Account - You can link your Phabricator account to an external account to allow you to log in more easily in the future. To continue, choose an account to link below. If you prefer not to link your account, you can skip this step." and the "Link External Account" button.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15606
Differential Revision: https://we.phorge.it/D25395
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=df6c315ace5f), phorge(head=master, ref.master=7040bd525764)
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/maniphest/query/ManiphestTaskQuery.php:1039]
```
Closes T15604
Test Plan: Have more than 100 tasks, run a broad search with more than 100 results, try to go to next page of results.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15604
Differential Revision: https://we.phorge.it/D25392
Summary:
The DifferentialChangeset getOldStatePathVector() method assumes oldFile and filename are set.
This worked under PHP <= 8.0, but fails for PHP >= 8.1 with error messsage
```
strlen(): Passing null to parameter #1 ($string) of type string is deprecated
```
Fixes T15517
Test Plan:
Create a diff in which a new file is added.
This file will have oldFile NULL and filename a string.
View the diff https://my.phorge.site/D1234
Reviewers: O1 Blessed Committers, Matthew
Reviewed By: O1 Blessed Committers, Matthew
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15517
Differential Revision: https://we.phorge.it/D25323
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(), ava(), phorge(), wmf-ext-misc()
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/applications/auth/provider/PhabricatorOAuth1AuthProvider.php:70]
```
```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(), ava(), phorge(), wmf-ext-misc()
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/applications/auth/view/PhabricatorAuthAccountView.php:32]
```
Closes T15590
Test Plan:
* As an admin, set up custom "MediaWiki" OAuth provider from from https://gitlab.wikimedia.org/-/ide/project/repos/phabricator/extensions/edit/wmf/stable/-/src/oauth/
* As an admin, apply D25373
* As a user, go to `/auth/login/mediawiki:whatever/`
* Select login button
* Allow authentication on third-party site
* Get redirected to Phorge instance
Phorge user account registration page "Create a New Account" at `/auth/register/abcdefghijklmnopqrstuvwxyz0123456/` now renders as expected, instead of displaying errors only.
Reviewers: O1 Blessed Committers, Matthew
Reviewed By: O1 Blessed Committers, Matthew
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15590
Differential Revision: https://we.phorge.it/D25375
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/applications/harbormaster/controller/HarbormasterStepEditController.php:142]
```
Closes T15592
Test Plan: Add a build step for an existing Harbormaster Build Plan, see that page `New Step: Drydock: Run Command` is correctly rendered at `/harbormaster/step/new/1/HarbormasterDrydockCommandBuildStepImplementation/`
Reviewers: O1 Blessed Committers, Matthew
Reviewed By: O1 Blessed Committers, Matthew
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15592
Differential Revision: https://we.phorge.it/D25377
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/applications/harbormaster/editor/HarbormasterBuildPlanEditEngine.php:106]
```
Closes T15591
Test Plan: Visit `/harbormaster/plan/edit/form/default/`. "Create Build Plan" page renders as expected.
Reviewers: O1 Blessed Committers, Matthew
Reviewed By: O1 Blessed Committers, Matthew
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15591
Differential Revision: https://we.phorge.it/D25376
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.
This change also replaces two other `strlen()` calls, only based on //reading// the code but not based on triggering exceptions.
```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(), ava(), phorge(), wmf-ext-misc()
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/applications/auth/adapter/PhutilOAuth1AuthAdapter.php:121]
```
```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(), ava(), phorge(), wmf-ext-misc()
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/applications/auth/adapter/PhutilOAuth1AuthAdapter.php:125]
```
Closes T15588
Test Plan: Go to `/auth/login/mediawiki:whatever/` and at least see no `strlen()` errors anymore (while further issues remain). See T15588 for details.
Reviewers: O1 Blessed Committers, Matthew
Reviewed By: O1 Blessed Committers, Matthew
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15588
Differential Revision: https://we.phorge.it/D25373
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=788098096e11), phorge(head=master, ref.master=840a7fab2bc8)
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/passphrase/view/PassphraseCredentialControl.php:53]
```
Closes T15580
Test Plan: Set URI for a Diffusion Repository, select URI and select "Set Credential". Page "Edit Repository URI" at `/diffusion/123/uri/edit/456/` renders as expected in web browser.
Reviewers: O1 Blessed Committers, Matthew
Reviewed By: O1 Blessed Committers, Matthew
Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15580
Differential Revision: https://we.phorge.it/D25370
Summary:
Fix DifferentialDiff getFieldValuesForConduit PHP 8.1 strlen(null) errors by replacing the strlen() calls with phutil_nonempty_string().
Also needed to update DifferentialDiffTestCase.php to being a PhabricatorTestCase, as it was throwing an exception prior to any code changes -
```
EXCEPTION (Exception): Trying to read configuration "policy.locked" before configuration has been initialized.
```
Fixes T15529
Test Plan: arc diff
Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey
Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey
Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15529
Differential Revision: https://we.phorge.it/D25333
Summary:
arc diff throws strlen(null) error from DifferentialCommitMessageField renderFieldValue when calling a Phorge server running PHP 8.1
Add unit test, which required a new DifferentialTestCommitMessageField class so as to be able to test the abstract DifferentialCommitMessageField class methods.
Fixes T15530
Test Plan:
Make a change in a git repo with remote a Phorge server running PHP 8.1
Run:
```
arc diff
```
See exception thrown as per T15530
Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey
Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15530
Differential Revision: https://we.phorge.it/D25334
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.
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261];
PHP message: arcanist(head=master, ref.master=6e4947b55f09), phorge(head=master, ref.master=7bebfa289aa1);
PHP message: #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/search/controller/PhabricatorSearchController.php:16];
Fix T15595
Test Plan: Search something using the main search bar. It should return something (or not) instead to throwing a RuntimeException.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15595
Differential Revision: https://we.phorge.it/D25380
Summary:
This call was preventing notification servers configuration to be properly initialized.
Indeed, 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.
Fix T15596
Test Plan:
Sign in as an administrator, configure the notification server without filling admin path field,
you shouldn't get an invalid configuration error
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15596
Differential Revision: https://we.phorge.it/D25381
Summary:
Allow easier discovery and subscribing to Atom feeds of Phame blogs.
To be extra cautious, also make sure that the added line is a `PhutilSafeHTML`.
Original author: @20after4
Closes T15550
Test Plan: Go to `/phame/blog/view/1/` and check the HTML source code. See an additional `<link>` item with `type="application/atom+xml"` in the `<head>` section.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, 20after4, Cigaryno
Maniphest Tasks: T15550
Differential Revision: https://we.phorge.it/D25346
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=788098096e11), phorge(head=master, ref.master=840a7fab2bc8)
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/auth/provider/PhabricatorLDAPAuthProvider.php:430]
```
Closes T15587
Test Plan: Go to `/auth/config/edit/?provider=PhabricatorLDAPAuthProvider` and see that "Add Auth Provider: LDAP" page is rendered.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15587
Differential Revision: https://we.phorge.it/D25372
Summary:
In edit mode, Archived Panels are shown differently,
with content text in grey and with the usual red tag.
In view mode, Archived Panels are just not rendered.
This makes it easier to design Dashboards with individual
components that can be de-activated and re-activated on the fly.
Closes T15366.
{F325637}
Test Plan:
Have a Dashboard somewhere (e.g in your homepage).
Visit a Panel (W-ID) and Archive it.
See that now the Panel is not shown anymore on the Dashboard.
Edit the Dashboard and you see the Panel again as "Archived".
Anything around is just normal.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15366
Differential Revision: https://we.phorge.it/D25368
Summary:
The current hovercard of a wiki document has no further information except the title. This commit adds object type, project tags, parent documents, last author and last edited time to the card.
Preview:
{F313614}
Preview in a pessimistic case:
{F325478}
Closes T15433
Test Plan: Edit a wiki document with/without project tags and parent documents and see the hovercard in the feed.
Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey, Cigaryno
Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey, Cigaryno
Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15433
Differential Revision: https://we.phorge.it/D25303
Summary:
Fix arc patch PHP 8.1 strlen(null) error.
When doing an 'arc patch Dxxxx' against a diff on a PHP 8.1 Phorge server we get a strlen(null) error.
Updating the idx call to provide a default empty string resolves it.
Providing a default empty string to
Fixes T15571
Test Plan: arc patch Dxxxx
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15571
Differential Revision: https://we.phorge.it/D25364
Summary:
Doing a git push into a fresh repository over https fails with a PHP 8.1 strlen(null) error.
This fixes it.
Fixes T15569
Test Plan:
* Create an empty repository on a PHP 8.1 Phorge instance
* Clone it
* Add some data
* git push
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15569
Differential Revision: https://we.phorge.it/D25361
Summary:
The rationale for this change is that reloading a page unnecessarily wastes
resources that we can save.
This includes human resources (time) but also computational resources
as well. We probably save a fawn for every 1,000 clicks on that button.
A message to Greta Thunberg: now Phorge is with you.
This is the involved "Cancel" button, that does not reload the page anymore:
{F309894}
Closes T15471
Test Plan:
Workboard > Column > Edit > Cancel:
The popup closes gracefully instead of a page refresh.
Also try opening the workflow in another tab (/project/board/1/hide/2/) > Cancel:
You turn back to your Workboard just as before.
Reviewers: O1 Blessed Committers, Matthew
Reviewed By: O1 Blessed Committers, Matthew
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15471
Differential Revision: https://we.phorge.it/D25302
Summary:
Fix "Update Patch" for generic Differential revisions.
You can now help a coworker to update their patch, from the web.
So, if you visit Diff 111 (not yours) and you click "Update Diff", and you paste a valid diff,
you arrive here:
https://we.phorge.it/differential/diff/222/?revisionID=111
That page as default was suggesting some Diffs that are yours. Plus, now it shows Diff 111.
Omitting the Diff from which the workflow was started made no sense and it's certainly an oversight.
Any follow-up change is welcome to modernize the selector using AphrontFormTokenizerControl.
Closes T15538
Test Plan:
- create a Diff on Differential with user A
- click on Update Diff from user B (pasting a valid diff) and Continue
Now that patch is selected (and selectable). No dead-end anymore.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15538
Differential Revision: https://we.phorge.it/D25337
Summary:
If you manually upload a Diff (patch) from the Differential web
interface you can select the Repository.
We fix the default value of the Repository field during creation mode,
guessing the repo from the potentially already-available diff.
Closes T15537
Test Plan:
- Create a cute Differential Revision from web
- Pick a Repository > Continue
- Create a new Revision > Continue
At this point you should still have that repository instead of nothing.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15537
Differential Revision: https://we.phorge.it/D25338
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.
Use `isset` instead to check if the argument "start" was passed to the CLI command.
```
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=repoMaintStartHistory, ref.master=1c098c273d06, ref.repoMaintStartHistory=0a4a34143528)
#0 PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/management/PhabricatorRepositoryManagementMaintenanceWorkflow.php:45]
```
Closes T15476
Test Plan: After applying this change, executing `./bin/repository maintenance --stop R1` on the CLI shows `Took repository "R1" out of maintenance mode.` as expected, instead of an exception.
Reviewers: O1 Blessed Committers, valerio.bozzolan, aklapper, avivey
Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey
Subscribers: Sten, avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15476
Differential Revision: https://we.phorge.it/D25296
Summary:
Fix PHP 8.1 strlen(null) error in PhabricatorFile::newChunkedFile().
Fixes T15499
Test Plan:
Added a unit test which replicated the fault, so you can test simply by 'arc unit'
Alternatively, see test in T15499
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15499
Differential Revision: https://we.phorge.it/D25352
Summary:
When someone tries to log on with invalid credentials, a record is inserted in the phabricator_user.user_log table with action = login-fail.
The session value of this record is set to null in this case.
When you open the People → Activity Logs screen, you will see an exception because of this.
Fixes T15555
Test Plan: Go to https://my.phorge.site/people/logs/ after a login failure and a login success.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15555
Differential Revision: https://we.phorge.it/D25351
Summary:
When viewing the top level of a diffusion repository, $request->getPath() returns null, which then causes trim() to throw an error.
Fix is to update DiffusionRequest getPath() such that it returns '' instead of null, thus fixing it's return type.
Fixes T15522
Test Plan: Go to https://my.phorge.site/source/myrepo
Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey
Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15522
Differential Revision: https://we.phorge.it/D25329
Summary:
DifferentialChangesetViewController loadCoverage() fails under PHP 8.1 when test coverage is provided for some but not all modified files.
Update the code to replace strlen() with phutil_nonempty_string().
Fixes T15521
Test Plan: View a diff with test coverage for some but not all files.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15521
Differential Revision: https://we.phorge.it/D25328
Summary:
/dashboard/adjust/remove/ POST requests fail under PHP 8.1 with strlen(null) errors.
This change fixes it.
Fixes T15549
Test Plan:
View a dashboard which has some panels. Eg https://my.phorge.site/dashboard/view/1/
Click on a 'Manage Panel' dropdown and click on 'Remove Panel'
Confirm the panel is removed and no strlen(null) errors are generated.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15549
Differential Revision: https://we.phorge.it/D25345
Summary:
Fix PHP 8.1 strlen(null) in DiffusionView linkBrowse().
Fixes T15542
Test Plan:
# Go into a diff (eg https://my.phorge.site/D1234)
# Click on the Revision Contents - History tab
# Click on a base link (the short git revision ID)
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15542
Differential Revision: https://we.phorge.it/D25340
Summary:
This change implements some messages that were only present
from this class:
DifferentialRevisionHasCommitEdgeType.php
But not in its inverse class:
DiffusionCommitHasRevisionEdgeType.php
In short:
| Before | After |
|-----------|-----------|
| {F313282} | {F313281} |
Closes T15314
Test Plan:
Visit a Diffusion commit and use the sidebar menu "Edit Related Objects" to:
- add 1 revision
- add 2 revisions
- remove 1 revision
- remove 2 revisions
- add 1 revision, remove 1 revision
- add 2 revisions, remove 2 revisions
Check that you don't see phrases like "added an edge" anymore.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15314
Differential Revision: https://we.phorge.it/D25306
Summary:
It seems that if you use the "Move Right" action, you can get
an unrelated error message about "Move Left":
This is already the first tab. It can not move any farther to the left.
This happened because there was an incomplete condition.
Closes T15493
Test Plan:
- visit /dashboard/panel/ and create at least two panels (please, owned by you)
- visit a Dashboard, and add a Tabs Panel
- take the first Tab: "Move Right"
After this change, "Move Right" now always works.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15493
Differential Revision: https://we.phorge.it/D25307
Summary:
If you happen to create a Dashboard by retrieving other people's panels,
you were probably affected by this visual bug.
It seems that you cannot Move things, but you really can (and that is OK).
The reason is, to move things in a Parent Panel, you only need edit permission there,
and you do not need to also have edit permissions on every Child Panel.
Closes T15492
Test Plan:
- From user A:
- Create two panels - editable by: User A
- From user B:
- Create a "Tabs Panel" in a Dashboard
- Attach the panels from User A (so, editable by, not you)
The meaningful "Move" actions should be not disabled anymore.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15492
Differential Revision: https://we.phorge.it/D25308
Summary:
When iterating through the fields of a differential commit, the DifferentialGetCommitMessageConduitAPIMethod execute method explicitly allows a value to be either a string or a null. It then calls strlen upon this possibly null value.
We could replace the strlen with phutil_nonempty_string, but as the code has already eliminated variable types other than string or null, it is more efficient to explicitly check for null or ''
```
$value === null or $value == ''
```
Fixes T15527
Test Plan:
Run
```
arc diff
```
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15527
Differential Revision: https://we.phorge.it/D25332
Summary: Fixes T15275.
Test Plan:
Project pages have "Developer > View Hovercard" button. Or hover over a project handle, I guess.
Also test for empty description and with the description field disabled.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15275
Differential Revision: https://we.phorge.it/D25331
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=1c098c273d06)
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/xaction/PhabricatorRepositoryMaintenanceTransaction.php:28]
```
Closes T15475
Test Plan:
Play a bit with maintenance messages:
./bin/repository maintenance --start "Start" R1
./bin/repository maintenance --start "Start Yeah" R1
./bin/repository maintenance --stop R1
Then visit `/diffusion/1/manage/history/` and look at `Diffusion put this repository into maintenance mode.` as expected, instead of an exception.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15475
Differential Revision: https://we.phorge.it/D25295
Summary:
The error was caused by the overlay lightbox, since it gives the possibility to share a comment on any file
in any general Remarkup text, but that comment textarea is indeed empty as default.
`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=T15498, ref.master=a25af8ccef69, ref.T15498=a25af8ccef69)
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/files/controller/PhabricatorFileLightboxController.php:23]
```
Closes T15498
Test Plan: Visit a blog post with a file/image dropped in the content and click on the thumbnail. No error overlay dialog is displayed anymore.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15498
Differential Revision: https://we.phorge.it/D25310
Summary:
Fix PHP 8.1 strlen(null) error in DifferentialBranchField getBranchDescription()
Fixes T15531
Test Plan:
Simply viewing a diff (eg https://my.phorge.site/D1234) on a PHP 8.1 server generated the error fixed by this diff.
Also created a unit test.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15531
Differential Revision: https://we.phorge.it/D25335
Summary:
When a 'git pull' is done to an https git URL, the $_SERVER variables PHP_AUTH_USER and PHP_AUTH_PW will be unset, causing PHP 8.1 to throw strlen(null) errors.
This update fixes the issue by defaulting the values to '', which results in $have_user and $have_pass having false values as desired.
Fixes T15520
Test Plan: arc unit
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15520
Differential Revision: https://we.phorge.it/D25327
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: 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
