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

jetty 12: ee10: DefaultServlet: Binary data gets double-encoded via UTF-8 Writer, incomplete transmission #9134

Closed
kohlschuetter opened this issue Jan 7, 2023 · 0 comments · Fixed by #9144
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@kohlschuetter
Copy link
Contributor

Jetty version(s)
jetty 12.0.x alpha3, HEAD

Java version/vendor (use: java -version)
openjdk version "18.0.1" 2022-04-19
OpenJDK Runtime Environment Temurin-18.0.1+10 (build 18.0.1+10)
OpenJDK 64-Bit Server VM Temurin-18.0.1+10 (build 18.0.1+10, mixed mode)

OS type/version
macOS 13.1

Description
ee10 DefaultServlet's ServletCoreResponse currently checks "ServletContextResponse#isStreaming()" to determine whether the (potentially binary) data has to be encoded via an existing Writer or can be sent directly as a byte stream.

Unfortunately, isStreaming() returns false when no data has been sent, i.e., even when isWriting() would be false, too.
This causes all sorts of exceptions thrown (sadly only logged at DEBUG level) with messages like java.io.IOException: written 25746 > 12014 content-length, for example:

[qtp1698156408-59] DEBUG org.eclipse.jetty.io.AbstractConnection - onClose HttpConnection@7e454f14::SocketChannelEndPoint@668ef72e[{l=null,r=null,CLOSED,fill=-,flush=-,to=11/30000}{io=0/0,kio=-1,kro=-1}]->[HttpConnection@7e454f14[p=HttpParser{s=END,0 of -1},g=HttpGenerator@358f72b8{s=COMMITTED}]=>HttpChannelState@24a9d4a0{processing=null, processed=true, writeState=NOT_LAST, completed=true, writeCallback=Blocker@5e76dc2c{java.io.IOException: written 25746 > 12014 content-length}, request=GET@630beed http://localhost:8123/blog/assets/img/favicons/favicon.ico HTTP/1.1}]
java.io.IOException: written 25746 > 12014 content-length
	at org.eclipse.jetty.server.internal.HttpChannelState$ChannelResponse.write(HttpChannelState.java:1067)
	at org.eclipse.jetty.ee10.servlet.HttpOutput.channelWrite(HttpOutput.java:213)
	at org.eclipse.jetty.ee10.servlet.HttpOutput.channelWrite(HttpOutput.java:198)
	at org.eclipse.jetty.ee10.servlet.HttpOutput.flush(HttpOutput.java:679)
	at org.eclipse.jetty.ee10.servlet.writer.HttpWriter.flush(HttpWriter.java:55)
	at org.eclipse.jetty.ee10.servlet.writer.ResponseWriter.flush(ResponseWriter.java:142)
	at java.base/java.io.PrintWriter.checkError(PrintWriter.java:465)
	at org.eclipse.jetty.ee10.servlet.writer.ResponseWriter.checkError(ResponseWriter.java:93)
	at org.eclipse.jetty.util.IO.copy(IO.java:215)
	at org.eclipse.jetty.util.IO.copy(IO.java:135)
	at org.eclipse.jetty.ee10.servlet.DefaultServlet$ServletCoreResponse.write(DefaultServlet.java:830)
	at org.eclipse.jetty.server.ResourceService$ContentWriterIteratingCallback.process(ResourceService.java:887)
	at org.eclipse.jetty.util.IteratingCallback.processing(IteratingCallback.java:243)
	at org.eclipse.jetty.util.IteratingCallback.iterate(IteratingCallback.java:224)
	at org.eclipse.jetty.server.ResourceService.writeHttpContent(ResourceService.java:648)
	at org.eclipse.jetty.server.ResourceService.sendData(ResourceService.java:601)
	at org.eclipse.jetty.server.ResourceService.doGet(ResourceService.java:197)
	at org.eclipse.jetty.ee10.servlet.DefaultServlet.doGet(DefaultServlet.java:420)
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:527)
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:614)
	at org.eclipse.jetty.ee10.servlet.ServletHolder.handle(ServletHolder.java:738)
	at org.eclipse.jetty.ee10.servlet.ServletHandler$ChainEnd.doFilter(ServletHandler.java:1585)
	at org.eclipse.jetty.ee10.servlet.ServletHandler$MappedServlet.handle(ServletHandler.java:1518)
	at org.eclipse.jetty.ee10.servlet.ServletChannel.lambda$handle$0(ServletChannel.java:425)
	at org.eclipse.jetty.ee10.servlet.ServletChannel.dispatch(ServletChannel.java:655)
	at org.eclipse.jetty.ee10.servlet.ServletChannel.handle(ServletChannel.java:420)
	at org.eclipse.jetty.ee10.servlet.ServletHandler.process(ServletHandler.java:442)
	at org.eclipse.jetty.ee10.servlet.security.SecurityHandler.process(SecurityHandler.java:563)
	at org.eclipse.jetty.ee10.servlet.SessionHandler.process(SessionHandler.java:756)
	at org.eclipse.jetty.server.handler.ContextHandler.process(ContextHandler.java:696)
	at org.eclipse.jetty.server.handler.ContextHandlerCollection.process(ContextHandlerCollection.java:166)
	at org.eclipse.jetty.server.Handler$Wrapper.process(Handler.java:541)
	at org.eclipse.jetty.server.internal.HttpChannelState$HandlerInvoker.run(HttpChannelState.java:551)
	at org.eclipse.jetty.server.internal.HttpConnection.onFillable(HttpConnection.java:457)
	at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:322)
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:100)
	at org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.runTask(AdaptiveExecutionStrategy.java:473)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.consumeTask(AdaptiveExecutionStrategy.java:436)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.tryProduce(AdaptiveExecutionStrategy.java:288)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.lambda$new$0(AdaptiveExecutionStrategy.java:136)
	at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:411)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:934)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1078)
	at java.base/java.lang.Thread.run(Thread.java:1589)

The issue can be averted by replacing checks for isStreaming() by !isWriting().

How to reproduce?
In an embedded jetty, try to open a binary file (e.g., favicon.ico) that is served from a resource URL.

@kohlschuetter kohlschuetter added the Bug For general bugs on Jetty side label Jan 7, 2023
kohlschuetter added a commit to kohlschuetter/jetty.project that referenced this issue Jan 7, 2023
Checking for isStreaming() will return false even when no data has been
written yet. This erroneously triggers double-encoding of binary data in
a Writer.

If the Writer is not set to ISO-8859-1 or similar, the output will be
corrupted, causing errors like "java.io.IOException: written
25746 > 12014 content-length", and interrupted transmissions.

Replace the isStreaming() check by !isWriting().

jetty#9134

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
lorban pushed a commit that referenced this issue Jan 9, 2023
Checking for isStreaming() will return false even when no data has been
written yet. This erroneously triggers double-encoding of binary data in
a Writer.

If the Writer is not set to ISO-8859-1 or similar, the output will be
corrupted, causing errors like "java.io.IOException: written
25746 > 12014 content-length", and interrupted transmissions.

Replace the isStreaming() check by !isWriting().

#9134

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
lorban added a commit that referenced this issue Jan 9, 2023
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
lorban pushed a commit that referenced this issue Jan 10, 2023
Checking for isStreaming() will return false even when no data has been
written yet. This erroneously triggers double-encoding of binary data in
a Writer.

If the Writer is not set to ISO-8859-1 or similar, the output will be
corrupted, causing errors like "java.io.IOException: written
25746 > 12014 content-length", and interrupted transmissions.

Replace the isStreaming() check by !isWriting().

#9134

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
lorban added a commit that referenced this issue Jan 10, 2023
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
lorban pushed a commit that referenced this issue Jan 10, 2023
Checking for isStreaming() will return false even when no data has been
written yet. This erroneously triggers double-encoding of binary data in
a Writer.

If the Writer is not set to ISO-8859-1 or similar, the output will be
corrupted, causing errors like "java.io.IOException: written
25746 > 12014 content-length", and interrupted transmissions.

Replace the isStreaming() check by !isWriting().

#9134

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
lorban added a commit that referenced this issue Jan 10, 2023
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban lorban closed this as completed Jan 10, 2023
gregpoulos pushed a commit to gregpoulos/jetty.project that referenced this issue Jan 16, 2023
…x-document-modules

* upstream/jetty-12.0.x:
  Issue jetty#9167 - making assumption in flaky test
  jetty 12.0.x cleanup duplicate osgi pom metadata (jetty#9093)
  Jetty 12 - Add tests in util/resource for alternate FileSystem implementations (jetty#9149)
  Cleanup non-retainable `Retainable`s (jetty#9159)
  Fixes retainability of special Chunks (jetty#9073)
  TCK: Dispatch forward and includes attributes do not meet the spec (jetty#9074)
  re-enable h3 tests (jetty#8773)
  More fundamental test case
  Reorganization of jetty-client classes. (jetty#9127)
  Removing @disabled from SslUploadTest
  Removing @disabled from jetty-start
  jetty#9134 added test
  ee10: DefaultServlet: Replace checks for isStreaming() by !isWriting()
  jetty#9078 make HttpContent.getByteBuffer() implementations return new ByteBuffer instances and document that contract
  Fixes jetty#9141 - Thread-safe Content.Chunk#slice (jetty#9142)
  Remove `@Disabled` from `jetty-jmx` (jetty#9143)
  Bump maven.version from 3.8.6 to 3.8.7
  Bump maven.version from 3.8.6 to 3.8.7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
2 participants