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

Can't get server response while uploading with ChunkedInput #6706

Open
slandelle opened this issue May 5, 2017 · 5 comments
Open

Can't get server response while uploading with ChunkedInput #6706

slandelle opened this issue May 5, 2017 · 5 comments

Comments

@slandelle
Copy link
Contributor

slandelle commented May 5, 2017

Expected behavior

When a request is invalid (eg missing auth header), a server will immediately send a response without waiting for the full request body, all the more during a large upload. It will then usually close the socket.

In this case, a Netty client should be able to read this response so it can react based on the information in there (eg www-authenticate header).

Actual behavior

Usually, when using a ChunkedInput, Netty client never reads the response, but crashes when trying to write on the socket because it was closed by the server.

It seems DefaultFileRegion handles this better (I wasn't able to reproduce the issue with it so far).

I get the expected response with Postman too.

Minimal yet complete reproducer code (or URL to code)

See https://github.com/slandelle/read-while-streaming-issue. UploadTest contains tests for DefaultFileRegion, ChunkedFile and ChunkedNioFile.

Server side is Jetty based. JettyServer has a main method so you can run it standalone and test with Postman and such too.

Netty version

4.1.22

JVM version (e.g. java -version)

java version "1.8.0_131"
Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)

OS version (e.g. uname -a)

OSX 10.11.6 (El Capitan)
Darwin Kernel Version 15.6.0

@slandelle
Copy link
Contributor Author

@normanmaurer Gentle ping. Do you have any idea how I could achieve expected behavior?

@slandelle slandelle reopened this Feb 26, 2018
@slandelle
Copy link
Contributor Author

Actually, from RFC7230:

A client sending a message body SHOULD monitor the network connection for an error response while it is transmitting the request. If the client sees a response that indicates the server does not wish to receive the message body and is closing the connection, the client SHOULD immediately cease transmitting the body and close its side of the connection.

How can this be implemented with Netty? Any advice would be greatly appreciated.

@normanmaurer
Copy link
Member

@slandelle what exactly you want to implement here ? Like stopping to write ?

@slandelle
Copy link
Contributor Author

slandelle commented Feb 27, 2018

I would like to be able to read the early HTTP response the server sends before the FIN. Here's another example with some Wireshark dumps: https://groups.google.com/forum/#!topic/asynchttpclient/hGbinqPLHi0

beargummy pushed a commit to beargummy/async-http-client that referenced this issue Jul 18, 2018
Motivation:
Netty doesn't provide any ability to handle early server response when uploading chunked file and instead, `IOException: Pipe closed` is thrown (see netty/netty#6706 for details).

Changes:
NettyInputStreamBody swallows ChannelOutputShutdownException in case it is thrown from netty's ChunkedWriteHandler.

Result:
Multipart early response is available to be handled properly.
beargummy pushed a commit to beargummy/async-http-client that referenced this issue Jul 18, 2018
Motivation:
Netty doesn't provide any ability to handle early server response when uploading chunked file and instead, `IOException: Pipe closed` is thrown (see netty/netty#6706 for details).

Changes:
NettyInputStreamBody swallows ChannelOutputShutdownException in case it is thrown from netty's ChunkedWriteHandler.

Result:
Multipart early response is available to be handled properly.
beargummy pushed a commit to beargummy/async-http-client that referenced this issue Jul 23, 2018
Motivation:
Netty doesn't provide any ability to handle early server response when uploading chunked file and instead, `IOException: Pipe closed` is thrown (see netty/netty#6706 for details).

Changes:
NettyInputStreamBody and BodyChunkedInput swallow ChannelOutputShutdownException in case it is thrown from netty's ChunkedWriteHandler.

Result:
Multipart early response is available to be handled properly.
@kachayev
Copy link
Contributor

kachayev commented Jan 21, 2019

This issue is kinda old... but I hit the same problem pretty recently. @slandelle thanks for the tests, they were really helpful. Talking about this code...

It seems DefaultFileRegion handles this better

That's because DefaultFileRegion bypasses ChunkedWriteHandler completely (as any other non-chunked input). Typically DefaultFileRegion would be handled by your OS, that's why it works.

In all other cases, to "fix" the problem we need to set AUTO_CLOSE to false and suppress ClosedChannelExpection in our handler. Otherwise...

  1. The client writes and flushes request headers
  2. The client is writing & flushing chunks in while loop
  3. The server sends 401 response
  4. The server "tries" to read the rest of the body to ensure that we can keep the connection open (note, this is specific for a Jetty server implementation, this step might be missing; in this particular use case it turned out to be a lose-lose situation as we're burning bandwidth for no reason; I think this is done with the assumption the client would behave correctly)
  5. The server closes the connection

Step 3-5 happens concurrently with 2 (with unpredictable timings). If we hit the write failure during Step 2 because on the other side we've already committed Step 5... our channel automatically closes (due to the default value of AUTO_CLOSE) and the client loses the data that was written on Step 3.

So, in fact, it's resolvable but the behavior of the handler doesn't follow the RFC mentioned (we're not monitoring the network connection). @normanmaurer, would you suggest how this loop should be implemented correctly: "write, flush, check if there's something to read, read if necessary, goto begin or break if no need to write more"? It seems like resubmitting write task to the executor is not enough.

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

No branches or pull requests

3 participants