1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-22 21:40:55 +01:00
Commit graph

17671 commits

Author SHA1 Message Date
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
Andre Klapper
b44b566b13 Add missing variable names to PHPDoc @param of methods
Summary:
Add variable names (`$varname` in `@param type $varname explanation`) to PHPDoc method headers, for fun and profit.

Closes T15923

Test Plan:
* Read the method signatures and their corresponding PHPDoc headers at your fireplace
* Still run `./bin/diviner generate` without explosions (though it is very lenient anyway?)

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15923

Differential Revision: https://we.phorge.it/D25794
2024-08-21 15:06:26 +02:00
Andre Klapper
9f07e5083d Add some DateTime handling related PHPDocs
Summary:
As I was fiddling with an exception handling invalid DateTime, add some PHPDoc
for the next time someone needs to understand this code.

Test Plan: Use `get_class()` to check parameters and return values.

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/D25793
2024-08-20 13:47:27 +02:00
Andre Klapper
e1e1570c11 Remove unused methods from ConpherenceDurableColumnView
Summary: Calls to private methods `ConpherenceDurableColumnView::buildStatusText()` and `ConpherenceDurableColumnView::buildSendButton()` were removed in https://we.phorge.it/rP95d174956639aac7525d52368944edfff7cdb824

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/D25784
2024-08-20 10:11:24 +02:00
Andre Klapper
cf5abd9e4b Remove unused HeraldAdapter:buildTokenizerFieldValue()
Summary:
This private function was added in https://we.phorge.it/rP715233fb611569b86f8fd4d8339a7e514c749d69.
Its last use was removed in https://we.phorge.it/rP776caa507b15ef013d3d623d30b294181e2b4c11.

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/D25785
2024-08-20 10:09:01 +02:00
Andre Klapper
9c684c80d6 Fix various potential PHP 8.1 "strlen(null)" exceptions
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.

All lines changed in this patch had `Parameter #1 $string of function strlen expects string, string|null given` reported by PHPStan. Thus these should be safe to replace with `phutil_nonempty_string` as no calls care about the actual `strlen()` return value (length of a string).

Test Plan: Run static code analysis via `phpstan analyse -l 9`

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/D25778
2024-08-19 14:48:44 +02:00
Andre Klapper
66bd13b193 Remove unused PHUICalendarMonthView::getMaxDailyEventsForWeek()
Summary: Call to this private function was removed in https://we.phorge.it/rPaee9d88c17e25b924ecd5293691cc6fe38d6c7d9.

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/D25788
2024-08-18 14:57:09 +02:00
Andre Klapper
edb2175a9b Fix PhabricatorEmptyQueryException handling in DiffusionCommitQuery::loadPage()
Summary: Add an additional check that `$empty_exception` is not `null` before try to `throw` it.

Test Plan: Read the previous 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/D25781
2024-08-15 20:23:52 +02:00
b9ea6f1ce8 docs/file_domain: remove cloudflare mention of autominify
Summary:
[Auto Minify](https://developers.cloudflare.com/speed/optimization/content/auto-minify/)[1][2] is deprecated and will be removed after 2024-08-05.

(For distant future where Cloudflare deletes the docs)
[1]: [web.archive.org](https://web.archive.org/web/20240507123639/https://developers.cloudflare.com/speed/optimization/content/auto-minify/)
[2]: [archive.today](https://archive.md/wWPpM)

Signed-off-by: Yongmin Hong <revi@omglol.email>

Test Plan:
1. Wait for 2024-08-05
2. Browse to `https://dash.cloudflare.com/<account_id>/<domain>/speed/optimization/content`
3. Verify "Auto Minify" is gone

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25617
2024-08-13 11:24:37 +02:00
Andre Klapper
58f1c83a5a Fix PHP 8.1 "strlen(null)" exception in DivinerFindController
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/diviner/controller/DivinerFindController.php:40]
```

Closes T15910

Test Plan:
Go to this (or any other Diviner Find URL not passing a `type` URI parameter): now it works:

http://phorge.localhost/diviner/find/?name=PhutilSafeHTML

Go to this (or any other Diviner Find URL passing a `type` URI parameter): it still works:

http://phorge.localhost/diviner/find/?name=PhutilSafeHTML&type=class

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15910

Differential Revision: https://we.phorge.it/D25768
2024-08-11 22:36:44 +02:00
Dylan F
2b7c0ec92f Destroy file attachments when file is deleted, or object is deleted
Summary:
Adds file attachment deletion logics:

- PhabricatorFile: delete the attachment if file is deleted
- destruction engine extension: delete attachment if object is deleted
- SQL patch: delete existing leftover attachments from deleted files

To apply the cleanup, as usual, run:

    ./bin/storage upgrade

This cleanup may take some time, proportionally to the size of these tables:

    phabricator_file.file
    phabricator_file.file_attachment

Just as an indication: the storage upgrade, in a Phorge with `file` count 1.3M rows and `file_attachment` consisting in 9K rows, it may delete 170K rows in less than 1 second on average hardware.

Closes T15110

Test Plan:
Apply the patch, run `./bin/storage/upgrade`:

- no "Unknown Object" in any "Referenced Files" curtain of any object.

Have phd daemon running.

Upload file, attach the file to a task, delete the file from the web interface:

- no "Unknown Object" in "Referenced Files" curtain of that task.
- the query `SELECT * FROM file_attachment WHERE filePHID = '<file phid>'` returns empty result

Upload file, attach the file to a task, delete the task from the `./bin/remove destroy` workflow:

- the query `SELECT * FROM file_attachment WHERE objectPHID = '<task phid>'` returns empty result

Reviewers: O1 Blessed Committers, avivey, valerio.bozzolan

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

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

Tags: #files

Maniphest Tasks: T15110

Differential Revision: https://we.phorge.it/D25051
2024-08-10 22:16:53 +02:00
Andre Klapper
b74f1ad519 Fix 404 link in Diviner to "Configuring File Storage" article
Summary: Fix a broken link in documentation.

Test Plan:
1. Go to https://we.phorge.it/book/dev/class/PhabricatorFileStorageEngine/
2. Click link "For more information, see File Storage Technical Documentation."
3. Get a 404 via https://we.phorge.it/diviner/find/?name=File_Storage_Technical_Documentation&type=article&jump=1 beforehand; get an article via https://we.phorge.it/diviner/find/?name=Configuring_File_Storage&type=article&jump=1 afterhand.

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/D25762
2024-08-07 17:40:55 +02:00
Andre Klapper
88225b99f2 Fix PHP 8.1 "strlen(null)" exception in ManiphestTaskPointsTransaction
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/maniphest/xaction/ManiphestTaskPointsTransaction.php:85]
```

Closes T15902

Test Plan: Unclear due to lack of steps to reproduce.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15902

Differential Revision: https://we.phorge.it/D25765
2024-08-07 13:57:07 +02:00
Andre Klapper
89778dc9e6 Add Open Graph protocol meta tags to Maniphest task pages
Summary:
Add OGP <meta> tags to Maniphest task pages when the task is publicly accessible and anonymously accessed. See https://ogp.me/

Based on rP2c72c2b924ffa3f8a49dbec636a2cdca3bae004f reverted in rP49b57eae7df52c189aef1d973823c697fc97fd4b.

Closes T15472

Test Plan:
* Use the default Phorge logo, open a Maniphest task, look at the headers in its HTML.
* Set a custom Phorge logo via `config/edit/ui.logo/`.
* Access a task with "View Policy: All Users" while logged in: No OGP headers included.
* Access a task with "View Policy: Public" while logged in: No OGP headers included.
* Access a task with "View Policy: All Users" while logged out: No OGP headers included; "Access Denied: Restricted Maniphest Task" displayed.
* Access a task with "View Policy: Public" while logged out: OGP headers included.
* Access a task with "View Policy: Public" while logged out with a task description and a task without a task description: OGP headers included.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

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

Maniphest Tasks: T15472

Differential Revision: https://we.phorge.it/D25668
2024-08-07 13:54:11 +02:00
Andre Klapper
1cc04fb83c Explicitly cast "limit" (page size) API parameter to int
Summary:
Do not throw an exception when the `limit` (page size) Conduit API parameter is a float but explicitly convert to int.
As an admin, I am not interested in having invalid user-committed query data trigger an error in the server logs.

```
ERROR 8192: Implicit conversion from float to int loses precision at [/var/www/html/phorge/phorge/src/view/control/AphrontCursorPagerView.php:76]
```

Closes T15810

Test Plan: Call `/conduit/method/maniphest.search/` with a float value for the `limit` field. Check the server logs or DarkConsole.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

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

Maniphest Tasks: T15810

Differential Revision: https://we.phorge.it/D25614
2024-08-04 12:16:05 +02:00
Andre Klapper
ec6ba69666 Add Copy file path button to file header bar in Differential/Diffusion
Summary:
When looking at a commit/revision, allow to copy the path of a file to the clipboard, so you can quickly open that file in your local code checkout from the command line.

Closes T15897

Test Plan:
Open a proposed commit in Differential or a merged commit in Diffusion. Click the "Copy file path" button, paste that clipboard content.
Change the width of the viewport and see that the button is not rendered when in phone mode.
Run `./bin/celerity map` again and no changes.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15897

Differential Revision: https://we.phorge.it/D25757
2024-08-04 11:53:24 +02:00
Andre Klapper
0bfe9ca88f Log Herald rules with invalid actions via phlog()
Summary:
When Herald rules fail unexpectedly due to relying on a (now) invalid action, allow Phorge administrators to get aware by creating an entry in the error log via `phlog()` instead of crossing fingers that someone may from time to time get the idea to open and read Herald transcripts.

Refs T15887

Test Plan: See steps in T15887; check error log.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

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

Maniphest Tasks: T15887

Differential Revision: https://we.phorge.it/D25737
2024-08-04 11:35:55 +02:00
Andre Klapper
71615f4b99 Log Herald rules having disabled Herald rules as condition via phlog()
Summary:
Do not silently fail when a Herald rule refers in its conditions to a disabled rule.
Allow Phorge administrators to get aware by creating an entry in the error log via phlog() instead of crossing fingers that someone may from time to time get the idea to open and read Herald transcripts.

This is similar to T15885 / D25735.

Refs T15869

Test Plan:
1. Phorge at `0873b36569bfcf502a092870d7d5f4d088ed9003`
2. Set up a Herald rule and disable it
3. Set up a second Herald rule with several conditions, the first condition being "Another Herald rule matches" on the disabled first Herald rule
4. Go to http://phorge.localhost/herald/test/ to apply the Herald rule on a corresponding object
5. See an exception in the resulting transcript at http://phorge.localhost/herald/transcript/123456/
6. After applying this patch, also find that exception in the error log

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15869

Differential Revision: https://we.phorge.it/D25736
2024-08-04 11:35:31 +02:00
Andre Klapper
f6302c7ee3 Log Herald rules with invalid keys via phlog()
Summary:
When Herald rules fail unexpectedly due to relying on a (now) invalid key, allow Phorge administrators to get aware by creating an entry in the error log via `phlog()` instead of crossing fingers that someone may from time to time get the idea to open and read Herald transcripts.

Refs T15885

Test Plan: See steps in T15885; check error log.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15885

Differential Revision: https://we.phorge.it/D25735
2024-08-04 11:35:05 +02:00
Andre Klapper
93c9afd2f3 Do not add actor as subscriber when removing a comment
Summary:
When an admin removes a comment (e.g. spam), the admin gets subscribed to the task. This is usually unwanted as the removal action does not imply that the admin is interested in receiving future notifications about the task, in contrast to e.g. adding a comment to a discussion in the task.

Any transaction of a comment (add, edit, remove) is a `"core:comment"` action. The code calls `applyImplicitCC()` which calls `shouldImplyCC()` which returns the bool `$xaction->isCommentTransaction()`. Expand this bool to `$xaction->isCommentTransaction() && !($xaction->getComment()->getIsRemoved())`.

Closes T15899

Test Plan:
* As an admin, go to a task which has comments and to which you are not subscribed
* Click the dropdown for the comment, select Remove comment
* See that you did not get subscribed to the task

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15899

Differential Revision: https://we.phorge.it/D25760
2024-08-04 11:30:03 +02:00
Andre Klapper
4da3b096b0 Configuration Guide: Set UnsafeAllow3F for Apache RewriteRule
Summary:
Since Apache HTTP Server 2.4.61 including https://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/mappers/mod_rewrite.c?r1=1918560&r2=1918561&pathrev=1918561&diff_format=h due to https://www.cve.org/CVERecord?id=CVE-2024-38474, URIs including %3F throw a HTTP 403 error and the following error log entry:

`AH10508: Unsafe URL with %3f URL rewritten without UnsafeAllow3F`

Update the corresponding RewriteRule in the Phorge configuration guide to explicitly set UnsafeAllow3F.

https://httpd.apache.org/docs/2.4/rewrite/flags.html#flag_unsafe_allow_3f

Closes T15889

Test Plan: Run Apache HTTP Server 2.4.61, go to https://phorge.localhost/maniphest/task/edit/form/default/?title=%3f and get a HTTP 403 (before) or a "?" as task title (after).

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15889

Differential Revision: https://we.phorge.it/D25739
2024-08-04 09:58:57 +02:00
Andre Klapper
62aaa0fcbe Restrict maximum task title length to 255 characters
Summary:
A task title is supposed to summarize a task. For long novels, there is the task description.

Closes T15898

Test Plan: Go to http://phorge.localhost/maniphest/task/edit/form/default/ and enter a task title with multi-byte characters above the threshold, press "Enter".

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15898

Differential Revision: https://we.phorge.it/D25759
2024-08-04 01:43:51 +02:00
Andre Klapper
d8b94dd87d Misc PHPDoc additions to PhabricatorFile
Summary: Add some PHPDoc comments to the `PhabricatorFile` class as I was playing with calling some of its methods.

Test Plan: Call those methods and see what they do and return.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25763
2024-08-03 14:19:49 +02:00
Andre Klapper
81ffe4f8b1 Fix PHP 8.1 "strlen(null)" exception in PhabricatorAuthAccountView
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/view/PhabricatorAuthAccountView.php:66]
```

Closes T15900

Test Plan: Try to register an account via LDAP.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15900

Differential Revision: https://we.phorge.it/D25761
2024-08-02 21:26:58 +02:00
Andre Klapper
a110aa471e Suppress PHP 8.1 warnings "Return type mixed is not covariant with tentative return type mixed of method Iterator::key()"
Summary:
PHP 8.1 requires classes which implement `Iterator` to have all `Iterator` methods declared with appropriate return types or annotated with `#[\ReturnTypeWillChange]`. In my (limited) understanding we cannot declare return types while remaining backward-compatible, thus annotate.

Closes T15835

Test Plan: Run PHPStan static code analyzer, see no warnings anymore.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15835

Differential Revision: https://we.phorge.it/D25711
2024-08-02 11:44:25 +02:00
jesse
721d5ed64b fix PHP8 array_slice($results, null) which cause diffusion pattern search broken
Test Plan: check diffusion pattern search if working

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/D25756
2024-07-31 09:51:18 +08:00
Andre Klapper
e2919211f3 Avoid PhabricatorApplicationTransactionStructureException on editors not supporting Mute Notifications
Summary:
Do not expose the "Mute Notifications" sidebar menu entry when the underlying Editor for that object type does not support muting notifications (means: creating a transaction of type "core:edge") at all.
This avoids a disappointing `PhabricatorApplicationTransactionStructureException` after two clicks.

Disabling the menu entry while still exposing it makes no sense here as the user could never get it enabled anyway.

```
EXCEPTION: (PhabricatorApplicationTransactionStructureException) Attempting to apply a transaction (of class "PhabricatorFileTransaction", with type "core:edge") which has not been constructed correctly: Transaction has type "core:edge", but that transaction type is not supported by this editor (PhabricatorFileEditor). at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1830]
```

```
EXCEPTION: (PhabricatorApplicationTransactionStructureException) Attempting to apply a transaction (of class "LegalpadTransaction", with type "core:edge") which has not been constructed correctly: Transaction has type "core:edge", but that transaction type is not supported by this editor (LegalpadDocumentEditor). at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1830]
```

Closes T15378

Test Plan:
* Open various different object types:
** Files: http://phorge.localhost/F1 (not implemented)
** Legalpad: http://phorge.localhost/legalpad/view/1/ (not implemented)
** Maniphest: http://phorge.localhost/T1 (implemented)
* Click "Subscribe" on these various types of objects.
* Click "Mute Notifications" on these various types of objects, remember which objects throw an exception.
* Apply this patch.
* Check that "Mute Notifications" is not exposed for those types of objects that threw an exception.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15378

Differential Revision: https://we.phorge.it/D25730
2024-07-30 10:52:07 +02:00