Parse-transit will ignore body is input-stream doesn't have data available right away

[Deraen]

https://github.com/dakrone/clj-http/commit/72e6ac22ab05c6949bcc1b0c6eba484770fe969e

The parse-transit function is checking if response body InputStream has some data available, using .available method. If the server (or some proxy?) takes some time to write the response body, there might be zero bytes available when parse-transit is called. JSON or other decoding functions don't have this check.

Not exactly sure when this might happen, guess is that we are seeing this with large enough responses and with Amazon Application Load Balancer.

This can be reproduced by calling parse-transit on an InputStream before data is available. Without the .available check Transit will keep trying to read the InputStream untill data is available or stream is closed.

Empty IS case can also be tested with the piped streams, by closing the OS before writing anything. Without .available check, transit/read will throw in that case, as is mentioned in the Transit API docs.

(comment
  (import '[java.io ByteArrayInputStream PipedOutputStream PipedInputStream])

  (let [is (ByteArrayInputStream. (.getBytes "[\"^ \",\"~:a\",[1,2]]"))]
    (parse-transit is :json nil))
  ;; => {:a [1 2]}

  (let [os (PipedOutputStream.)
        is (PipedInputStream. os)]
    (future
      (println "parse: "(parse-transit is :json nil)))
    (Thread/sleep 50)
    (.write os (.getBytes "[\"^ \",\"~:a\",[1,2]]"))
    ))
  ;; parse: nil

[dakrone]

I'm not sure how this would be fixed, is there an indication anywhere that is something like "data is eventually coming, but not yet"?

[Deraen]

It is not possible to know that before all data is transmitted, i.e. the stream is closed.

I think the solution is to remove available check, then the Transit read will block until either data is available for read or the stream is closed. If the stream is closed before any data was available, Transit will throw an exception.

That exception could be caught and nil value returned, but I'm not sure if that is good design or if it would be better for users to handle/fix those cases themselves.

[viesti]

Hi!

I bumped into this again, and found out that there isn't a release that would contain the fix. Could 3.12.4 be release with this fix, and other pending changes?

Cheers!

[dakrone]

Sure, I can do that

[viesti]

Awesome! Thank you! :)

[dakrone]

I have release both 3.12.4 and 3.13.0 (I recommend the later as it includes updated dependencies). Hope this helps!

[viesti]

Nice! I'll try the new version tomorrow, thank you so much :)

[viesti]

Both versions seem to work nicely, thanks for the deps update too :)