Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion pyodata/v2/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,13 +297,19 @@ def add_headers(self, value):
def _build_request(self):
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:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

resolved = urljoin(self._url, self._next_url)
parsed_next = urlparse(resolved)
Comment on lines +302 to +303

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

else:
resolved = self._next_url
parsed_base = urlparse(self._url)
if (parsed_next.scheme, parsed_next.netloc) != (parsed_base.scheme, parsed_base.netloc):
raise PyODataException(
f'cross-origin __next URL rejected: {self._next_url!r} differs from '
f'service root {self._url!r}'
)
url = self._next_url
url = resolved
else:
url = urljoin(self._url, self.get_path())
# pylint: disable=assignment-from-none
Expand Down
36 changes: 36 additions & 0 deletions tests/test_service_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -2463,6 +2463,42 @@ def test_next_url_same_origin_allowed(service):
assert result[0].ID == 23


@responses.activate
def test_next_url_relative_resolved_against_service_root(service):
"""Relative __next value (valid OData v2) is resolved against the service root before dispatch."""
# pylint: disable=redefined-outer-name
relative_next = "Employees?$skiptoken='opaque'"
expected_url = f"{service.url}/Employees?$skiptoken='opaque'"

responses.add(
responses.GET,
expected_url,
json={'d': {
'results': [
{'ID': 42, 'NameFirst': 'Jane', 'NameLast': 'Doe'}
]
}},
status=200)

request = service.entity_sets.Employees.get_entities().next_url(relative_next)
result = request.execute()
assert len(result) == 1
assert result[0].ID == 42
assert responses.calls[0].request.url == expected_url


def test_next_url_relative_cross_origin_raises(service):
"""Relative __next that resolves to a different origin (e.g. protocol-relative) must be refused."""
# pylint: disable=redefined-outer-name
# A protocol-relative URL has no scheme but does have a netloc — treated as absolute origin check.
cross_origin_relative = "//attacker.example.com/Employees?$skiptoken=x"

request = service.entity_sets.Employees.get_entities().next_url(cross_origin_relative)
with pytest.raises(PyODataException, match="cross-origin"):
request.execute()
assert len(responses.calls) == 0


@responses.activate
def test_count_with_chainable_filter_lt_operator(service):
"""Check getting $count with $filter with new filter syntax using multiple filters"""
Expand Down
Loading