From 46772ec39a813d6dc5bf02edb24a169f8a39856c Mon Sep 17 00:00:00 2001 From: I335851 Date: Sun, 15 Nov 2026 10:29:11 +0100 Subject: [PATCH 1/2] service: percent-encode slashes inside entity key values The problem and fix: urllib.parse.quote() defaults to safe='/', so a slash inside a key value (e.g. SoftwareComponents('/BAZ/FOO')) was left unencoded in the request path. The entity-set name and key string were quoted as one string, making it impossible to encode key-value slashes while preserving the structural slashes that separate navigation segments. Add encode_entity_path(), a parenthesis-depth-aware encoder: text outside the key predicate is quoted with safe='/' (structural separators kept), text inside (...) with safe='' (every reserved char, including /, encoded). Output is byte-identical to the old quote() for keys without slashes, so no existing path changes. Applied to EntityGetRequest, EntityDeleteRequest, EntityModifyRequest and GetEntitySetRequest; batch inherits it via get_path(). encode_path=False still bypasses encoding unchanged. Additional notes: This is outbound-only by design and has no from_literal-style counterpart: encode_entity_path runs solely in get_path() to build request URLs. pyodata never decodes a key back out of a path (the server does the percent-decoding; the __next link is reused verbatim), so there is no reverse direction to keep symmetric. The encode_path=False workaround from PR #243 is intentionally left in place for backward compatibility: callers who pre-encode keys and disable encoding must keep working unchanged, and the flag also serves as a raw-path override for non-conformant servers. Removing it is deferred to a future major release. Fixes #282 --- .github/workflows/python-linters.yml | 2 +- CHANGELOG.md | 4 ++ pyodata/v2/service.py | 47 +++++++++++++++++-- tests/test_service_v2.py | 67 ++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 5 deletions(-) diff --git a/.github/workflows/python-linters.yml b/.github/workflows/python-linters.yml index 2b1a1020..4180f263 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/CHANGELOG.md b/CHANGELOG.md index f90c9f7a..d8b29871 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 5f88ea92..094f54fc 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 0e768b52..41819211 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""" From 774b8f97a3e05b8c44786788388f523df09daf1d Mon Sep 17 00:00:00 2001 From: I335851 Date: Sun, 15 Nov 2026 11:29:54 +0100 Subject: [PATCH 2/2] ci: update lxml compatibility matrix from wheel availability The compatibility matrix is a gate: every (python, lxml) cell must pass, and a red cell should mean pyodata itself broke against that pair. That assumption only holds if `pip install lxml==X` drops in a prebuilt wheel so pyodata is the only variable under test. The exclude list was hand-derived from lxml's declared python_requires, not from actual wheel availability. These are different facts: a release can support a Python version yet never publish a binary wheel for it. On the Ubuntu 26.04 runner (libxml2 2.15 / GCC 14) the no-wheel cells fell back to compiling lxml from source, and the older pre-generated etree.c failed to build (-Wincompatible-pointer-types). That surfaced as a compatibility failure when it was really a packaging/toolchain failure before pyodata ran at all. Rebuild the matrix from PyPI wheel availability instead: - Exclude every pair with no published Linux CPython wheel: add 3.13 x {5.0.1, 5.1.1, 5.2.2} and 3.14 x {5.0.1, 5.1.1, 5.2.2, 5.3.2, 5.4.0}. - Install with `--only-binary lxml` so a missing wheel fails fast with a clear "no matching distribution" instead of silently compiling. - Drop the erroneous 3.12/4.9.4 exclusion: 4.9.4 has a cp312 wheel and was 3.12's intended test target. It became excluded in 08e93e3 when a dangling 4.9.1 exclude entry was retargeted to 4.9.4 rather than removed. - Document the rule in the exclude block so it isn't regenerated from python_requires again. Every surviving cell now installs a real wheel, so the gate tests pyodata rather than the runner's C toolchain. --- .../workflows/python-tests-compatibility.yml | 42 ++++++++++++++----- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/.github/workflows/python-tests-compatibility.yml b/.github/workflows/python-tests-compatibility.yml index 888711c4..5245d581 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