http icon indicating copy to clipboard operation
http copied to clipboard

Improve request body io-alike objects handling

Open ixti opened this issue 7 years ago • 2 comments

As of #468 pipes will cause failure:

reader, writer = IO.pipe
writer.puts "abc"
writer.close

HTTP.post(url, :body => reader)

That is because reader is an IO that supports rewind, but it's illegal on pipes:

Errno::ESPIPE: Illegal seek

So, I think we should become more strict about what is allowed to be used as request body.

@httprb/core comments and thoughts are welcomed and appreciated!

ixti avatar Apr 24 '18 16:04 ixti

I think it would be nice to support pipes and other non-rewindable objects (another example would be Down::ChunkedIO), because HTTP.rb itself doesn't need the body to be rewindable. Is there any existing or potential features that would benefit from the constraint that the request body needs to be rewindable?

I think that Webmock, which is the one that motivated this change, could just replace the original request body object with the accumulated string, once it reads the original request body. For me that seems like the ideal solution.

janko avatar Apr 24 '18 17:04 janko

I agree that we should not limit body to be rewindable. We just need to raise proper error if response body can't be rewinded upon re-usage attempt.

About WebMock in fact I agree that we should improve integration. I was thinking that I can "better" patch request in webmock... And in fact after some thinking - I guess we can/should extract webmock integration into a gem under httprb - so that adapter will be "in-sync" with http gem itself.

ixti avatar Apr 24 '18 20:04 ixti