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

receiveResponse() may not return response body stream #4180

Closed
SpitchAG opened this issue Oct 10, 2023 · 1 comment
Closed

receiveResponse() may not return response body stream #4180

SpitchAG opened this issue Oct 10, 2023 · 1 comment
Assignees
Labels

Comments

@SpitchAG
Copy link

SpitchAG commented Oct 10, 2023

version 1.12.x, using SSL and ChunkedStream request.
something looks wrong in the receiveResponse() implementation:

[Note that in the following scenario, receiveResponse() follow a peekResponse() returning false because other end didnt send 100 continue (and also close connection after sending a body response)].

at begining of receiveResponse() a flushRequest() is performed.
If the other end closed the connection after having sent a response body,
the flushRequest , which deletes the requestStream object, ends up calling HTTPChunkedStreamBuf::close () which writes some stuff on the underlying socket.
This can throw because remote end closed the sock, but it shall not prevent receiveResponse() to provide a response body reader, and caller to retrieve this body.

i did a quick work around so that the flush exception is ignored and it seems to work ok (however it is probably unsafe)

Before 1.12.x, strangely enough, HTTPChunkedStreamBuf::close was sending the 5 bytes 0/r/n/r/n in a raw, and SSL_write() didnt trigger any exc (probably because nothing was really sent on the sock),
but since response trailer header impl, this was changed to 2 consecutive writes (3 long, then 2 long) , and this triggers the exception in my case (i didnt look further in SSL_write see what happens there)

thanks and keep the good work,

@SpitchAG SpitchAG changed the title receiveResponse() may not read response body receiveResponse() may not return response body stream Oct 10, 2023
obiltschnig added a commit that referenced this issue Oct 15, 2023
…final \r\n would not be sent. May be related to #4180
obiltschnig added a commit that referenced this issue Oct 15, 2023
…final \r\n would not be sent. May be related to #4180
@obiltschnig obiltschnig self-assigned this Oct 15, 2023
@obiltschnig obiltschnig added this to the Release 1.12.5 milestone Oct 15, 2023
@obiltschnig
Copy link
Member

Could you try the fix in 1.12.5?

@aleks-f aleks-f added the fixed label Oct 23, 2023
@aleks-f aleks-f closed this as completed Oct 24, 2023
aleks-f pushed a commit that referenced this issue Nov 23, 2023
aleks-f pushed a commit that referenced this issue Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants