[issue2756] urllib2 add_header fails with existing unredirected_header

[Terry J. Reedy]

Terry J. Reedy  added the comment:

Bug needs to be verified on 3.10 or 3.11.

--
nosy: +terry.reedy
versions: +Python 3.10, Python 3.11 -Python 2.6, Python 2.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[Volodymyr Antonevych]

Volodymyr Antonevych added the comment:

Test urllib2 file

--
nosy: +volodyaa
Added file: http://bugs.python.org/file29705/1.py

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue2756
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[Volodymyr Antonevych]

Volodymyr Antonevych added the comment:

Test HTTP server

--
Added file: http://bugs.python.org/file29706/s.py

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue2756
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[Facundo Batista]

Facundo Batista facu...@taniquetil.com.ar added the comment:

Senthil, I'm assigning this issue to you because you know more about this 
subject than me. Feel free to unassign if will not be working in it.

Thanks!

--
assignee: facundobatista - orsenthil

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue2756
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[Daniel Diniz]

Changes by Daniel Diniz aja...@gmail.com:


--
keywords: +easy
priority:  - normal

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue2756
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[Daniel Diniz]

Changes by Daniel Diniz aja...@gmail.com:


--
stage:  - test needed
versions: +Python 2.6 -Python 2.5

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue2756
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[John J Lee]

John J Lee [EMAIL PROTECTED] added the comment:

I agree there is a bug here: clearly the methods on the Request class
are inconsistent with AbstractHTTPHandler.do_open() .  I think Facundo's
patch is good, though it needs a test.

The general principle when fixing earlier bugs has been that the
.add_*header() methods override default headers, though there are some
reasonable violations this where the implementation relies on specific
headers being sent -- e.g. Connection: close.

--
nosy: +jjlee

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2756
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[Facundo Batista]

Facundo Batista [EMAIL PROTECTED] added the comment:

What I think is that the AbstractHTTPHandler is grabbing the headers, in
the do_open() method, in an incorrect way.

Check the get_header() method from Request:

def get_header(self, header_name, default=None):
return self.headers.get(
header_name,
self.unredirected_hdrs.get(header_name, default))

What it's doing there is to grab the header from self.header, and if not
there use the one in self.unredirected_hdrs, and if not there return the
default.

So, to emulate this behaviour, in do_open() I just grabbed first the
unredirected headers, and then updated it with the normal ones.

See my simple patch I attach here, which solves the issue, and passes
all the tests also.

Added file: http://bugs.python.org/file11126/issue2756-facundo.diff

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2756
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[Senthil]

Senthil [EMAIL PROTECTED] added the comment:

The submitted patch has problems. It does not correctly solve this issue
(which I want to confirm, if there is issue at all after understanding
the logic behind unredirected_headers). My explanation of this issue and
comments on the patch is here:
http://urllib-gsoc.blogspot.com/2008/08/issue2756-urllib2-addheader-fails-with.html

Now, coming back to the current issue. We see that addition of
unredirected_hdrs takes place in the do_request_ call of
AbstractHTTPHandler and it adds the unredirected_hdrs based on certain
conditions, like when Content-Type is not there in header add the
unredirected header ('Content-Type','application/x-www-form-urlencoded')
The value of Content-Type is hardcoded here, but other header values are
not hardcoded and got from header request only.

Question here is: When the request contains the Content-Type header and
has a updated value, why is it not supposed to change the
unredirected_header to the updated value?  (Same for other request
header items).

John J Lee, can perhaps help us understand more.

If it is supposed to change, then the following snippet (rough) at
do_request_
for key, value in request.headers:
  request.add_unredirected_header(key,request.get_header(key))
should update it, there is no need to change the add_header and
add_unredirected_header method as proposed by the patch.

On our conclusion, I shall provide the updated patch (if required).

Thanks,
Senthil

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2756
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[Senthil]

Senthil [EMAIL PROTECTED] added the comment:

The problem with the patch was:

The attached patch modifies the add_header() and
add_unredirected_header() method to remove the existing headers of the
same name alternately in the headers and unredirected_hdrs.

What we observe is unredirected_hdrs item is removed during add_header()
calland it is never added back/updated in teh undirected_hdrs.

Let us discuss on the points mentioned in my previous post.

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2756
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[Facundo Batista]

Facundo Batista [EMAIL PROTECTED] added the comment:

BitTorment, what would increase the possibility of accepting this is a
patch also for the test suite (test_urllib2.py), checking this
behaviour, for us to be sure that does not break again in the future.

Could you please submit also this?

--
assignee:  - facundobatista
nosy: +facundobatista, orsenthil

___
Python tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2756
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[Martin McNickle]

Changes by Martin McNickle [EMAIL PROTECTED]:


Added file: http://bugs.python.org/file10280/add_header_complete.patch

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2756
__
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[Martin McNickle]

Changes by Martin McNickle [EMAIL PROTECTED]:


Removed file: http://bugs.python.org/file10201/add_header_complete.patch

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2756
__
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[Martin McNickle]

Martin McNickle [EMAIL PROTECTED] added the comment:

I decided that the user should have full control over the headers sent.
 Any call to add_header() or add_redirected_header() will now check if a
header with that same name has been set in the Request before (in both
the header and the unredirected header dictionaries), and if so
overwrite it.

The tests run fine with the patch installed, and also included is a
patch for the urllib2 documentation.

--
keywords: +patch
Added file: http://bugs.python.org/file10200/add_header_complete.patch

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2756
__
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[Martin McNickle]

Changes by Martin McNickle [EMAIL PROTECTED]:


Removed file: http://bugs.python.org/file10200/add_header_complete.patch

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2756
__
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[Martin McNickle]

Changes by Martin McNickle [EMAIL PROTECTED]:


Added file: http://bugs.python.org/file10201/add_header_complete.patch

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2756
__
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[Martin McNickle]

Martin McNickle [EMAIL PROTECTED] added the comment:

I can see that there will be a problem in this case.

However, it may be that the authors found it more intuitive to always
set the Content-Type to application/x-www-form-urlencoded when data is set.

Their implementation is inconsistent though:

#---
request = urllib2.Request('http://www.somesite.com/')
request.add_data(data)
f = urllib2.urlopen(request)

# 'Content-Type: application/x-www-form-urlencoded' is sent in the header
#---

whereas:

#---
request = urllib2.Request('http://www.somesite.com/')
request.add_header('Content-Type', 'application/xml')
request.add_data(data)

# 'Content-Type: application/xml' is sent in the header
#---

We need to decide what is the best way to handle this case.  Should the
normal headers be given preference, or should the unredirected headers
be given preference?

--- Martin

--
nosy: +BitTorment

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2756
__
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[Martin McNickle]

Martin McNickle [EMAIL PROTECTED] added the comment:

Sorry, the first example should read:

#---
request = urllib2.Request('http://www.whompbox.com/headertest.php')
request.add_data(data)
f = urllib2.urlopen(request)

request.add_header('Content-Type', 'application/xml')
f = urllib2.urlopen(request)

# 'Content-Type: application/x-www-form-urlencoded' is sent in both
headers.  Second header (should?) be 'application/xml'
#---

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2756
__
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2756] urllib2 add_header fails with existing unredirected_header

[david reid]

New submission from david reid [EMAIL PROTECTED]:

In urllib2 when using reusing a Request calling add_header doesn't work
when an unredirected_header has been added. 

A good example (and the one that caught me out) is content-type. When
making a POST request with no content-type set the current code sets the
content-type as an unredirected_header to
'application/x-www-form-urlencoded' (line 1034 of urllib2.py) but in a
subsequent request, setting the content type via add_header will see
this ignored as unredirected_headers are appended after the headers.

A possible solution is to check whether the header being added already
exists in the requests undredirected_headers and remove it if it does.

--
components: Library (Lib)
messages: 66213
nosy: zathras
severity: normal
status: open
title: urllib2 add_header fails with existing unredirected_header
type: behavior
versions: Python 2.5

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2756
__
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com