Removing a default header of a session #1920

Closed
miikka opened this Issue Feb 14, 2014 · 5 comments

Comments

Projects
None yet
3 participants
Contributor

miikka commented Feb 14, 2014

The docs say that you can prevent sending a session header by setting the headers value to None in the method's arguments. You would expect (as discussed on IRC) that this would work for session's default headers, too:

session = requests.Session()
# Do not send Accept-Encoding
session.headers['Accept-Encoding'] = None

What happens is that "None" gets sent as the value of header.

Accept-Encoding: None

For the reference, here is a way that works:

del session.headers['Accept-Encoding']
Owner

Lukasa commented Feb 14, 2014

We could do this, but I'm actually increasingly believing that the default headers dict is the right call here.

Owner

sigmavirus24 commented Feb 14, 2014

We could do this, but I'm actually increasingly believing that the default headers dict is the right call here.

I'm not sure what you're talking about.

Owner

Lukasa commented Feb 14, 2014

@sigmavirus24 Sorry, I had the context for this issue already. =)

Basically, we allow you to temporarily unset a header like this:

s = requests.Session()
s.get(url, headers={'Accept-Encoding': None})

But if you try to permanently unset a header on a Session in an analogous way, you get surprising behaviour:

s = requests.Session()
s.headers['Accept-Encoding'] = None
s.get(url)  # Sends the header "Accept-Encoding: None"

The question is, should we allow the example above to work, or should we just continue to use the del behaviour?

Owner

sigmavirus24 commented Feb 14, 2014

Actually, I think this is a bug in how we merge the headers before firing off a request. I'm going to send a PR in a few with a fix

@sigmavirus24 sigmavirus24 added a commit to sigmavirus24/requests that referenced this issue Feb 14, 2014

@sigmavirus24 sigmavirus24 Do not set headers with None value
- Regardless of whether they are on the session or not
- Fixes #1920
d2f647c
Owner

sigmavirus24 commented Feb 14, 2014

@Lukasa I think this is actually a regression in how we used to behave but such are the consequences when less tests are preferred to more tests. =P

kennethreitz closed this in #1921 Mar 3, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment