diff --git a/.github/workflows/python-linters.yml b/.github/workflows/python-linters.yml index 2b1a102..4180f26 100644 --- a/.github/workflows/python-linters.yml +++ b/.github/workflows/python-linters.yml @@ -16,7 +16,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.10.10"] + python-version: ["3.10"] lxml-version: ["4.9.1"] steps: diff --git a/.github/workflows/python-tests-compatibility.yml b/.github/workflows/python-tests-compatibility.yml index 888711c..5245d58 100644 --- a/.github/workflows/python-tests-compatibility.yml +++ b/.github/workflows/python-tests-compatibility.yml @@ -21,36 +21,56 @@ jobs: python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] lxml-version: ["4.6.5", "4.7.1", "4.8.0", "4.9.4", "5.0.1", "5.1.1", "5.2.2", "5.3.2", "5.4.0", "6.0.4", "6.1.1"] exclude: + # Excluded = no Linux CPython wheel published on PyPI for this + # (python, lxml) pair. This job installs wheels only (see the + # --only-binary flag below); without a wheel pip would fall back to + # compiling lxml from source against the runner's system libxml2, + # which tests the toolchain rather than pyodata. Derive these from + # actual PyPI wheel availability, NOT from lxml's python_requires. - python-version: 3.11 lxml-version: 4.6.5 - python-version: 3.11 - lxml-version: 4.7.1 + lxml-version: 4.7.1 - python-version: 3.11 - lxml-version: 4.8.0 + lxml-version: 4.8.0 - python-version: 3.12 lxml-version: 4.6.5 - python-version: 3.12 - lxml-version: 4.7.1 + lxml-version: 4.7.1 - python-version: 3.12 - lxml-version: 4.8.0 - - python-version: 3.12 - lxml-version: 4.9.4 + lxml-version: 4.8.0 - python-version: 3.13 lxml-version: 4.6.5 - python-version: 3.13 - lxml-version: 4.7.1 + lxml-version: 4.7.1 - python-version: 3.13 - lxml-version: 4.8.0 + lxml-version: 4.8.0 - python-version: 3.13 lxml-version: 4.9.4 + - python-version: 3.13 + lxml-version: 5.0.1 + - python-version: 3.13 + lxml-version: 5.1.1 + - python-version: 3.13 + lxml-version: 5.2.2 - python-version: 3.14 lxml-version: 4.6.5 - python-version: 3.14 - lxml-version: 4.7.1 + lxml-version: 4.7.1 - python-version: 3.14 - lxml-version: 4.8.0 + lxml-version: 4.8.0 - python-version: 3.14 lxml-version: 4.9.4 + - python-version: 3.14 + lxml-version: 5.0.1 + - python-version: 3.14 + lxml-version: 5.1.1 + - python-version: 3.14 + lxml-version: 5.2.2 + - python-version: 3.14 + lxml-version: 5.3.2 + - python-version: 3.14 + lxml-version: 5.4.0 steps: - uses: actions/checkout@v2 @@ -66,7 +86,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - python -m pip install lxml==${{ matrix.lxml-version }} + python -m pip install --only-binary lxml lxml==${{ matrix.lxml-version }} python -m pip install -r dev-requirements.txt python -m pip install -r requirements.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index f90c9f7..d8b2987 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Fixed + +- service: entity key values containing `/` are now percent-encoded as `%2F` in the request path, while structural slashes in navigation property paths stay unencoded (#282) + ## [1.12.2] diff --git a/pyodata/v2/service.py b/pyodata/v2/service.py index 5f88ea9..094f54f 100644 --- a/pyodata/v2/service.py +++ b/pyodata/v2/service.py @@ -32,6 +32,45 @@ def urljoin(*path): return '/'.join((part.strip('/') for part in path)) +def encode_entity_path(path): + """Percent-encode an OData resource path built from entity-set names and + entity keys. + + Structural slashes that separate path segments (e.g. the separator in + ``Employees(23)/Addresses``) are preserved, while slashes that appear + inside an entity key value (e.g. ``SoftwareComponents('/BAZ/FOO')``) are + encoded as ``%2F``. + + This is achieved by encoding the parts outside of the key parentheses + with ``safe='/'`` (keeping structural separators intact) and the parts + inside the key parentheses with ``safe=''`` (encoding every reserved + character, including slashes). + """ + + encoded = [] + depth = 0 + start = 0 + + for index, char in enumerate(path): + if char == '(': + if depth == 0: + encoded.append(quote(path[start:index], safe='/')) + start = index + depth += 1 + elif char == ')': + if depth > 0: + depth -= 1 + if depth == 0: + encoded.append(quote(path[start:index + 1], safe='')) + start = index + 1 + + # Encode the trailing part that is outside of any key parentheses. If the + # parentheses were unbalanced, the remaining key part keeps full encoding. + encoded.append(quote(path[start:], safe='' if depth > 0 else '/')) + + return ''.join(encoded) + + def encode_multipart(boundary, http_requests): """Encode list of requests into multipart body""" @@ -412,7 +451,7 @@ def expand(self, expand): def get_path(self): if self.get_encode_path(): - return quote(self._entity_set_proxy.last_segment + self._entity_key.to_key_string()) + return encode_entity_path(self._entity_set_proxy.last_segment + self._entity_key.to_key_string()) return self._entity_set_proxy.last_segment + self._entity_key.to_key_string() def get_default_headers(self): @@ -569,7 +608,7 @@ def __init__(self, url, connection, handler, entity_set, entity_key, encode_path def get_path(self): if self.get_encode_path(): - return quote(self._entity_set.name + self._entity_key.to_key_string()) + return encode_entity_path(self._entity_set.name + self._entity_key.to_key_string()) return self._entity_set.name + self._entity_key.to_key_string() def get_encode_path(self): @@ -613,7 +652,7 @@ def __init__(self, url, connection, handler, entity_set, entity_key, method="PAT def get_path(self): if self.get_encode_path(): - return quote(self._entity_set.name + self._entity_key.to_key_string()) + return encode_entity_path(self._entity_set.name + self._entity_key.to_key_string()) return self._entity_set.name + self._entity_key.to_key_string() def get_method(self): @@ -1367,7 +1406,7 @@ def filter(self, *args, group=False, **kwargs): def get_path(self): if self.get_encode_path(): - path = quote(self._last_segment) + path = encode_entity_path(self._last_segment) else: path = self._last_segment diff --git a/tests/test_service_v2.py b/tests/test_service_v2.py index 0e768b5..4181921 100644 --- a/tests/test_service_v2.py +++ b/tests/test_service_v2.py @@ -839,6 +839,73 @@ def test_delete_entity_http_error(service): assert caught_ex.value.response.status_code == 404 +# Issue #282 — slashes inside an entity key value must be percent-encoded as %2F, +# while structural slashes that separate path segments must stay unencoded. + +def test_get_entity_path_encodes_slash_in_key(service): + """GET: a slash in the key value is encoded as %2F (issue #282)""" + + # pylint: disable=redefined-outer-name + request = service.entity_sets.Cars.get_entity('/BAZ/FOO') + + assert request.get_path() == "Cars%28%27%2FBAZ%2FFOO%27%29" + + +def test_delete_entity_path_encodes_slash_in_key(service): + """DELETE: a slash in the key value is encoded as %2F (issue #282)""" + + # pylint: disable=redefined-outer-name + request = service.entity_sets.Cars.delete_entity('/BAZ/FOO') + + assert request.get_path() == "Cars%28%27%2FBAZ%2FFOO%27%29" + + +def test_update_entity_path_encodes_slash_in_key(service): + """PATCH/MODIFY: a slash in the key value is encoded as %2F (issue #282)""" + + # pylint: disable=redefined-outer-name + request = service.entity_sets.Cars.update_entity('/BAZ/FOO') + + assert request.get_path() == "Cars%28%27%2FBAZ%2FFOO%27%29" + + +def test_get_entities_path_encodes_slash_in_parent_key(service): + """GET entity set: a slash in a parent key value is encoded as %2F (issue #282)""" + + # pylint: disable=redefined-outer-name + request = service.entity_sets.Customers.get_entity('/BAZ/FOO').nav('Orders').get_entities() + + assert isinstance(request, pyodata.v2.service.GetEntitySetRequest) + assert request.get_path() == "Customers%28%27%2FBAZ%2FFOO%27%29/Orders" + + +def test_entity_path_keeps_structural_slash(service): + """Navigation property separator stays an unencoded slash (issue #282)""" + + # pylint: disable=redefined-outer-name + request = service.entity_sets.Cars.get_entity('/BAZ/FOO').nav('IDPic') + + assert request.get_path() == "Cars%28%27%2FBAZ%2FFOO%27%29/IDPic" + + +def test_entity_path_encode_path_false_bypasses_encoding(service): + """encode_path=False still produces a raw, unencoded path (regression guard)""" + + # pylint: disable=redefined-outer-name + request = service.entity_sets.Cars.get_entity('/BAZ/FOO', encode_path=False) + + assert request.get_path() == "Cars('/BAZ/FOO')" + + +def test_entity_path_encodes_other_special_characters_in_key(service): + """Other reserved characters in the key value are still encoded (issue #282)""" + + # pylint: disable=redefined-outer-name + request = service.entity_sets.Cars.get_entity('a/b?c&d') + + assert request.get_path() == "Cars%28%27a%2Fb%3Fc%26d%27%29" + + def test_update_entity_with_entity_key(service): """Make sure the method update_entity handles correctly the parameter key which is EntityKey"""