search

tus / tus-java-client

The tus client for Java.
https://tus.io
MIT License
213 stars 88 forks source link

uploadChunk() should check for error codes #4

Closed cjhenck closed 8 years ago

cjhenck commented 8 years ago

If the server sends back an error code, uploadChunk() will happily keep putting data onto the outputstream.

The current tusd implementation does not close the connection on such an error code (presumably to reuse it efficiently). Is that the more proper thing to do in the HTTP spec?

Acconut commented 8 years ago

This is a great idea and I would be pleased to have this feature but Java's HTTPUrlConnection does not play well with it. If we want to see if the server has already sent a response code back, the OutputStream, using which we upload the file, is automatically closed and therefore unusable. Basically, after you checked the response code, you can not write to a HTTP request.

I will see if we can work around this but I am not very sure about this.

Acconut commented 8 years ago

I've been working in this for some while now and guess, we have reached a point worth sharing.

If the server sends back an error code, uploadChunk() will happily keep putting data onto the outputstream.

This is a more general issue in the world of HTTP. While the specification allows a response to be sent before the entire request, including its optional body, has been transmitted, it only encourages implementations to respect this behavior. This has led to a situation where not all clients, servers and proxies support it (see http://stackoverflow.com/questions/18367824/how-to-cancel-http-upload-from-data-events/18370751#18370751 and https://github.com/golang/go/issues/4637#issuecomment-66073563, for example). Java's built in HTTP client using HTTPUrlConnection does not allow writing to the request body after reading the response. If a server wants to indicate the client that it should stop sending the request body it can send the response in hope that it may respect it and then close the connection to prevent further data transmission. If this scenario happens (the connection is closed) uploadChunk() will throw an IOException.

However, there is another solution which helps sometimes: Using the 100 Continue status code a server can indicate that it may accept a request based on only the headers (see https://support.urbanairship.com/entries/59909909--Expect-100-Continue-Issues-and-Risks for how this works). Since this may be convenient in some situations, tus-java-client will attempt to use it since a265113a5ef0a19df616f4b5cc0ac978754e4d5e.

The current tusd implementation does not close the connection on such an error code (presumably to reuse it efficiently).

It uses HTTP's keep-alive mechanism to reuse the connection as you said.

Is that the more proper thing to do in the HTTP spec?

Why would you close the connection if you may want to use it again? You do not need to close the channel only to indicate that the request has failed or finished.

Acconut commented 8 years ago

The 0.3.0 release includes the check for the 100 Continue status code. I doubt, there is much more we can in this regard.