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

Support 100 continue responses #592

Closed
matuszewskijan opened this issue Feb 3, 2020 · 5 comments · Fixed by #593
Closed

Support 100 continue responses #592

matuszewskijan opened this issue Feb 3, 2020 · 5 comments · Fixed by #593

Comments

@matuszewskijan
Copy link

It seems that HTTP isn't following the 100-continue responses:

HTTP.headers('Content-Type' => 'application/json', 'Accept' => 'application/json').post('https://foo.bar/api', body: '{...}')
=> #<HTTP::Response/1.1 100 Continue {}>

when cURL does:

curl -v -X POST \
>   https://foo.bar/api \
>   -H 'Accept: application/json' \
>   -H 'Content-Type: application/json' \
>   -d '{...}'
...
> POST /api HTTP/1.1
> Host: foo.bar
> Accept: application/json
> Content-Type: application/json
> Content-Length: 294
> 
* upload completely sent off: 294 out of 294 bytes
< HTTP/1.1 100 Continue
< HTTP/1.1 200 OK
...

I would expect the HTTP response to be <HTTP::Response/1.1 200 OK {}>.

Is it something what the HTTP gem should support?

@woutdegeyter
Copy link

Any feedback on this one, please?
This is currently blocking us from building out an integration as the partner API is returning these 1xx codes.
@ixti

Thanks already.

@ixti
Copy link
Member

ixti commented Feb 5, 2020

If I understand correctly, yes, that's something we should support. As far as I understand second response comes immediately after the 100 Continue - in other words 1 request, but 2 responses. Am I right?

@matuszewskijan
Copy link
Author

Indeed, it seems that we are receiving 2 responses from one request. I'll see to prepare a PR to support this 100 continue code.

@matuszewskijan
Copy link
Author

matuszewskijan commented Feb 6, 2020

I've tried to add support for reading the second response and the only solution which I've found is a change in readpartial method:

  return unless @pending_response
   chunk = @parser.read(size)
   return chunk if chunk
   finished = (read_more(size) == :eof) || @parser.finished?
+
+  if finished && @buffer.include?("HTTP/1.1 100 Continue")
+    @parser.reset
+    finished = false
+  end
+
   chunk    = @parser.read(size)
   finish_response if finished

It seems to be very hacky for me, and the response code will be always 100.

I've spent few hours more to find anything better, but it seems that Http::Connection is the only place where we can do anything with this(correct me if I'am wrong). Experimented with exposing final_chunk method:

def final_chunk?
  @state.final_chunk?
end

in Http::Response::Parser and tried to use it inside #readpartial but it seems to always return false for me(even when the response isn't in chunks at all).

Anyone is having any idea where we can put the logic to support this kind of scenario?

For more context - in Net::Http it's implemented in this way: (ruby-2.5.1/lib/ruby/2.5.0/net/http.rb)
image

@ixti
Copy link
Member

ixti commented Feb 7, 2020

eb7b952 -- half-working patch. It just needs to ensure that skipped case also works and will be good. :D

Realized that we should not listen for status complete event, instead handle message complete event in a special way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants