service: resolve relative __next URLs against the service root - #312
service: resolve relative __next URLs against the service root#312phanak-sap wants to merge 1 commit into
Conversation
OData v2 permits relative __next values. Previously these were passed verbatim into the cross-origin check, which compared an empty scheme/netloc against the service origin and always raised PyODataException. Now _build_request() detects a relative __next (no scheme, no netloc) and resolves it via urljoin(self._url, ...) before the origin check. Protocol- relative URLs (//host/...) are treated as absolute and validated as before. Tests added: - relative __next resolves to the correct absolute URL and dispatches - protocol-relative __next pointing to a foreign host is still rejected
|
This is a follow-up feature regression fix for #308. |
| resolved = urljoin(self._url, self._next_url) | ||
| parsed_next = urlparse(resolved) |
There was a problem hiding this comment.
I do not see the reason for not doing this:
url = urljoin(self._url, self._next_url)
the code below just checks the scheme and host and it comes from self._url.
| if self._next_url: | ||
| parsed_next = urlparse(self._next_url) | ||
| # OData v2 allows relative __next values; resolve them against the service root. | ||
| if not parsed_next.scheme and not parsed_next.netloc: |
There was a problem hiding this comment.
I would make the condition more strict. If the _next_url has scheme or netloc, then treat it as absolute URL and not just path.
|
Please provide links to OData V2 specifications of the relative next link. |
My current understanding is this:
If we agree on this being valid bugfix, I will address the other review comments. |
|
Please provide the evidence for your claim |
OK. No problem here closing directly as for odata v2 it is vague claim. |
OData v2 permits relative __next values. Previously these were passed verbatim into the cross-origin check, which compared an empty scheme/netloc against the service origin and always raised PyODataException.
Now _build_request() detects a relative __next (no scheme, no netloc) and resolves it via urljoin(self._url, ...) before the origin check. Protocol- relative URLs (//host/...) are treated as absolute and validated as before.
Tests added: