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

411 response results in exception in Session.send #2490

Closed
ben-crowhurst opened this issue Mar 14, 2015 · 14 comments
Closed

411 response results in exception in Session.send #2490

ben-crowhurst opened this issue Mar 14, 2015 · 14 comments

Comments

@ben-crowhurst
Copy link

Full details can be found over at StackOverflow

@ben-crowhurst ben-crowhurst changed the title 411 response status code results in exception in Session.send 411 response results in exception in Session.send Mar 14, 2015
@sigmavirus24
Copy link
Contributor

So this is probably a stupid question, but does that server actually work? As you discussed with @mjpieters on the question, this doesn't happen with our typical test server. Is there a server we can actually reproduce this with?

@ben-crowhurst
Copy link
Author

The application server works with Httplib2, cURL command line tool and browsers (Safari/Chrome).

@Lukasa
Copy link
Member

Lukasa commented Mar 14, 2015

I can see this happening in a number of cases. What I think we'd want to test is whether we can attempt to read from the socket after it threw an exception on the send. Put another way, if we call HTTPConnection.send() and it throws a socket exception, will HTTPConnection.get_response() work well (i.e. no blocking?).

@sigmavirus24
Copy link
Contributor

The application server works with Httplib2, cURL command line tool and browsers (Safari/Chrome).

I understand that it doesn't work with requests. Could you share (even privately, since our emails are publicly accessible) what server it is that you're experiencing problems with so we can attempt to debug this further?

@ben-crowhurst
Copy link
Author

Sorry, I'm afraid the client won't allow that to happen :(. I can inform you that placing a sleep(1) after asio::write( response ) within the server code allows Requests to do its job accurately.

Server

  1. receive request
  2. parse http headers
  3. search for content-length
    3.1 if not found return 411
  4. parse http body

@Lukasa
Copy link
Member

Lukasa commented Mar 14, 2015

So let's be clear, the problem is that the socket.send() throws an exception because the socket is no longer open for writing. What matters is whether we can still coerce httplib into providing us the response in that case without blocking (in case no such response was made).

@sigmavirus24
Copy link
Contributor

With that detail, we can definitely write a socket-level test. Thank you @ben-crowhurst

@Lukasa
Copy link
Member

Lukasa commented Mar 14, 2015

@sigmavirus24 FYI, if you're planning to take this, I think the fix will actually go into urllib3 (or maybe even httplib, if it turns out httplib can't handle it).

@sigmavirus24
Copy link
Contributor

Yeah. "socket-level test" was my hint at you that this would be in urllib3 ;)

@Lukasa
Copy link
Member

Lukasa commented Mar 14, 2015

So some quick local tests suggest that for EPIPE we're ok. Running this 'server':

import socket

s = socket.socket()
s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
s.bind(('', 8080))
s.listen(0)

while True:
    c = s.accept()[0]
    c.send('HTTP/1.1 411 Length Needed\r\nContent-Length: 0\r\n\r\n')
    c.close()

We can achieve the following in httplib:

>>> import httplib
>>> c = httplib.HTTPConnection('localhost', 8080)
>>> c.putrequest('GET', '/'); c.endheaders(); c.send('some data');
>>> c.send('some data')
>>> c.send('some data')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/lib/python2.7/httplib.py", line 826, in send
    self.sock.sendall(data)
  File "/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/lib/python2.7/socket.py", line 224, in meth
    return getattr(self._sock,name)(*args)
socket.error: [Errno 32] Broken pipe
>>> r = c.getresponse()
>>> r.status
411
>>> r.read()
''

So an EPIPE has the potential to give us a response when there is one.

If there isn't a response, it seems that instead we get a BadStatusLine error, which is exactly what I'd suspect. So I think we can bully httplib into doing what we need here.

@ben-crowhurst
Copy link
Author

Having looked at the HTTP Adapter I'm suggesting the following alteration?

        except (ProtocolError, socket.error) as err:
            raise ConnectionError(err, request=request, response=response)

This would stop making a behavioural change to the API but allow those interested to respond accordingly.

@Lukasa
Copy link
Member

Lukasa commented Mar 18, 2015

I don't think HTTP Adapter is low enough down. I think urllib3 needs to know what's going on here.

@sigmavirus24
Copy link
Contributor

@Lukasa you're correct.

@sethmlarson
Copy link
Member

Closing this as it was never a concern for Requests.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants