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

connection.py: BytesWarning: Comparison between bytes and string: if SKIP_HEADER not in values: #2071

Closed
jdufresne opened this issue Nov 18, 2020 · 7 comments · Fixed by #2141
Milestone

Comments

@jdufresne
Copy link
Member

Subject

Starting with the 1.26 series, I started receiving a BytesWarning in my project's test suite.

.../urllib3/connection.py:218: BytesWarning: Comparison between bytes and string
  if SKIP_HEADER not in values:

I run my test suite with Python's -b option to help catch misuses of bytes and string objects. My hunch is that urllib3 is sometimes treating headers as bytes and other times as string and then comparing them with each other.

Environment

Describe your environment.
At least, paste here the output of:

import platform
import urllib3

print("OS", platform.platform())
print("Python", platform.python_version())
print("urllib3", urllib3.__version__)
OS Linux-5.8.16-200.fc32.x86_64-x86_64-with-glibc2.2.5
Python 3.8.6
urllib3 1.26.2

Steps to Reproduce

This is reproducible using urllib3's own test suite:

Invoke using:

$ python -b -m pytest -W a test/with_dummyserver/test_socketlevel.py
...
test/with_dummyserver/test_socketlevel.py::TestProxyManager::test_connect_ipv6_addr
  /home/jon/.virtualenvs/urllib3/lib64/python3.9/site-packages/urllib3/connection.py:193: BytesWarning: Comparison between bytes and string
    if SKIP_HEADER not in values:

Expected Behavior

No ByteWarning emitted.

Actual Behavior

.../connection.py:193: BytesWarning: Comparison between bytes and string
    if SKIP_HEADER not in values:
@sethmlarson
Copy link
Member

Nice catch, we'll have to enable this in our test suite and remove the issue.

@The-Compiler
Copy link
Contributor

I'm seeing this as well FWIW - I did a quick bisect and this was introduced in 16b7b33 (#2018, "Add SKIP_HEADER for skipping automatically added headers").

The-Compiler added a commit to qutebrowser/qutebrowser that referenced this issue Nov 19, 2020
@sethmlarson sethmlarson added this to the v1.26 milestone Nov 20, 2020
@mgorny
Copy link
Contributor

mgorny commented Dec 30, 2020

Ping. This is causing a lot of cron spam for a lot of our users.

@fred-vogt
Copy link

Breaks python ... -bb ..., until this is fixed, can be worked around by downgrading in pip:

urllib3==1.25.11

@pquentin
Copy link
Member

@mgorny Can you please clarify what Gentoo cron spam is?

The issue here is that putheader accepts an array of value that are usually strings, but can also be bytes. For example, the standard library calls putheader("Host": encoded_netloc) in https://github.com/python/cpython/blob/v3.9.1/Lib/http/client.py#L1129-L1133. Opened #2141 to fix this.

@mgorny
Copy link
Contributor

mgorny commented Jan 25, 2021

@pquentin, I mean one of our most important tools happen to trigger this, and many users run it via cron in quiet mode. Now that Python suddenly started outputting this warning, quiet mode is no longer quiet and people are getting mails from cron that something is wrong.

@pquentin
Copy link
Member

This was released as part of urllib3 1.26.3. Sorry for the noise!

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

Successfully merging a pull request may close this issue.

6 participants