Hi Daniel,

On 15/10/2018, 12:53, Daniel Fuchs wrote:
Hi Michael,

On 15/10/2018 11:54, Michael McMahon wrote:
Could I get the following fix reviewed please.

http://cr.openjdk.java.net/~michaelm/8211437/webrev.1/index.html

Looks good in general.

In MultiExchange.java:

236             T nullBody = cf.get();

Though technically the body should be available by the
time we reach this line, since you completed the subscriber
just before, we can't really make any assumption on the
implementation of the subscriber.

So for the sake of robustness we should probably use
getBody().handle(...) to complete `result`
rather than calling cf.get();


Yes, that would be better.


Also I wonder what should happen if a body is present:

Should we simply read it instead?
Because if we don't then we should close the connection (HTTP/1.1)
or reset the stream (HTTP/2) - which probably means getting back
to the concrete  exchange implementation to make sure that
happens.


I see that as a protocol error. So, rather than attempting to read the
body, I think we should ensure that the request fails (which it does
if a content length or transfer encoding field is present). The connection
also needs to be closed (or stream reset) which I need to check is being done.

Thanks,
Michael

best regards,

-- daniel


Thanks,

Michael.

Reply via email to