-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
SSL3_WRITE_PENDING error from urllib3.contrib.pyopenssl#sendall #717
Comments
Thanks for this enormously detailed bug report @evnm! You've done a lot of really great digging here, which really makes our job a lot easier. 🍮 So your analysis seems generally to be correct. I haven't dived into whether it's correct that openssl's This behaviour was introduced in #628, where we pressed for the use of a memoryview despite the fact that PyOpenSSL will roundtrip back to bytes. Unfortunately, that was bad advice. It was predicated on the assumption that PyOpenSSL could in future behave better with treating memoryviews as buffers and avoid the need to convert them to bytestrings. Having since worked more closely with CFFI in this area, I don't believe this is likely to ever happen because of limitations in CFFI. Specifically, the discussions in CFFI issue 47 indicate that CFFI does not expect to support creating a For this reason, PyOpenSSL can't make the enhancement we thought it could make, which means the @evnm Given that you did such good work diagnosing the bug and proposing a draft patch, would you like to open a pull request with a fix? |
Sounds good. I confirmed that removing the |
Fixed in #719 |
This should remain in place until urllib3/urllib3#717 and kennethreitz/requests#2848 are released.
I upgraded my The only difference in the stack trace is that in pyopenssl.py, it says "line 218" for me (the 1.13 location of This is on Python 2.7.10 here. I'm going to try downgrading Ideas? |
If you can provide a good repro of this that would be enormously helpful! |
I'll see if I can isolate it, and provide a simple repro. Things we (think we) know so far:
|
I've got this very reproducible while developing a requests-based fork of pywinrm (against a Windows wsman endpoint), and I'm getting some form of it on everything from 2.3.0 to today's master. If I back off to requests 2.3.0-2.4.0, it fails with a WantWriteError that I've seen referenced lots of places, but even using the latest requests master (which has urllib3 1.13.1) still exhibits the issue with the naked OpenSSL SSL3_WRITE_PENDING/bad write retry for me. The larger the payload, the more likely it is to occur- wsman breaks after about 450k, so I can't go higher than that, but right near 450k, it'll happen on ~1:5 requests. OSX 10.11.2/Python 2.7.9, talking to a local VirtualBox-hosted Windows Server 2012R2 instance. |
@nitzmahone And your stack trace is exactly the same as this? |
Yup (this one is from PyPI-installed release 2.9.1):
|
Are you sending your large payload from a file object or something else? |
No- manually constructing the payloads as strings (b64-encoded file data inside SOAP envelope). |
@nitzmahone Does this work reproducibly in the same virtual environment just repeatedly POSTing large payloads (e.g. 800kB of data) to httpbin.org? |
What I'm aiming to do here is to get something that I can reproduce on my own machine. |
The client's a Mac- it fails against my local Windows VM with or without Fiddler in the middle. I'll try to put together an isolated repro that hits the problem on that setup, then run it on something else... |
@Lukasa : Finally figured out what's making this fail- the data string has to be of type unicode (instead of str), then it'll blow up quite reliably on an SSL retry. The following repro blows up great against httpbin.org for me (on a Mac, anyway):
|
Ah, good catch. Ironically, now that you've pointed that out I can tell you that if you had deprecation warnings turned on you'd have had warnings pop out that pointed the finger at exactly this problem. =( The ongoing downside of having deprecation warnings disabled in Python by default. The simplest fix here is to just not send unicode data. It's generally unwise to rely on the auto-encoding of unicode objects when attempting to send them over the wire, as you can get stunningly unexpected results. This is one of them. The core problem is that PyOpenSSL itself will convert a unicode object to a bytes object before sending it to OpenSSL: it has to, as OpenSSL has no concept of handling a unicode object. This necessarily creates a new object each time it's done. That means that if a unicode object is passed to PyOpenSSL and a I have mixed feelings on whether or not urllib3 should handle this appropriately. I'm inclined to say "no" because Python 3 doesn't allow you to send unicode objects into sockets (it'll raise a |
Yep, once I realized it only failed on unicode-typed input, I figured that might be the case after following the previous issues with WantWriteError and SSL_WRITE_PENDING. Just wanted to follow up with my resolution in case others trip over this later. There's no great reason for our input string to be unicode-typed in this case anyway (it was kinda accidental just because of the source of the original data), so easy fix for us. An outright early failure on HTTPS + unicode-typed input would've caught this (since we're on Python 2.6/2.7 and don't run w/ deprecation warnings), but I suspect that'd surprise a lot of folks as a breaking change (even though they're already subject to Heisenbugs from this now on Python 2.x). I prefer fail-early over fail-maybe, but it's your show, boss. ;) Thanks! |
I had the same problem. I was doing fn_some_replacements(unicode(data)) before sending the data. |
I'm having the exact problem, using python elasticsearch, and requests_aws4auth, any work around or expected fixes? |
@cristianocca Yes: stop sending unicode objects as request bodies. 😁 |
I have tried that but it doesn't seem I actually have control, somewhere in requests, requests_aws4auth or python-elasticsearch sends unicode. Guess I might have to report on those instead |
It won't be requests. I have a fuzzy recollection that python-elasticsearch has had a tendency of doing this. |
Yeah - a past version of requests could cause this issue, but I think we've mostly seen it in the context of python-elasticsearch, when that library attempts to send unicode. |
The issue seems to come from requests_aws4auth: |
I'm writing a program that makes somewhat large (e.g. tens to hundreds of kilobytes) POST requests over HTTPS using
requests
. Such POSTs fail due toSSL3_WRITE_PENDING
errors from OpenSSL. In short,SSL3_WRITE_PENDING
occurs when an incomplete write operation (i.e. a write that resulted in aWantWriteError
) is retried with arguments not exactly equal to those passed in the initial call (see here and here).Stracktrace from my call into
requests
down throughurllib3
:A bit of digging revealed #412 and #626 as issues that relate to the relevant code in urllib3.
WrappedSocket.sendall
proxies toWrappedSocket._send_until_done
, which invokesOpenSSL.SSL.send
in a loop. The conditions for loop termination are a) the write succeeds or b) a timeout expires while waiting for the socket to become writable in the case of aWantWriteError
.I think that this issue boils down to:
WrappedSocket.sendall
converts the data to be sent to amemoryview
.WrappedSocket._send_until_done
invokesOpenSSL.SSL.send
in a loop according to the conditions above.OpenSSL.SSL.send
calls thememoryview.tobytes()
and passes the bytestring result in the OpenSSL write call.WantWriteError
case, 2 and 3 are repeated and a new bytestring is passed to the OpenSSL write, resulting in aSSL3_WRITE_PENDING
error.I was able to get around this issue by patching
urllib3/contrib/pyopenssl.py
to enforce that a single bytestring is used for all calls toOpenSSL.SSL.send
: https://gist.github.com/evnm/af92092c6a7776e08339Please advise on whether this is a good way to fix the issue. I tried adding a test case to
test/with_dummyserver/test_https.py
, but haven't figured out how to tickle the specific issue I've run into.Relevant versions in use:
The text was updated successfully, but these errors were encountered: