PhutilSortVector.php:33)
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	aklapper
	Mar 14 2024, 08:38

Description

Steps to reproduce:

Go to https://phabricator.wikimedia.org/diffusion/EWIS/branches/master/composer.json;0df33d42c884c47bac8d23f3da60281c90ed966c (which is running on PHP 7.4 as far as I know)

Actual outcome:

[2024-03-09 01:22:35] EXCEPTION: (RuntimeException) strpos(): Non-string needles will be interpreted as strings in the future. Use an explicit chr() call to preserve the current behavior at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(), ava(), phorge(), translations(), wmf-ext-misc()
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<arcanist>/src/error/PhutilErrorHandler.php:261]
  #1 <#2> strpos(string, integer) called at [<arcanist>/src/utils/PhutilSortVector.php:33]
  #2 <#2> PhutilSortVector::addString(integer) called at [<phorge>/src/applications/diffusion/controller/DiffusionBranchTableController.php:132]
  #3 <#2> DiffusionBranchTableController::sortBranches(PhabricatorRepository, array) called at [<phorge>/src/applications/diffusion/controller/DiffusionBranchTableController.php:43]
  #4 <#2> DiffusionBranchTableController::handleRequest(AphrontRequest) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:284]
  #5 phlog(RuntimeException) called at [<phorge>/src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php:41]
  #6 PhabricatorDefaultRequestExceptionHandler::handleRequestThrowable(AphrontRequest, RuntimeException) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:751]
  #7 AphrontApplicationConfiguration::handleThrowable(RuntimeException) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:296]
  #8 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:204]
  #9 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phorge>/webroot/index.php:35]

Additional information:

See PHP 7.3 related info at https://www.php.net/manual/en/function.strpos.php
https://we.phorge.it/source/arcanist/browse/master/src/utils/PhutilSortVector.php$32 performs a strpos("\0", $value). $value is $branch->getShortName() being passed in https://we.phorge.it/source/phorge/browse/master/src/applications/diffusion/controller/DiffusionBranchTableController.php$115-132

Revisions and Commits

rARC Arcanist
	D25557	rARCef73b12b580e Fix "strpos(): Non-string needles will be interpreted as strings" in…

Event Timeline

aklapper created this task.Mar 14 2024, 08:38

Herald added subscribers: Cigaryno, Matthew, chris, tobiaswiese. · View Herald TranscriptMar 14 2024, 08:38

valerio.bozzolan subscribed.Mar 14 2024, 09:15

I also don't understand strpos("\0", $value) since the documentation says that the second argument is the needle

$ php --rf strpos
Function [ <internal:standard> function strpos ] {

  - Parameters [3] {
    Parameter #0 [ <required> string $haystack ]
    Parameter #1 [ <required> string $needle ]
    Parameter #2 [ <optional> int $offset = 0 ]
  }
  - Return [ int|false ]
}

Apparently PhutilSortVector.php is somehow recent, 2016, without any comment about that strpos("\0", $value) that is probably not correct

https://secure.phabricator.com/D15413

I assume we should change strpos("\0", $value) to strpos($value, chr(0))

Let's split the problems:

arguments order - yes they are wrong, let's swap
the needle "\0": we can maybe keep that string as-is (but as second argument) since it's probably correct since it's a string. The chr(0) is just an additional function call to obtain the same valid string. The error was about $value as needle that sometime is an integer. So maybe it's just fine with strpos($value, "\0")
about the $value that sometime is an integer (?): this may deserve additional attention to avoid a different warning in the future even with the correct strpos($value, "\0")

Probably better to clone that repository offline and catch that issue to investigate $value and get a better decision.

aklapper added a revision: D25557: Fix "strpos(): Non-string needles will be interpreted as strings" in PhutilSortVector.Mar 22 2024, 10:26

aklapper closed this task as Resolved by committing rARCef73b12b580e: Fix "strpos(): Non-string needles will be interpreted as strings" in….Mar 22 2024, 12:35

aklapper claimed this task.

aklapper added a commit: rARCef73b12b580e: Fix "strpos(): Non-string needles will be interpreted as strings" in….

PHP 7.3: strpos(): Non-string needles will be interpreted as strings (in arcanist/src/utils/PhutilSortVector.php:33)Closed, ResolvedPublicActions

Description

Revisions and Commits

Event Timeline

PHP 7.3: strpos(): Non-string needles will be interpreted as strings (in arcanist/src/utils/PhutilSortVector.php:33)
Closed, ResolvedPublic
Actions