Repository navigation
service: percent-encode slashes inside entity key values - #331
Open
phanak-sap wants to merge 2 commits into
Open
phanak-sap wants to merge 2 commits into
phanak-sap wants to merge 2 commits into
Conversation
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
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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