Improve tabs expansion #1

smortex · 2017-10-20T08:56:15Z

Dear fellow FreeBSD developers,

Tab expansion in Phabricator is really minimalist, which makes reviews hard to read for ports (or any files with tabs in the middle of a line), e.g. https://reviews.freebsd.org/D12461

This PR attempts to address this issue.

The expansion code itself was tested, but I cannot setup a full Phabricator instance on my side, so was hopping that somebody would be able to test this. Maybe we can upstream this PR at some point too.

My main question is "is $result a single line of text or does it contains multiple lines of text?". If it's a single line, of text, this PR should work like a charm. Otherwise, $result should be split at each \n, each part should be passed through expandLineTabs() and then joined again together. I would like to know I need to add a commit to do something like this.

Thanks!

Summary: Fixes T5965. Fixes two issues: - Observing an empty repository could write a warning to the log. - Mirroring an empty repository to a remote could fail. For observing: If newly-created with `git init --bare`, `git ls-remote` will return the empty string. Properly return an empty set of refs, rather than attempting to parse the single "line" that is produced by splitting that on newlines: ``` [2018-01-23 18:47:00] ERROR 8: Undefined offset: 1 at [/phab_path/phabricator/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:405] arcanist(head=master, ref.master=5634f8410176), phabricator(head=master, ref.master=12551a1055ce), phutil(head=master, ref.master=4755785517cf) #0 PhabricatorRepositoryPullEngine::loadGitRemoteRefs(PhabricatorRepository) called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:343] #1 PhabricatorRepositoryPullEngine::executeGitUpdate() called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:126] #2 PhabricatorRepositoryPullEngine::pullRepositoryWithLock() called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:40] #3 PhabricatorRepositoryPullEngine::pullRepository() called at [<phabricator>/src/applications/repository/management/PhabricatorRepositoryManagementUpdateWorkflow.php:59] ... ``` For mirroring: `git` treats `git push --mirror` specially when a repository is empty. Detect this case by seeing if `git for-each-ref --count 1` does anything. If the repository is empty, just bail. Test Plan: - Observed an empty and non-empty repository. - Mirrored an empty and non-empty repository. Reviewers: alexmv, amckinley Reviewed By: alexmv Subscribers: Korvin, epriestley Maniphest Tasks: T5965 Differential Revision: https://secure.phabricator.com/D18920

@abcdefghijklmnopqrstuvwxyz

Summary: See PHI513. `fprintf()` takes `(thing, pattern, args, ...)` but we aren't passing a `pattern`, so if the command returns a "%" in the output we get an error. Test Plan: - Installed `bytes`, a great useful program which prints all the bytes, on my HoaxOS(tm) system (see D19102). ``` epriestley@orbital ~/dev/phabricator $ ./bin/drydock command --lease 76287 -- bytes # Before patch. [2018-03-29 02:09:08] ERROR 2: fprintf(): Too few arguments at [/Users/epriestley/dev/core/lib/phabricator/src/applications/drydock/management/DrydockManagementCommandWorkflow.php:60] arcanist(head=experimental, ref.master=b8c9c385a7f5, ref.experimental=925c60e7b837), corgi(head=master, ref.master=6371578c9d32), instances(head=master, ref.master=d983b9517924), ledger(head=master, ref.master=4da4a24b8779), libcore(), phabricator(head=hoax1, ref.master=b586ee065a75, ref.hoax1=f8d7480bbdd1, custom=4), phutil(head=master, ref.master=1ad42491e44a), secure(head=master, ref.master=988cf9bd7958), services(head=master, ref.master=6b3fb8d8dd0a) #0 fprintf(resource, string) called at [<phabricator>/src/applications/drydock/management/DrydockManagementCommandWorkflow.php:60] #1 DrydockManagementCommandWorkflow::execute(PhutilArgumentParser) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:441] #2 PhutilArgumentParser::parseWorkflowsFull(array) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:333] #3 PhutilArgumentParser::parseWorkflows(array) called at [<phabricator>/scripts/drydock/drydock_control.php:21] epriestley@orbital ~/dev/phabricator $ ./bin/drydock command --lease 76287 -- bytes # After patch. !"#$%&'()*+,-./0123456789:;<=>?@abcdefghijklmnopqrstuvwxyz[\]^_`abcdefghijklmnopqrstuvwxyz{|}~???????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????? ``` Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Differential Revision: https://secure.phabricator.com/D19264

Since we want usernames to depend on the email provided, hack are way through it. An extension can not handle this level of intrusiveness I'm sorry oh coding gods

``` \tx xxxx\tx ``` should render: ``` x xxxx x ```

…ire protocol by git command (SSH transport) Summary: Makes `ssh-connect` compatible with Git v2 wire protocol over SSH More details about git V2 wire: https://opensource.googleblog.com/2018/05/introducing-git-protocol-version-2.html `git` command (2.18+) passes extra options (`-o "SendEnv GIT_PROTOCOL"`) to underlying `ssh` command to enable v2 wire protocol (environment variable enabling new protocol). Phabricator `ssh-connect` command doesn't understand `-o` options and interprets it as host parts hence when you enable git v2 all clones/ls-remotes crash with: ``` #0 ExecFuture::resolvex() called at [<phabricator>/src/applications/repository/storage/PhabricatorRepository.php:525] #1 PhabricatorRepository::execxRemoteCommand(string, PhutilOpaqueEnvelope) called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:400] #2 PhabricatorRepositoryPullEngine::loadGitRemoteRefs(PhabricatorRepository) called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:343] #3 PhabricatorRepositoryPullEngine::executeGitUpdate() called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:126] #4 PhabricatorRepositoryPullEngine::pullRepositoryWithLock() called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:40] #5 PhabricatorRepositoryPullEngine::pullRepository() called at [<phabricator>/src/applications/repository/management/PhabricatorRepositoryManagementUpdateWorkflow.php:59] #6 PhabricatorRepositoryManagementUpdateWorkflow::execute(PhutilArgumentParser) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:441] #7 PhutilArgumentParser::parseWorkflowsFull(array) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:333] #8 PhutilArgumentParser::parseWorkflows(array) called at [<phabricator>/scripts/repository/manage_repositories.php:22] COMMAND git ls-remote '********' STDOUT (empty) STDERR ssh: Could not resolve hostname -o: Name or service not known fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. at [<phutil>/src/future/exec/ExecFuture.php:369] ``` Test Plan: How to reproduce: 1. add repository to Phabricator which is accessed via `ssh` 2. Use git 2.18+ 3. Enable wire protocol in `/etc/gitconfig`: ``` [protocol] version = 2 ``` 4. Try refreshing repository: `phabricator/bin/repository update somecallsing` 5. Repository update fails with `ssh: Could not resolve hostname -o: Name or service not known` after this changes - updates will succeed Reviewers: epriestley, Pawka, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D19542

…ion entries Summary: When creating a Pholio mockup, `setIsCreateTransaction(true)` to avoid an empty `Transaction` field in the Feed and avoid `strncmp()` complaining about a null value being passed. This is very similar to T15659 about Differential Diffs. ``` ERROR 8192: strncmp(): Passing null to parameter freebsd#1 ($string1) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/policy/query/PhabricatorPolicyQuery.php:314] ``` Closes T15679 Test Plan: 1. Create a new Pholio mockup via `/pholio/create/` 2. Go to `/feed/transactions/query/all/` 3. Compare entries in the `Transaction` column before (empty ones) and after (no empty ones) applying the patch. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15679 Differential Revision: https://we.phorge.it/D25600

…ults 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 freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php:154] ``` See also similar rPe65ac7b8800d3397abfced7824a8d316ec573d8f Closes T15818 Test Plan: Go to http://phorge.localhost/conpherence/search/query/all/ Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15818 Differential Revision: https://we.phorge.it/D25629

…n PhabricatorSetupCheck 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 freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/config/check/PhabricatorSetupCheck.php:125] ``` Closes T15823 Test Plan: Run `./bin/cache purge --all`, check any Phorge page in browser before and after applying this patch. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15823 Differential Revision: https://we.phorge.it/D25633

…r.php 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 freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/auth/controller/PhabricatorAuthStartController.php:34] ``` Closes T15832 Test Plan: Run `arc unit` locally, probably also don't be logged in. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15832 Differential Revision: https://we.phorge.it/D25639

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 freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/files/document/render/PhabricatorDocumentRenderingEngine.php:211] ``` Closes T15826 Test Plan: Surf a Diffusion repository and play with {nav View Options}. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15826 Differential Revision: https://we.phorge.it/D25638

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 freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/dashboard/menuitem/PhabricatorDashboardPortalMenuItem.php:34] ``` Closes T15847 Test Plan: Go to `/portal/edit/form/default/`, set a name and click `Create Portal`. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15847 Differential Revision: https://we.phorge.it/D25681

…y panel with no query defined 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 freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/dashboard/editfield/PhabricatorDashboardQueryPanelQueryEditField.php:41] ``` ``` ERROR 8192: strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/dashboard/editfield/PhabricatorDashboardQueryPanelQueryEditField.php:59] ``` Closes T15791 Test Plan: Successfully create a Dashboard query panel searching for "Diffusion Raw Commits" (which has no Query defined per T15790) and try to edit it afterwards under PHP 8.1 or later. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15791 Differential Revision: https://we.phorge.it/D25596

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. Passing `null` to `preg_match()` is deprecated behavior since PHP 8.1. Thus only call `preg_match()` when the value is set. 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: preg_match(): Passing null to parameter freebsd#2 ($subject) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/calendar/import/PhabricatorCalendarImportEngine.php:238] ``` ``` ERROR 8192: strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/calendar/import/PhabricatorCalendarImportEngine.php:245] ``` Closes T15852 Test Plan: Import an ICS file which lists attendees without a name but only with an email address into the calendar via http://phorge.localhost/calendar/import/ > "Import Events" > "Import .ics File" Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15852 Differential Revision: https://we.phorge.it/D25686

block 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 freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/diviner/controller/DivinerAtomController.php:440] ``` Closes T15853 Test Plan: Run `./bin/diviner generate` and visit a Diviner page about a class which has no PHPDoc `@task` block, for example access http://phorge.localhost/diviner/find/?name=PhutilSafeHTML&type=class&jump=1 Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15853 Differential Revision: https://we.phorge.it/D25689

…en sending email 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. ``` ERROR 8192: strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [/var/www/htdocs/phorge/phorge/src/applications/metamta/adapter/PhabricatorMailSMTPAdapter.php:65] ``` Closes T15857 Test Plan: Send Welcome email via 'Manage' user screen and Run `./bin/phd log` afterwards to validate email action Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15857 Differential Revision: https://we.phorge.it/D25692

… in PhabricatorEditEngine 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 freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/transactions/editengine/PhabricatorEditEngine.php:1923] ``` ``` ERROR 8192: strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/transactions/editengine/PhabricatorEditEngine.php:2023] ``` Closes T15864 Test Plan: See steps in https://we.phorge.it/T15864 Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15864 Differential Revision: https://we.phorge.it/D25699

…View Summary: Passing null to `file_exists()` is deprecated behavior since PHP 8.1. The already existing `if ($file)` check in `AphrontStackTraceView` implies that `$file` can indeed be empty. Thus add another such check higher up in that class to avoid deprecation warnings when rendering stacktraces. ``` ERROR 8192: file_exists(): Passing null to parameter freebsd#1 ($filename) of type string is deprecated at [/var/www/html/phorge/arcanist/src/filesystem/Filesystem.php:1068]; #0 file_exists(NULL) called at [<arcanist>/src/filesystem/Filesystem.php:1068]; freebsd#1 Filesystem::pathExists(NULL) called at [<arcanist>/src/filesystem/Filesystem.php:1180]; freebsd#2 Filesystem::assertExists(NULL) called at [<arcanist>/src/filesystem/Filesystem.php:1020]; freebsd#3 Filesystem::isDescendant(NULL, string) called at [<phorge>/src/view/widget/AphrontStackTraceView.php:33]; ``` Closes T15881 Test Plan: Intentionally inject an `Array to string conversion bug` in `PhabricatorEmailPreferencesSettingsPanel` (see T15881). Then visit `/settings/panel/emailpreferences/`. See the same stacktrace before and after applying this change, but see only a single line of output in DarkConsole / Error Log after applying this patch and not anymore numerous `Passing null to parameter freebsd#1 ($filename) of type string is deprecated` errors in DarkConsole / Error Log. Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15881 Differential Revision: https://we.phorge.it/D25728

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. Note: This patch also corrects two further `strlen()` occurrences with the same pattern. ``` ERROR 8192: strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/auth/provider/PhabricatorLDAPAuthProvider.php:145] ``` Closes T15893 Test Plan: Create an LDAP user without setting their password; try to log into Phabricator with that user via the LDAP auth provider. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15893 Differential Revision: https://we.phorge.it/D25748

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 freebsd#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

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 freebsd#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

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 freebsd#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

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 freebsd#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

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 freebsd#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

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 freebsd#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 freebsd#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 freebsd#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

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 freebsd#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] freebsd#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

…tifier 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 freebsd#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

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 freebsd#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

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) freebsd#1 iconv_mime_decode(string, integer, string) called at [<arcanist>/src/utils/utils.php:1759] freebsd#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

Summary: Passing `null` as the first parameter to `ltrim()` is deprecated since PHP 8.1. Thus change the optional parameter of the function from null to an empty string. ``` ERROR 8192: ltrim(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/src/applications/nuance/source/NuanceSourceDefinition.php:211] ``` Closes T16000 Test Plan: * Create a Nuance queue at `/nuance/queue/edit/form/default/` * Create a Nuance "Web Form" type source at `/nuance/source/edit/form/default/` Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T16000 Differential Revision: https://we.phorge.it/D25890

Summary: This restores the pre-PHP 8.1 behavior, where values of static variables within inherited methods were independent of each other. With PHP 8.1, this was changed to be truly 'static', which causes problems when one derivate of PhabricatorLiskDAO defines a custom serializer but another does not. This came to light in T15726, but only for the Fund application, which is a prototype, and deprecated. This fixes Fund, but more importantly, everything else that would be broken by this, whatever it was. Ref: https://wiki.php.net/rfc/static_variable_inheritance Previous stacktrace: ``` EXCEPTION: (RuntimeException) Undefined array key "totalAsCurrency" at [<arcanist>/src/error/PhutilErrorHandler.php:273] #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php:345] freebsd#1 <freebsd#2> PhabricatorLiskDAO::willWriteData(array) called at [<phorge>/src/infrastructure/storage/lisk/LiskDAO.php:1085] freebsd#2 <freebsd#2> LiskDAO::insertRecordIntoDatabase(string) called at [<phorge>/src/infrastructure/storage/lisk/LiskDAO.php:958] freebsd#3 <freebsd#2> LiskDAO::insert() called at [<phorge>/src/infrastructure/storage/lisk/LiskDAO.php:927] freebsd#4 <freebsd#2> LiskDAO::save() called at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1405] [...] ``` Test Plan: On PHP 8.1+: 1. Visit http://phorge.localhost/applications/ and enable the deprecated prototype applications "Fund" and "Phortune" via "Configure" 2. Visit http://phorge.localhost/phortune/merchant/edit/ and create a merchant 3. Visit http://phorge.localhost/fund/create/ and click the "Create New Initiative" button Reviewers: O1 Blessed Committers, aklapper, valerio.bozzolan Reviewed By: O1 Blessed Committers, aklapper, valerio.bozzolan Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15064 Differential Revision: https://we.phorge.it/D25859

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 freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/auth/xaction/PhabricatorAuthFactorProviderNameTransaction.php:20] ``` Closes T15992 Test Plan: After applying the changes, go to http://phorge.localhost/feed/transactions/ and check the entries related to setting up an MFA provider. Reviewers: O1 Blessed Committers, slip, avivey Reviewed By: O1 Blessed Committers, slip, avivey Subscribers: slip, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15992 Differential Revision: https://we.phorge.it/D25867

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 freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/conpherence/query/ConpherenceFulltextQuery.php:58] ``` ``` ERROR 8192: strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/conpherence/query/ConpherenceFulltextQuery.php:73] ``` Closes T16062 Test Plan: Go to a Conpherence room and search for an existing string. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T16062 Differential Revision: https://we.phorge.it/D25994

Summary: Do not throw an exception when passing bogus parameter string values to hovercards. Instead, just convert to an empty array. In consequence, see a proper hovercard correctly displaying `Unknown Object (????)` instead of an exception: ``` EXCEPTION: (TypeError) idx(): Argument freebsd#1 ($array) must be of type array, string given, called in /var/www/html/phorge/phorge/src/applications/search/controller/PhabricatorSearchHovercardController.php on line 37 at [<arcanist>/src/utils/utils.php:37] ``` See also similar rPa5384ca60470e56a5312d5a7147ddd3ffa2e75d8. Closes T16075 Test Plan: * Go to http://phorge.localhost/search/hovercard/?__path__=%2fsearch%2fhovercard%2f&cards={"layout":"foo"} * Get an error message before applying the patch. Get an empty hovercard after applying the patch. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T16075 Differential Revision: https://we.phorge.it/D26023

…atorOAuthServerTokenController 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. ``` strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated #0 PhabricatorOAuthServerTokenController::handleRequest(AphrontRequest) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:284] ``` See Q182 Test Plan: Read the code. `strlen` is used to get the length of an existing string and not to check for emptiness of a string. There is no string length comparison in the existing code. 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/D26034

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 freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/search/controller/PhabricatorSearchController.php:34] ``` Test Plan: Visit `http://phorge.localhost/search/?search:primary=true` 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/D26061

grimreaper force-pushed the master branch from e939ae5 to dd7f0bd Compare October 27, 2017 03:05

grimreaper force-pushed the master branch from dd7f0bd to bc07e5e Compare November 7, 2017 07:00

grimreaper force-pushed the master branch from bc07e5e to 247dedc Compare December 11, 2017 04:06

grimreaper force-pushed the master branch 2 times, most recently from 161a5f0 to bc8a968 Compare January 1, 2018 01:29

grimreaper force-pushed the master branch from bc8a968 to a75c59f Compare January 24, 2018 05:09

grimreaper force-pushed the master branch from a75c59f to 827dfe1 Compare February 9, 2018 12:51

grimreaper force-pushed the master branch from 827dfe1 to 6879f79 Compare February 9, 2018 12:52

grimreaper force-pushed the master branch from 6879f79 to 3ff1f92 Compare March 7, 2018 16:35

grimreaper force-pushed the master branch from 3ff1f92 to bcf27c8 Compare March 18, 2018 03:59

grimreaper force-pushed the master branch from bcf27c8 to b881745 Compare April 8, 2018 03:23

grimreaper force-pushed the master branch from b881745 to 081b3b7 Compare April 21, 2018 05:15

grimreaper added 5 commits April 24, 2018 20:57

[FreeBSD] Freebsd custom patch for user auth

5517725

Since we want usernames to depend on the email provided, hack are way through it. An extension can not handle this level of intrusiveness I'm sorry oh coding gods

[FreeBSD] FreeBSD prefers 8 space tabs rather than 2

7fd1fb7

[FreeBSD] change our default pid

b5e5b6e

[FreeBSD] something is borked with preamble

06c2fb8

Inform users of username

c63bc14

grimreaper force-pushed the master branch from f979b3d to c63bc14 Compare April 25, 2018 03:57

grimreaper and others added 2 commits April 24, 2018 21:11

minor tweaks to how registration is layed out (see bug 227753)

38f6720

Improve tabs expansion

e15d1ff

``` \tx xxxx\tx ``` should render: ``` x xxxx x ```

smortex force-pushed the align-tabs-properly branch from efe339b to e15d1ff Compare May 11, 2018 19:52

grimreaper force-pushed the master branch from 38f6720 to 0f3938f Compare June 13, 2018 11:24

grimreaper force-pushed the master branch 2 times, most recently from 121ae72 to 012fc0e Compare July 10, 2018 01:44

grimreaper force-pushed the master branch from 012fc0e to 34d6e6e Compare July 21, 2018 11:00

grimreaper force-pushed the master branch from 34d6e6e to 8ad1e7c Compare August 6, 2018 04:08

grimreaper force-pushed the master branch from 8ad1e7c to 783077e Compare August 17, 2018 01:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improve tabs expansion #1

Improve tabs expansion #1

Uh oh!

smortex commented Oct 20, 2017

Uh oh!

Uh oh!

Improve tabs expansion #1

Are you sure you want to change the base?

Improve tabs expansion #1

Uh oh!

Conversation

smortex commented Oct 20, 2017

Uh oh!

Uh oh!