DOMjudge / domjudge

DOMjudge programming contest jury system
https://www.domjudge.org
GNU General Public License v2.0
720 stars 254 forks source link

Shadowing with a relative URL has errors #2378

Open vmcj opened 6 months ago

vmcj commented 6 months ago

When we shadow PC^2 they use a relative URL starting with a /, the baseURL we use ends with a / which leads to problems.

We tried by replacing the / and doing a concat but this is ugly.

_Originally posted by @eldering in https://github.com/DOMjudge/domjudge/pull/2371#discussion_r1527514117_

nickygerritsen commented 6 months ago

The question actually is what we define as 'relative URL' in the spec.

The RFC linked to in the specs saysL

A relative reference that begins with two slash characters is termed a network-path reference; such references are rarely used. A relative reference that begins with a single slash character is termed an absolute-path reference. A relative reference that does not begin with a slash character is termed a relative-path reference.

So if the URL starts with a /, is it relative to the baseurl or not? In PC^2 it currently seems to be, but I'm not sure if that is what it intended in the spec.

eldering commented 6 months ago

If the base url is https://example.org/foobar/api and the relative url is /bla, then the target URL is https://example.org/bla as I understand it, while if the relative URL is bla, then the target becomes https://example.org/foobar/api/bla.

Also, since a proper implementation of relative URLs per https://www.rfc-editor.org/rfc/rfc3986 seems not completely trivial, we should use a library for this.

nickygerritsen commented 6 months ago

If the base url is https://example.org/foobar/api and the relative url is /bla, then the target URL is https://example.org/bla as I understand it, while if the relative URL is bla, then the target becomes https://example.org/foobar/api/bla.

Also, since a proper implementation of relative URLs per rfc-editor.org/rfc/rfc3986 seems not completely trivial, we should use a library for this.

It seems league/uri is the most popular one.

I tried it, and it seems to adhere to the spec. However, the spec has something odd related to merging a base uri and relative-path, which this library follows. Consider this code:

        $uri = \League\Uri\Uri::new('https://www.domjudge.org/demoweb/api');
        dump(\League\Uri\Uri::fromBaseUri('/test', $uri->toString())->toString());
        dump(\League\Uri\Uri::fromBaseUri('test', $uri->toString())->toString());
        $uri = \League\Uri\Uri::new('https://www.domjudge.org/demoweb/api/');
        dump(\League\Uri\Uri::fromBaseUri('/test', $uri->toString())->toString());
        dump(\League\Uri\Uri::fromBaseUri('test', $uri->toString())->toString());

This results in:

"https://www.domjudge.org/test"
"https://www.domjudge.org/demoweb/test"
"https://www.domjudge.org/test"
"https://www.domjudge.org/demoweb/api/test"

which is correct if you read the spec, since everything after the last / in the base URI needs to be dropped.

However, this seems to be 'incompatible' with what the spec says about base URI's, since they do not end in a /. So should we change this in the spec?