Page MenuHomePhabricator

Replacing wfRemoveDotSegments() with GuzzleHttp\Psr7\UriResolver::removeDotSegments()
Open, Needs TriagePublic

Description

https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/477964/ attempts to replace wfRemoveDotSegments() with GuzzleHttp\Psr7\UriResolver::removeDotSegments() now we have access to Guzzle after T202110, but we get some test failures :(

We should work out which cases upstream (Guzzle) should actually be handling and report them as such. Or remove the test cases if it's just weird edge cases we shouldn't be supporting

17:25:24 There were 9 failures:
17:25:24 
17:25:24 1) WfRemoveDotSegmentsTest::testWfRemoveDotSegments with data set #41 ('b/../../a', '/a')
17:25:24 Testing b/../../a expands to /a
17:25:24 Failed asserting that two strings are equal.
17:25:24 --- Expected
17:25:24 +++ Actual
17:25:24 @@ @@
17:25:24 -'/a'
17:25:24 +'a'
17:25:24 
17:25:24 /workspace/src/tests/phpunit/includes/GlobalFunctions/wfRemoveDotSegmentsTest.php:15
17:25:24 /workspace/src/tests/phpunit/MediaWikiTestCase.php:424
17:25:24 /workspace/src/maintenance/doMaintenance.php:94
17:25:24 
17:25:24 2) WfRemoveDotSegmentsTest::testWfRemoveDotSegments with data set #42 ('b/.././a', '/a')
17:25:24 Testing b/.././a expands to /a
17:25:24 Failed asserting that two strings are equal.
17:25:24 --- Expected
17:25:24 +++ Actual
17:25:24 @@ @@
17:25:24 -'/a'
17:25:24 +'a'
17:25:24 
17:25:24 /workspace/src/tests/phpunit/includes/GlobalFunctions/wfRemoveDotSegmentsTest.php:15
17:25:24 /workspace/src/tests/phpunit/MediaWikiTestCase.php:424
17:25:24 /workspace/src/maintenance/doMaintenance.php:94
17:25:24 
17:25:24 3) WfRemoveDotSegmentsTest::testWfRemoveDotSegments with data set #43 ('b/./../a', '/a')
17:25:24 Testing b/./../a expands to /a
17:25:24 Failed asserting that two strings are equal.
17:25:24 --- Expected
17:25:24 +++ Actual
17:25:24 @@ @@
17:25:24 -'/a'
17:25:24 +'a'
17:25:24 
17:25:24 /workspace/src/tests/phpunit/includes/GlobalFunctions/wfRemoveDotSegmentsTest.php:15
17:25:24 /workspace/src/tests/phpunit/MediaWikiTestCase.php:424
17:25:24 /workspace/src/maintenance/doMaintenance.php:94
17:25:24 
17:25:24 4) WfRemoveDotSegmentsTest::testWfRemoveDotSegments with data set #45 ('b/../../', '/')
17:25:24 Testing b/../../ expands to /
17:25:24 Failed asserting that two strings are equal.
17:25:24 --- Expected
17:25:24 +++ Actual
17:25:24 @@ @@
17:25:24 -'/'
17:25:24 +''
17:25:24 
17:25:24 /workspace/src/tests/phpunit/includes/GlobalFunctions/wfRemoveDotSegmentsTest.php:15
17:25:24 /workspace/src/tests/phpunit/MediaWikiTestCase.php:424
17:25:24 /workspace/src/maintenance/doMaintenance.php:94
17:25:24 
17:25:24 5) WfRemoveDotSegmentsTest::testWfRemoveDotSegments with data set #46 ('b/.././', '/')
17:25:24 Testing b/.././ expands to /
17:25:24 Failed asserting that two strings are equal.
17:25:24 --- Expected
17:25:24 +++ Actual
17:25:24 @@ @@
17:25:24 -'/'
17:25:24 +''
17:25:24 
17:25:24 /workspace/src/tests/phpunit/includes/GlobalFunctions/wfRemoveDotSegmentsTest.php:15
17:25:24 /workspace/src/tests/phpunit/MediaWikiTestCase.php:424
17:25:24 /workspace/src/maintenance/doMaintenance.php:94
17:25:24 
17:25:24 6) WfRemoveDotSegmentsTest::testWfRemoveDotSegments with data set #47 ('b/./../', '/')
17:25:24 Testing b/./../ expands to /
17:25:24 Failed asserting that two strings are equal.
17:25:24 --- Expected
17:25:24 +++ Actual
17:25:24 @@ @@
17:25:24 -'/'
17:25:24 +''
17:25:24 
17:25:24 /workspace/src/tests/phpunit/includes/GlobalFunctions/wfRemoveDotSegmentsTest.php:15
17:25:24 /workspace/src/tests/phpunit/MediaWikiTestCase.php:424
17:25:24 /workspace/src/maintenance/doMaintenance.php:94
17:25:24 
17:25:24 7) WfRemoveDotSegmentsTest::testWfRemoveDotSegments with data set #49 ('b/../..', '/')
17:25:24 Testing b/../.. expands to /
17:25:24 Failed asserting that two strings are equal.
17:25:24 --- Expected
17:25:24 +++ Actual
17:25:24 @@ @@
17:25:24 -'/'
17:25:24 +''
17:25:24 
17:25:24 /workspace/src/tests/phpunit/includes/GlobalFunctions/wfRemoveDotSegmentsTest.php:15
17:25:24 /workspace/src/tests/phpunit/MediaWikiTestCase.php:424
17:25:24 /workspace/src/maintenance/doMaintenance.php:94
17:25:24 
17:25:24 8) WfRemoveDotSegmentsTest::testWfRemoveDotSegments with data set #50 ('b/../.', '/')
17:25:24 Testing b/../. expands to /
17:25:24 Failed asserting that two strings are equal.
17:25:24 --- Expected
17:25:24 +++ Actual
17:25:24 @@ @@
17:25:24 -'/'
17:25:24 +''
17:25:24 
17:25:24 /workspace/src/tests/phpunit/includes/GlobalFunctions/wfRemoveDotSegmentsTest.php:15
17:25:24 /workspace/src/tests/phpunit/MediaWikiTestCase.php:424
17:25:24 /workspace/src/maintenance/doMaintenance.php:94
17:25:24 
17:25:24 9) WfRemoveDotSegmentsTest::testWfRemoveDotSegments with data set #51 ('b/./..', '/')
17:25:24 Testing b/./.. expands to /
17:25:24 Failed asserting that two strings are equal.
17:25:24 --- Expected
17:25:24 +++ Actual
17:25:24 @@ @@
17:25:24 -'/'
17:25:24 +''
17:25:24 
17:25:24 /workspace/src/tests/phpunit/includes/GlobalFunctions/wfRemoveDotSegmentsTest.php:15
17:25:24 /workspace/src/tests/phpunit/MediaWikiTestCase.php:424
17:25:24 /workspace/src/maintenance/doMaintenance.php:94

Event Timeline

Reedy created this task.Feb 11 2019, 4:42 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 11 2019, 4:42 AM
Reedy updated the task description. (Show Details)Feb 11 2019, 4:42 AM
Krinkle added a subscriber: Krinkle.EditedOct 9 2019, 1:29 AM

I mentioned on Gerrit a while back: Might want to check whether the upstream GuzzleHttp actually intends to support handling file-system paths.

believe our function is meant to handle only that (it's use for url.path is more or less accidental, I think). Offloading its code to a URI library seems suspicious at first given the file vs url conflict. E.g. would it be appropriate for most consumers of this function to link against Guzzle or would it be odd (odd if they're dealing with file paths, could perhaps be hosted somewhere in our filebackend lib, though).