C and pure Python quoters disagree when % is in safe and requote is on
- Langage dominant
- Python
- Étoiles
- 1.5k
- Forks
- 215
- Merge moyen
- 1 j 2 min
- PR mergées (30 j)
- 13
Description
### Please confirm the following
- [x] I agree to follow the [aio-libs Code of Conduct][CoC]
[CoC]: ../../.github/blob/master/CODE_OF_CONDUCT.md
- [x] I have checked the [current issues][issue search] for duplicates.
[issue search]: ../search?q=is%3Aissue&type=issues
- [x] I understand this is open source software provided for free and that I might not receive a timely response.
- [x] I am positive I am **NOT** reporting a (potential) security
vulnerability, to the best of my knowledge. *(These must be shared by
submitting [this report form][vulnerability report form] instead, if
any hesitation exists.)*
[vulnerability report form]: ../security/advisories/new
- [x] I am willing to submit a pull request with reporoducers as xfailing test cases or even entire fix. *(Assign this issue to me.)*
### Describe the bug
`_Quoter` has two backends, the Cython `_quoting_c` and the pure Python
`_quoting_py`, and `yarl/_quoting.py` picks between them depending on the build
and on `YARL_NO_EXTENSIONS`. When `%` is included in `safe` and `requote` is
left at its default of `True`, the two backends return different strings.
The cause looks like the fast path in `_do_quote_or_skip`
(`yarl/_quoting_c.pyx`):
```cython
# If everything in the string is in the safe
# table and all ASCII, we can skip quoting
while idx:
idx -= 1
ch = PyUnicode_READ(kind, data, idx)
if ch >= 128 or not bit_at(self._safe_table, ch):
must_quote = 1
break
if not must_quote:
return val
```
The skip condition asks only whether every character is ASCII and in the safe
table; it does not take `_requote` into account. Putting `%` in `safe` sets that
bit, so a string like `%41` consists entirely of safe-table characters,
`must_quote` stays `0`, and `_do_quote` never runs. The pure Python backend has
no equivalent short-circuit, so it still performs the requote pass.
With `%` absent from `safe`, or with `requote=False`, the two backends agree.
I want to be clear about the scope: **this is not reachable through the public
API and I am not reporting it as a security issue.** `_Quoter` is private, none
of the quoters in `_quoters.py` put `%` in `safe`, and no test does either. You
may reasonably decide `safe="%"` is not a supported configuration and close
this.
The reason I thought it was still worth writing up is that
`PATH_SAFE_UNQUOTER = _Unquoter(ignore="/%", unsafe="+")` already exists, so a
quoter-side analogue that wants to leave `%` alone seems like a natural thing
for someone to add later. If that ever happened, the C and pure Python builds
would produce different output with nothing in the suite catching it, since the
existing parametrized tests never exercise that configuration.
### To Reproduce
```python
from yarl._quoting_c import _Quoter as CQuoter
from yarl._quoting_py import _Quoter as PyQuoter
for value in ["%41", "%4a", "%", "%zz", "%2f%2F"]:
c = CQuoter(safe="%")(value)
py = PyQuoter(safe="%")(value)
print(f"{value!r:10} C={c!r:10} py={py!r:10} {'DIFFER' if c != py else ''}")
```
### Expected behavior
The two backends return the same string for the same input and constructor
arguments, as they do for every other configuration I tried.
I do not have a view on which of the two is the correct answer here.
`safe="%"` and `requote=True` pull in opposite directions, one asking for `%` to
be left alone and the other asking for `%XX` to be interpreted, so the intended
resolution is a decision for you rather than something I can read off the code.
### Logs/tracebacks
```python-traceback
No exception is raised. The output of the reproducer above:
'%41' C='%41' py='A' DIFFER
'%4a' C='%4a' py='J' DIFFER
'%' C='%' py='%25' DIFFER
'%zz' C='%zz' py='%25zz' DIFFER
'%2f%2F' C='%2f%2F' py='%2F%2F' DIFFER
With `safe=""` the same script prints no `DIFFER` rows:
'%41' C='A' py='A'
'%4a' C='J' py='J'
'%' C='%25' py='%25'
'%zz' C='%25zz' py='%25zz'
'%2f%2F' C='%2F%2F' py='%2F%2F'
`safe="@:"` and `requote=False` behave like the `safe=""` control above.
For what it is worth, I compared the two backends over a wider space while
looking at this: percent-escape edge cases, the code point range in bands
including surrogates and astral characters, and a sweep of the constructor
arguments. Every divergence found was this one case, `%` in `safe` with
`requote=True`. I did not find any divergence in the unquoters.
```
### Python Version
```console
$ python -V
Python 3.13.13
Observed in a container built from the official `python:3.13-slim` image.
```
### multidict Version
```console
Not installed. The reproducer imports only yarl._quoting_c and yarl._quoting_py,
so multidict was not required.
```
### propcache Version
```console
Not installed, for the same reason as multidict
```
### yarl Version
```console
Built from master at 06abbc731ac269df9ba6ed36e6836e173b873dd6, with the Cython accelerator compiled, so that both backends could be imported in one process.
```
### OS
Debian GNU/Linux 13 (trixie), x86_64, inside the official python:3.13-slim container image.
### Additional context
Nothing about this looks platform specific: the difference is in which code path
runs, not in anything the platform decides. I have only actually run it on the
one machine above, though, so that is an inference from reading rather than
something I have measured across systems.
No proxy or network is involved; the reproducer only imports the two quoting
modules and calls them directly.
Guide de contribution
Ouvrir le guide de contribution
Évaluation
Cette issue n'a pas encore été évaluée.