[issue15799] httplib client and statusline

[karl]

New submission from karl:

The current parsing of HTTP status line seems strange with regards to its 
definition in HTTP.

http://hg.python.org/cpython/file/3.2/Lib/http/client.py#l307

Currently the code is 
version, status, reason = line.split(None, 2)

>>> status1 = "HTTP/1.1 200 OK"
>>> status2 = "HTTP/1.1 200 "
>>> status3 = "HTTP/1.1 200"
>>> status1.split(None, 2)
['HTTP/1.1', '200', 'OK']
>>> status2.split(None, 2)
['HTTP/1.1', '200']
>>> status3.split(None, 2)
['HTTP/1.1', '200']

According to the production rules of HTTP/1.1 bis only status1 and status2 are 
valid.

  status-line = HTTP-version SP status-code SP reason-phrase CRLF
  — http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-20#section-3.1.2

with reason-phrase  = *( HTAB / SP / VCHAR / obs-text ) aka 0 or more 
characters.

I'm also not sure what are the expected ValueError with additional parsing 
rules which seems even more bogus.

First modification should be

>>> status1.split(' ', 2)
['HTTP/1.1', '200', 'OK']
>>> status2.split(' ', 2)
['HTTP/1.1', '200', '']

Which would be correct for the first two, with an empty reason-phrase
The third one is still no good.

>>> status3.split(' ', 2)
['HTTP/1.1', '200']

An additional check could be done with 

len(status.split(' ', 2)) == 3

Will return False in the third case.

Do you want me to create a patch and a test for it?

--
messages: 169293
nosy: karlcow
priority: normal
severity: normal
status: open
title: httplib client and statusline
type: enhancement
versions: Python 3.2

___
Python tracker 

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

[issue15799] httplib client and statusline

[R. David Murray]

R. David Murray added the comment:

Could you explain the error you are seeing in more detail first?  You are talk 
about parsing and fixes here, but I'm not clear on what the actual bug is you 
are reporting.

--
nosy: +r.david.murray

___
Python tracker 

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

[issue15799] httplib client and statusline

[karl]

karl added the comment:

ok.

status lines 1 and 2 are valid. 
the third one is invalid and should trigger a 

raise BadStatusLine(line)

The code at line 318 is bogus as it will parse happily the third line without 
raising an exception.
http://hg.python.org/cpython/file/3.2/Lib/http/client.py#l318

--

___
Python tracker 

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

[issue15799] httplib client and statusline

[R. David Murray]

R. David Murray added the comment:

Why should it raise an error?  The postel principle suggests that we should 
treat it as equivalent to the second line.

--

___
Python tracker 

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

[issue15799] httplib client and statusline

[Antoine Pitrou]

Antoine Pitrou added the comment:

I would point out that the goal of the http module is to provide 
interoperability with real-life servers, rather than be a strict implementation 
of RFCs. So, starting to raise errors for "HTTP/1.1 200" while "HTTP/1.1 200 " 
remains ok might not be the best idea.

--
nosy: +orsenthil, pitrou

___
Python tracker 

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

[issue15799] httplib client and statusline

[karl]

karl added the comment:

Fair enough, it could be a warning when

* more than one space in between http version and status code
* if there is a missing space after the status code


I'm not advocating for being strict only. I'm advocating for giving the tools 
to developer to assess that things are right and choose or not to ignore and 
having to avoid to patch the libraries or rewrite modules when you create code 
which needs to be strict for specifically validating responses and requests. :)

ps: I haven't checked yet if the server counter part of httplib was strict in 
the production rule.

--

___
Python tracker 

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

[issue15799] httplib client and statusline

[karl]

karl added the comment:

So what do we do with it? 
Do I created a patch or do we close that bug? :)
No hard feelings about it.

--

___
Python tracker 

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

[issue15799] httplib client and statusline

[Antoine Pitrou]

Antoine Pitrou added the comment:

Well, I'd like to hear Senthil's opinion, since he's the main http / urllib 
maintainer these days.

--
versions: +Python 3.3 -Python 3.2

___
Python tracker 

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

[issue15799] httplib client and statusline

[R. David Murray]

R. David Murray added the comment:

I'm inclined to think it isn't worth the effort, myself.  I think a 
"validating" client would be a valuable tool, but that that isn't what the 
stdlib is focused on.  But yes, let's hear Senthil's opinion.

(That said, I am in fact adding a lot of validation capabilities to the email 
package...but in general those are only a matter of exposing the information 
about parsing failures that the code then has to recover from, rather than 
adding extra parsing to detect non-compliance.)

--
versions: +Python 3.2 -Python 3.3

___
Python tracker 

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

[issue15799] httplib client and statusline

[karl]

karl added the comment:

Let's close this.

>>> "HTTP/1.1301 ".split(None, 2)
['HTTP/1.1', '301']
>>> "HTTP/1.1301 ".split(' ', 2)
['HTTP/1.1', '', '  301 ']

I think it would be nice to have a way to warn without stopping, but the last 
comment from r.david.murray makes sense too. :)

--
resolution:  -> not a bug
status: open -> closed

___
Python tracker 

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

[issue15799] httplib client and statusline

[R. David Murray]

Changes by R. David Murray :


--
stage:  -> resolved

___
Python tracker 

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

[issue15799] httplib client and statusline

[Senthil Kumaran]

Senthil Kumaran added the comment:

Sorry that I did not get involved earlier.

It is difficult to prove any problem with the current behavior and it is 
rightly closed. The issue which was originally raised seems to me a cosmetic 
one, which won't get exhibited as well.

Here is  simple test case and the output with the current behavior.

# testcases.py

testcases = ["HTTP/1.1 200", "HTTP/1.1 200 OK", "HTTP/1.1 200  ", "HTTP/1.1   
200"]
for tc in testcases:
try:
version, status, reason = tc.split(None, 2)
print "%s (%s,%s,%s)" % (tc, version, status, reason)
except ValueError:
version, status = tc.split(None, 1)
print "%s (%s, %s)" % (tc, version, status)


$ python testcases.py
HTTP/1.1 200 (HTTP/1.1, 200)
HTTP/1.1 200 OK (HTTP/1.1,200,OK)
HTTP/1.1 200   (HTTP/1.1, 200  )
HTTP/1.1   200 (HTTP/1.1, 200)

The problem is the status code (str at the moment) has a trailing spaces. 
And now, the status code is not used as string. Just after the parsing, the 
status is converted to integer, Line 337: status = int(status) and rest of 
urllib and http/client's behavior use status as int rather than as string.

Thanks!

--
type: enhancement -> behavior

___
Python tracker 

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