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

17697 commits

Author SHA1 Message Date
Andre Klapper
dbcc447249 Fix error in Mercurial when no offset is specified
Summary:
When viewing the history of a Mercurial diffusion repository the server return a 500. This is
because a function call to array_slice requires the offset to be set to an integer. So when it is
not specified it can be set to 0 as a default.

The error is as follows:

```
ERROR 8192: array_slice(): Passing null to parameter #2 ($offset) of type int is deprecated at [/usr/local/www/phorge/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php:167]; PHP message: arcanist(head=master, ref.master=3cb117684f4e), phorge(head=master, ref.master=4bf5c452eb28); PHP message:   #0 array_slice(array, NULL) called at [<phorge>/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php:167]
```

Test Plan: The history tab should not return a 500 when not specifying an offset

Reviewers: O1 Blessed Committers, valerio.bozzolan, speck

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

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

Differential Revision: https://we.phorge.it/D25673
2024-12-11 12:02:14 +01:00
Valerio Bozzolan
7429da91d2 Repository Identity "Automatically Detected User": don't trust unverified emails
Summary:
Make sure that Repository Diffusion Identities "Automatically Detected User " are not created from unverified emails.

Closes T15965

Test Plan:
Find at least one identity that is assigned to nobody:

http://phorge.localhost/diffusion/identity/

(For example, you may easily find an identity of "GitHub <noreply@github.com>")

(Double check that its "Assigned To" is unset or make sure it's unset for this test)

Be evil: add *that* email in your {nav Profile > Settings > Email addresses}. So, for example add "noreply@github.com", like a rogue. The email can stay unverified.

Run this command to immediately cause an effect:

    ./bin/repository rebuild-identities --all-identities

- before this change, you can reproduce that you successfully steal that identity and you become "GitHub" or whoever
- after this change, you see that "Automatically Detected User" is unset again
- after this change, any other identity manually assigned, is still assigned to that value
- after this change, any other identity automatically assigned to verified emails, are still "Automatically Detected User"

Reviewers: O1 Blessed Committers, speck, 20after4

Reviewed By: O1 Blessed Committers, speck, 20after4

Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15965

Differential Revision: https://we.phorge.it/D25845
2024-12-11 09:31:14 +01:00
Valerio Bozzolan
14fcf61a1e Fix Diffusion commands in non-English environments
Summary:
Closes T15966

Force Diffusion commands to be executed in the "LC_ALL=C" language mode, that is, English,
defusing whatever custom language you have in your LANGUAGE variable.

Related reading:

https://we.phorge.it/T15872

Test Plan:
Make sure Subversion is installed on your system. Explicitly set `LC_ALL`.
Finally, this unit test works, even if you have something esoteric in your LANGUAGE:

    arc unit ./src/applications/repository/worker/__tests__/PhabricatorChangeParserTestCase.php

Before, it was crashing, like mentioned in T15966.

Reviewers: O1 Blessed Committers, aklapper

Reviewed By: O1 Blessed Committers, aklapper

Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15966

Differential Revision: https://we.phorge.it/D25846
2024-12-11 08:33:04 +01:00
Pppery
c4f88847ef Fix typos in Harbormaster
Summary: See https://phabricator.wikimedia.org/T381897 and https://phabricator.wikimedia.org/T381896

Test Plan: Read the code

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

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

Differential Revision: https://we.phorge.it/D25852
2024-12-10 23:05:08 -05:00
Merula Turdus
9448e81572 Show table of contents by default on wide screens
Summary:
Show table of content in left whitespace area in wide screens (width >= 1792px)

Closes T15920

Test Plan:
1) Open Phriction wiki page in browser which contain a number of chapter titles
2) Open Responsive Design Mode in browser (Ctrl-Shift-M in Firefox) and test different width's of the page (e.g. 1024, 1280, 1440, 1600, 1920, ...)
3) Open Remarkup Reference at `/book/phorge/article/remarkup/` and repeat step 2
4) Open a Differential revision to confirm that the unrelated ToC panel has no changes

Reviewers: O1 Blessed Committers, aklapper

Reviewed By: O1 Blessed Committers, aklapper

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

Maniphest Tasks: T15920

Differential Revision: https://we.phorge.it/D25790
2024-12-09 18:33:33 +01:00
Andre Klapper
a5384ca604 Hovercards: Avoid "Undefined index: objectPHID" when passing bogus data
Summary:
Do not throw an exception when passing bogus parameters to hovercards but properly handle the missing array index by setting `$object_phid` to `null`.
In consequence, see a proper hovercard correctly displaying `Unknown Object (????)` instead of an exception.

Closes T15969

Test Plan:
* Go to http://phorge.localhost/search/hovercard/?cards={"whatever":{"whatever":"foobar"}} or http://phorge.localhost/?__path__=%2fsearch%2fhovercard%2f&cards={%22whatever%22:{%22whatever%22:%22foobar%22}} and see a nice `Unknown Object (????)` instead of an ugly exception
* Go to http://phorge.localhost/search/hovercard/?cards={%22PHID-TASK-12345678%2F%22%3A{%22objectPHID%22%3A%22PHID-TASK-12345678%22%2C%22contextPHID%22%3Anull}} (make sure to use a valid task PHID) and still see a correct hovercard rendered

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15969

Differential Revision: https://we.phorge.it/D25849
2024-12-09 12:00:03 +01:00
Taavi Väänänen
e5d7c9aa4e
docs: Remove extra 'B' from PHP config value
Summary:
Turns out that if you specify 32MB PHP interprets that as 32 bytes,
which is not what you want.

Test Plan:
Checked that file uploads start working on my Phorge instance after
removing the trailing 'B'.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25851
2024-12-08 14:21:02 +02:00
Valerio Bozzolan
9d3e258853 Add first unit test for mimemailparser headers
Summary:
Uploaded an example email with a lot of accents called 'test_accents.mbox' and
expected headers in the file 'test_accents.headers.txt'.

Better than nothing.

This change also includes a minor refactor in the library loading.

Ref T15960

Test Plan:
Manually run the new unit test and see green lights:

    arc unit src/applications/metamta/externals/__tests__/PhabricatorExternalMimeMailParserTestCase.php

Double-check that the new class is already recorded:

    arc liberate

Just as extra care, re-apply the same test plan of:

D25839

So, for example, run this, and see no exceptions:

    ./scripts/mail/mail_handler.php < src/applications/metamta/externals/__tests__/data/test_accents.mbox

Reviewers: aklapper, taavi, O1 Blessed Committers

Reviewed By: aklapper, O1 Blessed Committers

Subscribers: tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15960

Differential Revision: https://we.phorge.it/D25844
2024-12-06 13:12:54 +01:00
Pppery
9c73d62c44 Rewrite regex for project names to be not prone to catastrophic backtracking
Summary: Fixes T15371

Test Plan:
- Save the text `{{#translation:}}` in remarkup and see that it renders.
- Create a project or projects with the hashtags  `a`, `b`, `ab`, `foo`, `f.o.o`.
- Observe that both before and after this patch you can link to all of them by hashtag.
- Create a project or projects with the hashtags `a.`, `.b`, `.foo`, `foo.`.
- Observe that both before and after this patch you can link to none of them by hashtag.

Reviewers: O1 Blessed Committers, aklapper

Reviewed By: O1 Blessed Committers, aklapper

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

Maniphest Tasks: T15371

Differential Revision: https://we.phorge.it/D25838
2024-12-05 12:40:44 -05:00
Valerio Bozzolan
48fd3f1c40 Delete Repository button: disable if not admin, but show popup to all
Summary:
The "Delete Repository" button should be disabled, just like any other admin button in the page,
if you cannot edit the repository, so to do not cause anxiety situations to users without
editing permissions.

Moreover, probably, if you still want to click that disabled button,
you should still be allowed to see the super-nice related information message,
just because it's super-lovely, super-informative and it does nothing wrong.

Closes T15964

Test Plan:
Be normal user. See the button finally disabled. Click it, you finally see its related info popup instead "You shall not pass",
so you unlock extra documentation to read.

Be admin user. See the button still enabled. Click it, you still can see the info popup.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15964

Differential Revision: https://we.phorge.it/D25843
2024-12-04 07:51:55 +01:00
Andre Klapper
c589529155 Fix parsing of incoming mail with UTF-8 encoded headers
Summary:
rPa76444a8e238f647dc96f756e6c88aa2fafcdbfe updated our 13 year old copy of the mimemailparser library.
That included a behaviour change in the library not covered by Phorge code: The library now decodes MIME encoded UTF8 data in headers. Phorge passes that header to the `iconv_mime_decode()` PHP function which does not accept already encoded content.

```
EXCEPTION: (RuntimeException) iconv_mime_decode(): Detected an illegal character in input string at [<arcanist>/src/error/PhutilErrorHandler.php:273]
arcanist(head=master, ref.master=29ca3df1122b), phorge(head=master, ref.master=6ec5c88bee24)
  #0 PhutilErrorHandler::handleError(integer, string, string, integer)
  #1 iconv_mime_decode(string, integer, string) called at [<arcanist>/src/utils/utils.php:1759]
  #2 phutil_decode_mime_header(string) called at [<phorge>/scripts/mail/mail_handler.php:64]
```

Closes T15960

Test Plan: * Have an email file called `tmp.mbox` with a UTF-8 encoded `From:` header. In `scripts/mail/mail_handler.php`, replace `file_get_contents('php://stdin')` with `file_get_contents('./tmp.mbox')`. Insert `echo $headers['subject']; echo "\n"; echo $headers['from'];` statements for debugging. Run `php ./mail_handler.php`.

Reviewers: O1 Blessed Committers, taavi, valerio.bozzolan

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

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

Maniphest Tasks: T15960

Differential Revision: https://we.phorge.it/D25839
2024-12-03 13:39:15 +01:00
Andre Klapper
eb380f922c Account registration: Restrict Real Name length
Summary:
Avoid a database exception at user account registration when users enter very long real names by setting a maximum length.
This does not affect existing account data as it is only called in the account registration code.

Fixes T15962

Test Plan: Go to http://phorge.localhost/auth/register/ and enter long values into the "Real Name" field

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

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

Maniphest Tasks: T15962

Differential Revision: https://we.phorge.it/D25841
2024-12-02 20:36:39 +01:00
Andre Klapper
0bfdcaa005 Revert "Fix PHP 8.3 "Usage of ldap_connect with two arguments is deprecated" exception"
Summary:
Revert commit 700090917a per T15963.

Fixes T15963
Opens T15724

Test Plan: N/A.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15963, T15724

Differential Revision: https://we.phorge.it/D25842
2024-12-02 10:28:15 +01:00
Andre Klapper
a68b8daafd Avoid "Unknown Object (????)" for custom Select field values in Herald editor
Summary:
Custom Select field values are rendered as `Unknown Object (????)` when set as Herald rule conditions and editing the Herald rule.
This is unhelpful; at least display their key value.

Closes T15860

Test Plan: Create a `"type": "select"` custom field via `/config/edit/maniphest.custom-field-definitions/`, set up a Herald rule with a condition based on that custom field, edit the Herald rule.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15860

Differential Revision: https://we.phorge.it/D25696
2024-11-28 12:41:41 +01:00
Andre Klapper
89be7a51d8 Validate some user provided calendar query range dates
Summary:
Calendar search form allows users to define date ranges. Entering gibberish data leads to a cryptic exception due to calling `format()` on `null`, as `AphrontFormDateControlValue::getDateTime()` can return `null` instead of a `DateTime` object.

Also add some additional PhpDoc as a result of playing with this code.

Note that other calendar query forms are more lenient and still accepts gibberish after applying this patch. The intention behind this patch is replacing a cryptic exception with a more appropriate and descriptive error; this patch does not attempt to introduce validation everywhere.

```
EXCEPTION: (Error) Call to a member function format() on null at [<phorge>/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php:469]
```

Closes T15943

Test Plan:
* On http://phorge.localhost/calendar/query/month/, click "Edit Query", check "Occurs After", replace default date value with "abcde", click "Search" button
* On http://phorge.localhost/calendar/query/month/, click "Edit Query", check "Occurs After", replace default time value with "abcde", click "Search" button
* On http://phorge.localhost/calendar/query/month/, click "Edit Query", check "Occurs Before", replace default date value with "abcde", click "Search" button
* On http://phorge.localhost/calendar/query/month/, click "Edit Query", check "Occurs Before", replace default time value with "abcde", click "Search" button
* On http://phorge.localhost/calendar/query/day/, click "Edit Query", check "Occurs After", replace default date value with "abcde", click "Search" button
* On http://phorge.localhost/calendar/query/day/, click "Edit Query", check "Occurs After", replace default time value with "abcde", click "Search" button
* On http://phorge.localhost/calendar/query/day/, click "Edit Query", check "Occurs Before", replace default date value with "abcde", click "Search" button
* On http://phorge.localhost/calendar/query/day/, click "Edit Query", check "Occurs Before", replace default time value with "abcde", click "Search" button

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15943

Differential Revision: https://we.phorge.it/D25825
2024-11-26 15:12:22 +01:00
Andre Klapper
b253675917 mimemailparser: Fix implicitly nullable parameter declarations for PHP 8.4
Summary: Backport upstream's 39633e054b for PHP 8.4 compatibility.

Test Plan: Run static code analysis.

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/D25840
2024-11-26 15:02:10 +01:00
Andre Klapper
6ec5c88bee Make maniphest.gettasktransactions include metadata and title
Summary:
Include task title and metadata in Conduit's `maniphest.gettasktransactions` output.
This data can for example be used by IRC bots to improve their output (e.g. conditions which changes [not] to emit or text coloring based on metadata).

Upstreaming from https://phabricator.wikimedia.org/rPHABf2fd14dc1edeb41aa2874336548cfaa7fa0e87a0

NOTE: this specific API is in "frozen" status. This method is frozen and will eventually be deprecated. New code should use "transaction.search" instead.

Closes T15916

Test Plan: Call `maniphest.gettasktransactions` on a task, see two additional fields.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15916

Differential Revision: https://we.phorge.it/D25777
2024-11-26 14:08:48 +01:00
Andre Klapper
828ea9bca0 Allow zooming on pages
Summary:
Zooming on pages is currently disabled for reasons mentioned in rP953ab039acc0894e287304364920bc7cc4962cc0 from 2017.
This violates WCAG and is especially unhelpful on a mobile phone to see a larger version of an image file.
Thus allow zooming, crossing fingers that mobile platforms have reduced the unwanted side effects over the last seven years.

Closes T15945

Test Plan: Use Firefox on an Android phone in portrait orientation; Visit a Phorge task which has an image embedded; Click on the image comment; Pinch with fingers to zoom.

Reviewers: O1 Blessed Committers, 20after4

Reviewed By: O1 Blessed Committers, 20after4

Subscribers: 20after4, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15945

Differential Revision: https://we.phorge.it/D25827
2024-11-26 14:07:09 +01:00
Andre Klapper
700090917a Fix PHP 8.3 "Usage of ldap_connect with two arguments is deprecated" exception
Summary:
Per https://www.php.net/manual/en/function.ldap-connect.php the signature `$conn = @ldap_connect($host, $this->port);` is deprecated since PHP 8.3.
Thus pass a full LDAP URI as the only parameter.

```
ERROR 8192: Usage of ldap_connect with two arguments is deprecated at [/var/www/html/phorge/phorge/src/applications/auth/adapter/PhutilLDAPAuthAdapter.php:308]
```

Closes T15724

Test Plan: Set up LDAP as auth provider, try to log in.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

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

Maniphest Tasks: T15724

Differential Revision: https://we.phorge.it/D25792
2024-11-26 14:05:09 +01:00
Andre Klapper
c0ebc7b405 Fix a typo in documentation
Summary: Platypus vs Platyplus. This was reported downstream in https://phabricator.wikimedia.org/T380365

Test Plan: Read slowly.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

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

Differential Revision: https://we.phorge.it/D25837
2024-11-26 11:44:38 +01:00
BlankEclair
0ea95d41a6
Fix submitting forms in a new tab using Ctrl+Return
Summary:
This commit adds a keydown listener to <input> elements to activate a flag when
Ctrl (and other keys) are pressed, which causes forms to be submitted to a new
tab.

This commit also modifies the click event listener for buttons to ignore
synthetic clicks from the browser, which is important as they clobber the
"new_tab" flag otherwise.

Closes T15914

Test Plan:
Open the Advanced Search form, and do Ctrl+Return inside one of the text boxes
to ensure that the result is opened in a new tab. Also do a plain Return, plain
click on "Search", and Ctrl+Click on Search to check for regressions.

Reviewers: O1 Blessed Committers, valerio.bozzolan, aklapper

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

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

Maniphest Tasks: T15914

Differential Revision: https://we.phorge.it/D25773
2024-11-25 21:09:36 +11:00
Andre Klapper
a76444a8e2 Update mimemailparser from May 2011 version to 8.0.4
Summary:
Bump to version 8.0.4 from 2024-09-11 per https://github.com/php-mime-mail-parser/php-mime-mail-parser/releases before this ancient code copy falls apart.
`scripts/mail/mail_handler.php` (used for incoming (!) mail) is the only consumer.

Closes T15940

Test Plan: Feed `mail_handler.php` with various test emails (formats: plain text, HTML, multipart; encodings: UTF-8, ASCII, ISO-8859-something) by manually replacing `php://stdin` with corresponding text files and adding some `phlog`s for output checking as I don't have mail server glue handy. Get only expected errors for broken emails.

Reviewers: O1 Blessed Committers, 20after4

Reviewed By: O1 Blessed Committers, 20after4

Subscribers: 20after4, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15940

Differential Revision: https://we.phorge.it/D25829
2024-11-01 08:59:05 +01:00
mainframe98
ddf26e89c8 DarkConsole: Fix "strlen(null)" but for the "tab" query parameter
Summary:
See rPf4d2811

Bug: T15344

Test Plan: Opened DarkConsole with /~/, saw no errors in DarkConsole

Reviewers: O1 Blessed Committers, aklapper

Reviewed By: O1 Blessed Committers, aklapper

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25834
2024-10-28 09:26:20 +01:00
mainframe98
74bf6c0e3c APC: Set ttl to 0 when ttl is not given
Summary:
Passing null as a value for parameter $ttl is deprecated.
Omitting the parameter is considered to be equivalent to 0.

Bug: T15064

Test Plan: * Looked in the deprecation log and did not see a deprecation message from apc.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25833
2024-10-27 15:12:01 +01:00
Andre Klapper
d643ca4c4c ManiphestReportController: Separate legacy and synthetic data handling
Summary:
Due to code additions in rPcb957f8d and rPadbd7d4f required due to rPd321cc81, the code intertwines handling legacy data with handling/creating modern data.
Make things more understandable by clearly separating between both (handle one after the other) and by renaming some variables for clarity, so it will become slightly easier in the future to investigate this bottleneck (it is the only code querying the ManiphestTransaction table, leading to timeouts in large Phorge installations).

Also add a specific reference to the corresponding code change in a code comment, instead of a vague "late 2017".
Also, don't use the variable name `$table` for two different things (database vs AphrontTableView) in the same function.

Test Plan: Carefully read the code. Optionally, play with http://phorge.localhost/maniphest/report/burn/ with and without setting a project filter having tasks created in the codebase before 2017-11-22, and compare that the output is still the same.

Reviewers: O1 Blessed Committers, 20after4

Reviewed By: O1 Blessed Committers, 20after4

Subscribers: 20after4, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25828
2024-10-24 17:33:15 +02:00
Andre Klapper
5f4587bb88 Fix implicitly nullable parameter declarations for PHP 8.4
Summary:
Followup to rPdb61eb20 and rPf3d49f74. This patch should cover all remaining issues now that PHPStan covers it (instead of the previous trial-and-error approach).

Implicitly nullable parameter declarations are deprecated in PHP 8.4:
https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated

The proposed syntax was introduced in PHP 7.1 and Phorge requires PHP 7.2 now.

Test Plan: Run static code analysis.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25832
2024-10-24 17:19:42 +02:00
Andre Klapper
f3d49f7407 Fix implicitly nullable parameter declarations for PHP 8.4
Summary:
Implicitly nullable parameter declarations are deprecated in PHP 8.4:
https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated

The proposed syntax was introduced in PHP 7.1.

This patch is a followup on top of D25814.

Refs T15935

Test Plan:
* Try to view a task in the browser
* Try to run `./bin/auth recover` on the CLI
* Try to browse projects in the browser
* Try to create a new project without a name in the browser

Reviewers: O1 Blessed Committers, chris

Reviewed By: O1 Blessed Committers, chris

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15935

Differential Revision: https://we.phorge.it/D25816
2024-10-23 11:10:11 +02:00
Andre Klapper
db61eb20f0 Fix implicitly nullable parameter declarations for PHP 8.4
Summary:
Implicitly nullable parameter declarations are deprecated in PHP 8.4:
https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated

The proposed syntax was introduced in PHP 7.1.

Note: This code change does *not* fix all occurrences but only allows passing `./bin/storage upgrade`.

Refs T15935

Test Plan: Try to successfully run `./bin/storage upgrade` with PHP 8.4

Reviewers: O1 Blessed Committers, chris

Reviewed By: O1 Blessed Committers, chris

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15935

Differential Revision: https://we.phorge.it/D25814
2024-10-23 11:09:36 +02:00
Andre Klapper
b08c7c761c Fix PHP 8.1 "strlen(null)" exception rendering JSON file as Jupyter
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.

```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/files/document/PhabricatorJupyterDocumentEngine.php:326]
```

Closes T15951

Test Plan: See T15951

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

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

Maniphest Tasks: T15951

Differential Revision: https://we.phorge.it/D25830
2024-10-22 10:35:04 +02:00
Andre Klapper
23a49eb403 Bump PHP version requirement from 5.2.3 to 7.2.25
Summary:
Supporting PHP 8.4 requires syntax introduced in PHP 7.1.
Thus require PHP version 7.2.25 released on 21 November 2019.
Phorge previously required version 5.2.3 released on 31 May 2007.

Closes T15047

Test Plan: None.

Reviewers: O1 Blessed Committers, Sten, avivey

Reviewed By: O1 Blessed Committers, Sten, avivey

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

Maniphest Tasks: T15047

Differential Revision: https://we.phorge.it/D25815
2024-10-18 14:20:21 +02:00
Andre Klapper
b0bf1c689a Avoid RuntimeException passing bogus "service" parameter to Diffusion commit view
Summary:
`PhabricatorRepository::parseRepositoryServicePath()` can return `null` per https://we.phorge.it/source/phorge/browse/master/src/applications/repository/storage/PhabricatorRepository.php;123831b53fb7572cba11e9c990dcb9d247614890$635, thus make the code in `DiffusionServeController:getRequestDirectoryPath()` at least not crash when handling `null` and instead fall back to serving a "403 This repository is read-only over HTTP."

There is probably a cleaner approach which I happily leave to any future person willing to figure it out.

Closes T15944

Test Plan: Go to http://phorge.localhost/rABCD0123456789abcdef0123456789abcdef01234567?service=foo

Reviewers: O1 Blessed Committers, 20after4

Reviewed By: O1 Blessed Committers, 20after4

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15944

Differential Revision: https://we.phorge.it/D25826
2024-10-09 11:19:26 +02:00
Andre Klapper
9cd62bdcc4 Fix PHP 8.1 "strlen(null)" exception in SVN commit query without identifier
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.

```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/srv/phorge/phorge/src/applications/diffusion/query/DiffusionCommitQuery.php:779]
```

Closes T15936

Test Plan: Unclear.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15936

Differential Revision: https://we.phorge.it/D25821
2024-09-20 08:49:08 +02:00
Valerio Bozzolan
9acdd88893 Diffusion commit feed: fix commit title repeated twice
Summary:
Before this change any commit in your web feed had a duplicated commit
title. Old example:

> FooBar committed REPOFOO 132abc: add documentation (authored by FooBar)
> add documentation

After this change the commit title "add documentation" is repeated only once.

| Before              | After               |
|---------------------|---------------------|
| {F313288,size=full} | {F313289,size=full} |

So the web feed is slimmer and less distracting, more space for more useful info.

Having a NULL body seems OK. In fact, the upstream caller already skips
body rendering in that case:

https://we.phorge.it/source/phorge/browse/master/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php;0fe0b9f681d0da79b313e0907933665930074704$157-160

P.S.

If you think it would be nice to show a second line in the body web feed,
we think so too! Look at the mentioned task and please propose that feature.
This is just an early UX fix to avoid to repeat the same info twice.

Closes T15489

Test Plan:
Before this change, look at your web feeds about commits and reproduce
the original problem.

Apply this change and restart phd and do some commits and appreciate
that you do not see anymore duplicated commit titles in each commit feed.

Your email notifications are unchanged.

Reviewers: O1 Blessed Committers, aklapper

Reviewed By: O1 Blessed Committers, aklapper

Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15489

Differential Revision: https://we.phorge.it/D25824
2024-09-20 03:57:58 +02:00
Andre Klapper
85f51c5430 Fix PHP 8.1 "str_replace(null)" exception in DivinerAtomRef
Summary:
Passing null instead of a string or array to `str_replace()` deprecated since PHP 8.1.

Thus do not create a title array with a `null` entry in `DivinerFindController` when there is no `$query_text`, later to be read via `$this->titles` in `DivinerAtomRef`.

```
ERROR 8192: str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated at [/var/www/html/phorge/phorge/src/applications/diviner/atom/DivinerAtomRef.php:205]
  #0 str_replace(string, string, NULL) called at [<phorge>/src/applications/diviner/atom/DivinerAtomRef.php:205]
  #1 DivinerAtomRef::normalizeTitleString(NULL) called at [<phorge>/src/applications/diviner/query/DivinerAtomQuery.php:344]
```

Credits to valerio.bozzolan for finding the right spot in the code.

Closes T15911

Test Plan: Go to http://phorge.localhost/diviner/find/ (not passing a `name` URI parameter), optionally with D25768 applied to avoid another exception

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15911

Differential Revision: https://we.phorge.it/D25769
2024-09-17 09:20:30 +02:00
Andre Klapper
39237c0854 Set base-uri as User-Agent for OAuth1 and Github authentication
Summary: Closes T15848

Test Plan: Unclear.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

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

Maniphest Tasks: T15848

Differential Revision: https://we.phorge.it/D25758
2024-09-16 10:21:59 +02:00
Valerio Bozzolan
0fe0b9f681 Documentation: add article about Javelin DOM stuff
Summary:
Finally we know something about:

- how to get an element by id ($)
- how to find some children by a parent (scry)
- how to find an exact child by a parent (find)
- how to find a parent by a child (findAbove)

Closes T15870

Test Plan:
You read this article and you find something interesting and
probably also accurate.

Reviewers: O1 Blessed Committers, aklapper

Reviewed By: O1 Blessed Committers, aklapper

Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15870

Differential Revision: https://we.phorge.it/D25822
2024-09-14 15:40:32 +02:00
Andre Klapper
30c47690ff Remove unused *ParallelQueries() database connection methods
Summary:
`executeParallelQueries()` is never called as `supportsParallelQueries()` returns `false` both in `AphrontDatabaseConnection` and `AphrontMySQLDatabaseConnection`.
Thus remove this unused code which was intentionally meant for HHVM which does not support PHP>=7 anyway.

P.S. Dear Facebook, if you are using this, let us know. We don't want to hurt your legacy Phabricator installation.

Closes T15919

Test Plan: Grep for both functions.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15919

Differential Revision: https://we.phorge.it/D25789
2024-09-09 15:51:46 +02:00
Andre Klapper
aa2b70708e PHPDoc: Replace "@return this" with "@return $this"
Summary:
Per https://github.com/php-fig/fig-standards/blob/master/proposed/phpdoc.md the valid keyword is `$this`.
Thus replace `this` to make output of static code analysis slightly less noisy.

(See rARC4b7ee1985b3c15b4af81b5480522ae24c0afd6a2 for its Arcanist counterpart.)

Test Plan: Read the docs.

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/D25819
2024-09-09 15:48:09 +02:00
Andre Klapper
f25c531e56 Fix unit test failure in link-alternate
Summary: `arc unit --everything` triggers a `FAIL  PhutilRemarkupEngineTestCase::testEngine` due to adding the CSS class `remarkup-link-ext` in rP349f006904fabf1d4df31ff4840502af3ab379a7. Thus update it accordingly.

Test Plan: Run `arc unit`.

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/D25820
2024-09-07 11:00:41 +02:00
Andre Klapper
123831b53f Drop question mark suffix from optional PHPDoc @param types
Summary:
The question mark in `@param type? $foo Desc` is a custom notation not consistently applied across the codebase and not necessarily obvious to the reader (because custom and not mentioned in https://github.com/php-fig/fig-standards/blob/master/proposed/phpdoc.md ).
Instead, explicitly state "optional" in the parameter description for clarity.

Closes T15925.

Test Plan: Run PHPStan, see no `PHPDoc @param has invalid value (type? [...]` style output anymore.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15925

Differential Revision: https://we.phorge.it/D25797
2024-09-06 19:17:00 +02:00
Andre Klapper
9e6507d37a Fix exception handling in ConpherenceViewController::handleRequest()
Summary:
It is only possible `throw` an `Exception` class. `Aphront400Response` is not an Exception class but a `Phobject`.
Thus `return` it, like the `Aphront404Response`s within the same method.

Test Plan:
Check parent classes; run static code analysis. Hack the code to force that return,
and successfully obtain an HTTP 400 status code page.

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/D25780
2024-09-06 10:26:38 +02:00
Andre Klapper
b4bc480817 Fix PHP 8.1 "strlen(null)" exceptions creating an OAuth provider
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.

```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/auth/provider/PhabricatorOAuth1AuthProvider.php:163]
```

```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/auth/provider/PhabricatorOAuth1AuthProvider.php:178]
```

```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/auth/provider/PhabricatorOAuthAuthProvider.php:147]
```

Closes T15912

Test Plan: As an admin, create an OAuth provider (such as using Bitbucket) and check the error logs or Dark Console.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15912

Differential Revision: https://we.phorge.it/D25771
2024-09-05 12:28:22 +02:00
Andre Klapper
43539b220c Remove trivial cases of unreachable code
Summary:
Static code analysis can detect `Unreachable statement - code above always terminates.`
The vast majority of occurrences in the Phorge codebase are due to an unreachable `break` within a `case` after a `return` or after an all-covering `if/else`.
All this noise makes it harder to spot real logic issues (there are some!), thus fix these trivial cases.

Test Plan: Examine the code; run static code analysis.

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/D25802
2024-09-04 12:17:28 +02:00
Andre Klapper
8c3a62d6af Fix PHP 8.1 "strlen(null)" exception rendering a File's default Alt text
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.

Refs T15936

Test Plan: Read the code; compare to D25186.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15936

Differential Revision: https://we.phorge.it/D25817
2024-09-04 12:14:10 +02:00
Andre Klapper
24ebac8ad7 Remove unused shouldUse* functions from PhabricatorRepository
Summary:
Calls to private functions `PhabricatorRepository::shouldUseSSH()` and `PhabricatorRepository::shouldUseHTTP()` were removed in https://we.phorge.it/rP29d1115037b84454c4fd186c47f33f162cc337b9.
Call to private function `PhabricatorRepository::shouldUseSVNProtocol()` was removed in https://we.phorge.it/rP575c01373ee77c12b06696b34cfdf43a2eb26c42.

Test Plan: Grep the code; run static code analysis.

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/D25786
2024-08-29 10:25:56 +02:00
Valerio Bozzolan
3e53151815 Calendar Import: add unit tests to cover participants
Summary:
Add unit tests to easily double-check matched participants in imported calendar events.

This will simplify the addition of future features without the risk to break older workflows.

Ref T15564

Closes T15905
Closes T15906

Test Plan:
See green lights over your new unit tests:

    arc unit src/applications/calendar/import/__tests__/CalendarImportTestCase.php

Reviewers: O1 Blessed Committers, aklapper

Reviewed By: O1 Blessed Committers, aklapper

Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15906, T15564, T15905

Differential Revision: https://we.phorge.it/D25767
2024-08-28 09:31:18 +02:00
Bartosz Dziewoński
d4d620fa6d Workboard: have thin scrollbars
Summary:
The original intention was to hugely improve the UX interaction on Workboards for Microsoft Windows
users, since they have BIG GIANT SCROLLBARS. So we adopted thin scrollbars, that are graphically
pleasant for all other "normal" browsers too.

Note that this can be really thin now. The premise is: probably you will never notice this, since
you never try to click on the scrollbar.

In case, if you have problems, contact us. But note:

- you can use the mouse wheel as usual
- you can use keyboard navigation (try the tab key - it auto-scrolls!)
- you can use usual touch movements on relevant devices.

The non-standard CSS version is kept for compatibility.

Scrollbar examples in Microsoft Windows with Chromium-based browser:

| Before    | After     |
|-----------|-----------|
| {F312321} | {F312323} |

Scrollbar examples in GNU/Linux with KDE, on mouse hover:

| Before    | After     |
|-----------|-----------|
| {F313082} | {F313083} |

... on bar selected:

| Before    | After     |
|-----------|-----------|
| {F313084} | {F313085} |

Ref T15488

Test Plan:
View a Workboard and a Differential side panel
on Firefox with static scrollbars enabled.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

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

Maniphest Tasks: T15488

Differential Revision: https://we.phorge.it/D25305
2024-08-24 22:29:57 +02:00
Valerio Bozzolan
b02615bd50 Subversion: fix empty "Authored on" date on commit pages
Summary:
Subversion does not allow to have local commits. So, manage this case.

Closes T15612

Test Plan:
Surf a git commit: nothing changed.

Surf a svn commit: the "Authored on" date is not empty anymore.

Reviewers: O1 Blessed Committers, aklapper

Reviewed By: O1 Blessed Committers, aklapper

Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15612

Differential Revision: https://we.phorge.it/D25796
2024-08-24 15:34:48 +02:00
Andre Klapper
3c5e43cccb Fix PHP 8.1 "strlen(null)" exception in DiffusionGitRawDiffQuery
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.

```
Deprecated: strlen(): Passing null to parameter #1 ($string) of type string is deprecated in /home/phd/phabricator/src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php on line 38
```

Closes T15399

Test Plan: Read the code.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15399

Differential Revision: https://we.phorge.it/D25779
2024-08-22 11:17:34 +02:00
Andre Klapper
b9fb8d7891 Replace invalid PHPDoc @return type boolean with bool
Summary: `@return boolean` is not a valid type thus use `@return bool`.

Test Plan: None.

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/D25795
2024-08-22 11:17:07 +02:00