Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comparison between bytes and string in defining a frozenset throws exception #1236

Open
chaofanhan opened this issue Aug 6, 2020 · 4 comments
Labels

Comments

@chaofanhan
Copy link

https://github.com/python-hyper/hyper-h2/blob/3b0b241d79f5a9ff9382bbc038f84862e0d80abf/src/h2/utilities.py#L20-L26.

Hi, when a python process runs with a flag -bb, the above part of code will throw exception and make h2 not work. May I ask why we define both bytes and string in the frozenset? Is it possible to use only bytes or string? Frozenset will compare keys for deduplication.

@stroeder
Copy link

stroeder commented Feb 7, 2022

I agree this should be fixed, especially this module is used in many other stacks. Sets with mixed string types seem totally broken to me.

Maybe a custom set-class instead of frozenset?

@stroeder
Copy link

stroeder commented Feb 7, 2022

I tried to fix this but I give up for now due to lack of time. This seems really seriously broken! This whole module package serves as a good example why you should have typing.

IMO the devs have to decide where in the call-stack to decode the lower protocol data and refactor everything else above that. Especially remove the really strange kludges like h2.utility._custom_startswith().

@straz
Copy link

straz commented Jan 5, 2023

Even without -bb, this throws noisy warnings:

[BytesWarning] Comparison between bytes and string (.../h2/utilities.py:20)
[BytesWarning] Comparison between bytes and string (.../h2/utilities.py:29)
[BytesWarning] Comparison between bytes and string (.../h2/utilities.py:39)
[BytesWarning] Comparison between bytes and string (.../h2/utilities.py:46)
[BytesWarning] Comparison between bytes and string (.../h2/utilities.py:55)
[BytesWarning] Comparison between bytes and string (.../h2/utilities.py:60)

These warnings should at least be less noisy. Perhaps using warnings.simplefilter? reference

@pquentin
Copy link

This also affects urllib3 who runs tests with -bb to catch issues, but will have to stop to adopt h2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants