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

Http client, Bad Status Line triggered for no reason #86598

Closed
sicarius mannequin opened this issue Nov 22, 2020 · 9 comments
Closed

Http client, Bad Status Line triggered for no reason #86598

sicarius mannequin opened this issue Nov 22, 2020 · 9 comments
Labels
3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@sicarius
Copy link
Mannequin

sicarius mannequin commented Nov 22, 2020

BPO 42432
Nosy @ericvsmith, @tiran

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2020-11-23.09:29:12.860>
created_at = <Date 2020-11-22.09:59:54.229>
labels = ['type-bug', 'library', '3.9']
title = 'Http client, Bad Status Line triggered for no reason'
updated_at = <Date 2020-11-23.09:29:12.859>
user = 'https://bugs.python.org/sicarius'

bugs.python.org fields:

activity = <Date 2020-11-23.09:29:12.859>
actor = 'sicarius'
assignee = 'none'
closed = True
closed_date = <Date 2020-11-23.09:29:12.860>
closer = 'sicarius'
components = ['Library (Lib)']
creation = <Date 2020-11-22.09:59:54.229>
creator = 'sicarius'
dependencies = []
files = []
hgrepos = []
issue_num = 42432
keywords = []
message_count = 9.0
messages = ['381606', '381638', '381645', '381646', '381648', '381649', '381651', '381658', '381659']
nosy_count = 3.0
nosy_names = ['eric.smith', 'christian.heimes', 'sicarius']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue42432'
versions = ['Python 3.9']

@sicarius
Copy link
Mannequin Author

sicarius mannequin commented Nov 22, 2020

Hey,
BadStatusLine triggered when protocol version is in lowercase.
I've encountered a server that answers "Http/1.0 404 Not Found\r\n" instead of "HTTP/1.0 404 Not Found\r\n"

## Expected Result

Requests understanding the status line.

## Actual Result

Requests is closing the connection.

## Reproduction Steps
### Setup a server that answers the line above
bash: while 1;do echo "Http/1.0 404 Not Found\r\n" | sudo nc -lnvp 80; done
### get the server

import requests
req = req = requests.get("http://127.0.0.1/", verify=False, allow_redir=False )

## problem location
Look at line 287 of http/client.py
the word "HTTP" should be matched in lowercase too.

if not version.startswith("HTTP/"):```

Regards.

@sicarius sicarius mannequin added 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Nov 22, 2020
@ericvsmith
Copy link
Member

requests is a third-party library, and this isn't the right place to report issues with it. It looks like the requests issue tracker is at https://github.com/psf/requests/issues

If you can duplicate this problem with only using the python standard library, then please let us know and we'll take a look. I'd have to do some research through the standards to determine if the problem is really with your server, though.

@sicarius
Copy link
Mannequin Author

sicarius mannequin commented Nov 23, 2020

Hi,

Here I'm using requests to show the behavior because it relies on python's http lib, and it is faster/simplier to use. The Exception "BadStatusLine" is a part or the http/client.py library.

As per the RFC2616 section 6.1 https://tools.ietf.org/html/rfc2616#section-6.1, there's nothing specifying that the HTTP verb must be uppercase, I think it's more a matter of "common sense". I might have missed something though!

@sicarius
Copy link
Mannequin Author

sicarius mannequin commented Nov 23, 2020

Here is the poc for this error with http.client only (for the server: use the same nc command as my first message): ```python
import http.client

h1 = http.client.HTTPConnection("127.0.0.1:80")
h1.request("GET", "/")
r1 = h1.getresponse()
Traceback (most recent call last):
File "", line 1, in
File "/usr/lib/python3.8/http/client.py", line 1347, in getresponse
response.begin()
File "/usr/lib/python3.8/http/client.py", line 307, in begin
version, status, reason = self._read_status()
File "/usr/lib/python3.8/http/client.py", line 289, in _read_status
raise BadStatusLine(line)
http.client.BadStatusLine: Http/1.0 404 Not Found

@ericvsmith
Copy link
Member

Thanks for the reproducer and the research!

https://tools.ietf.org/html/rfc2616#section-3.1 says the result header is "HTTP", and doesn't say anything else is acceptable. I'd be interested in what other frameworks (probably in other languages) support. I'll do some research.

@tiran
Copy link
Member

tiran commented Nov 23, 2020

https://tools.ietf.org/html/rfc2616#section-3.1 defines HTTP version indicator as

HTTP-Version = "HTTP" "/" 1*DIGIT "." 1*DIGIT

so the check

   version.startswith("HTTP/")

is correct.

@sicarius
Copy link
Mannequin Author

sicarius mannequin commented Nov 23, 2020

Hi @christian.heimes,
Thank you for your research too. We've also discovered that this check is correct, but this check is very strict and blocking (error raised, stopping the connection), we should maybe be more "laxist" and allow the lowercase version ? As they do in the others libs ? I've nerver encountered this error with urllib for instance.

The server that answered this HTTP response line is a clone of the "spip" framework used in many websites. This is clearly a human behavior, but this http.client error could be a bit more "intelligent" I guess.

@tiran
Copy link
Member

tiran commented Nov 23, 2020

urllib is a high level API on top of http.client. It wraps and uses http.client internally. urllib does not accept invalid protocol identifiers either:

>>> urllib.request.urlopen('http://localhost:8080')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python3.8/urllib/request.py", line 222, in urlopen
    return opener.open(url, data, timeout)
  File "/usr/lib64/python3.8/urllib/request.py", line 525, in open
    response = self._open(req, data)
  File "/usr/lib64/python3.8/urllib/request.py", line 542, in _open
    result = self._call_chain(self.handle_open, protocol, protocol +
  File "/usr/lib64/python3.8/urllib/request.py", line 502, in _call_chain
    result = func(*args)
  File "/usr/lib64/python3.8/urllib/request.py", line 1379, in http_open
    return self.do_open(http.client.HTTPConnection, req)
  File "/usr/lib64/python3.8/urllib/request.py", line 1354, in do_open
    r = h.getresponse()
  File "/usr/lib64/python3.8/http/client.py", line 1347, in getresponse
    response.begin()
  File "/usr/lib64/python3.8/http/client.py", line 307, in begin
    version, status, reason = self._read_status()
  File "/usr/lib64/python3.8/http/client.py", line 289, in _read_status
    raise BadStatusLine(line)
http.client.BadStatusLine: Http/1.1 200 OK

curl is more forgiving but does not recognize "Http/1.1" as a valid HTTP/1.1 header either. Instead it assumes that any non-valid header means HTTP/1.0.

$ curl -v http://localhost:8080
*   Trying ::1:8080...
* connect to ::1 port 8080 failed: Connection refused
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET / HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.69.1
> Accept: */*
> 
* HTTP 1.0, assume close after body
< Http/1.1 200 OK
< Server: BaseHTTP/0.6 Python/3.8.6
< Date: Mon, 23 Nov 2020 09:10:38 GMT
< Content-Type: text/plain
< 
* Closing connection 0

I'm against changing the behavior of http.client.

@sicarius
Copy link
Mannequin Author

sicarius mannequin commented Nov 23, 2020

Alright, that was a bad idea.

@sicarius sicarius mannequin closed this as completed Nov 23, 2020
@sicarius sicarius mannequin closed this as completed Nov 23, 2020
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants