Skip to content

Discard parent segments that climb past the root - #952

Open
Amoifr wants to merge 1 commit into
jsonrainbow:mainfrom
Amoifr:fix-950-uri-resolver-parent-segments
Open

Discard parent segments that climb past the root#952
Amoifr wants to merge 1 commit into
jsonrainbow:mainfrom
Amoifr:fix-950-uri-resolver-parent-segments

Conversation

@Amoifr

@Amoifr Amoifr commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #950.

combineRelativePathWithBasePath() threw when a .. segment could not be popped. RFC 3986 section 5.2.4 discards those instead, so it now does.

The guard had a second problem beyond the one in the issue: count($collapsedSegments) <= 1 also refused to pop a real first segment, so ../bar.json against a relative base foo/baz.json threw rather than resolving to bar.json. The condition now protects only what it meant to, the leading empty segment that stands for the root of an absolute path.

One thing that needs your call. UriRetrieverTest::testResolveExcessLevelUp asserted the exception this change removes:

$this->expectException(UriResolverException::class);
$retriever->resolve('../schema.json#', 'http://example.org/schema.json#');

Your acceptance list mentions UriResolverTest, so I suspect this one was not on the radar. I turned it into an assertion on the resolved value (http://example.org/schema.json) rather than deleting it, but it is a deliberate behaviour change and it is yours to confirm.

Note this does not on its own make the exact example from the issue work: resolve('../bar.json', 'internal://mySchema') still stops earlier, on the empty base path, until #947 lands. The two are independent, as you noted, and the .. handling is what this PR covers.

Testing

Four new cases, all failing without the change.

Full suite green (3185 tests), same warning and skips as main, and PHPStan reports no errors.

On the measurement you asked for, with shouldNotYieldTest() disabled: 7037 tests, 30 errors, 550 failures, identical before and after. So this neither fixes nor breaks anything in the official suite. One caveat on that run: the vendored suite here has a v1 directory that is not in $skippedDrafts, which makes the data provider throw v1 is not a valid constraint name (that is the single warning already present on main). I excluded it for the measurement only, nothing of that is in this PR.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUCMYykYqJHv67fmwiWZip
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UriResolver::resolve() throws on a parent-relative reference against a base with no path

1 participant