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

ServletInputStream::isReady results in IllegalArgumentException #10315

Closed
poutsma opened this issue Aug 15, 2023 · 3 comments · Fixed by #10343
Closed

ServletInputStream::isReady results in IllegalArgumentException #10315

poutsma opened this issue Aug 15, 2023 · 3 comments · Fixed by #10343
Assignees
Labels
Bug For general bugs on Jetty side High Priority

Comments

@poutsma
Copy link

poutsma commented Aug 15, 2023

Jetty version(s)
Jetty 12.0.0

Jetty Environment
ee10

Java version/vendor (use: java -version)
openjdk version "17.0.7" 2023-04-18 LTS

OS type/version
MacOS 13.5

Description
The Spring Framework is upgrading to 12.0 as part of its 6.1 release.
Using 12.0.0 GA, we run into one particular issue that occurs in an asynchronous servlet, where when we check ServletInputStream::isReady it results in a IllegalArgumentException. This is the stack trace:

at org.eclipse.jetty.server.internal.HttpChannelState$ChannelRequest.demand(HttpChannelState.java:1003) ~[jetty-server-12.0.0.jar:12.0.0]
at org.eclipse.jetty.server.Request$Wrapper.demand(Request.java:718) ~[jetty-server-12.0.0.jar:12.0.0]
at org.eclipse.jetty.server.handler.ContextRequest.demand(ContextRequest.java:41) ~[jetty-server-12.0.0.jar:12.0.0]
at org.eclipse.jetty.ee10.servlet.AsyncContentProducer.isReady(AsyncContentProducer.java:231) ~[jetty-ee10-servlet-12.0.0.jar:12.0.0]
at org.eclipse.jetty.ee10.servlet.HttpInput.isReady(HttpInput.java:178) ~[jetty-ee10-servlet-12.0.0.jar:12.0.0]
at org.springframework.http.server.reactive.ServletServerHttpRequest$RequestBodyPublisher.checkOnDataAvailable(ServletServerHttpRequest.java:312) ~[main/:?]
at org.springframework.http.server.reactive.AbstractListenerReadPublisher.changeToDemandState(AbstractListenerReadPublisher.java:235) ~[main/:?]
at org.springframework.http.server.reactive.AbstractListenerReadPublisher$State$2.request(AbstractListenerReadPublisher.java:350) ~[main/:?]
at org.springframework.http.server.reactive.AbstractListenerReadPublisher$ReadSubscription.request(AbstractListenerReadPublisher.java:276) ~[main/:?]
at reactor.core.publisher.BaseSubscriber.request(BaseSubscriber.java:214) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at org.springframework.http.codec.multipart.MultipartParser.requestBuffer(MultipartParser.java:193) ~[main/:?]
at org.springframework.http.codec.multipart.MultipartParser.hookOnSubscribe(MultipartParser.java:114) ~[main/:?]
at reactor.core.publisher.BaseSubscriber.onSubscribe(BaseSubscriber.java:148) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at org.springframework.http.server.reactive.AbstractListenerReadPublisher$State$1.subscribe(AbstractListenerReadPublisher.java:318) ~[main/:?]
at org.springframework.http.server.reactive.AbstractListenerReadPublisher.subscribe(AbstractListenerReadPublisher.java:109) ~[main/:?]
at reactor.core.publisher.FluxSource.subscribe(FluxSource.java:74) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at org.springframework.http.codec.multipart.MultipartParser.lambda$parse$1(MultipartParser.java:103) ~[main/:?]
at reactor.core.publisher.FluxCreate.subscribe(FluxCreate.java:95) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.InternalFluxOperator.subscribe(InternalFluxOperator.java:62) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.FluxDefer.subscribe(FluxDefer.java:54) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:64) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.MonoFlatMap$FlatMapMain.onNext(MonoFlatMap.java:165) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.FluxSwitchIfEmpty$SwitchIfEmptySubscriber.onNext(FluxSwitchIfEmpty.java:74) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.MonoNext$NextSubscriber.onNext(MonoNext.java:82) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.FluxConcatMapNoPrefetch$FluxConcatMapNoPrefetchSubscriber.innerNext(FluxConcatMapNoPrefetch.java:258) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.FluxConcatMap$ConcatMapInner.onNext(FluxConcatMap.java:863) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.FluxConcatMap$WeakScalarSubscription.request(FluxConcatMap.java:479) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.Operators$MultiSubscriptionSubscriber.request(Operators.java:2305) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.FluxConcatMapNoPrefetch$FluxConcatMapNoPrefetchSubscriber.request(FluxConcatMapNoPrefetch.java:338) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.MonoNext$NextSubscriber.request(MonoNext.java:108) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.Operators$MultiSubscriptionSubscriber.set(Operators.java:2341) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.Operators$MultiSubscriptionSubscriber.onSubscribe(Operators.java:2215) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.MonoNext$NextSubscriber.onSubscribe(MonoNext.java:70) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.FluxConcatMapNoPrefetch$FluxConcatMapNoPrefetchSubscriber.onSubscribe(FluxConcatMapNoPrefetch.java:164) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.FluxIterable.subscribe(FluxIterable.java:201) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.FluxIterable.subscribe(FluxIterable.java:83) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:64) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.MonoDefer.subscribe(MonoDefer.java:53) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.MonoDefer.subscribe(MonoDefer.java:53) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:64) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.MonoDefer.subscribe(MonoDefer.java:53) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.Mono.subscribe(Mono.java:4495) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.MonoIgnoreThen$ThenIgnoreMain.subscribeNext(MonoIgnoreThen.java:263) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.MonoIgnoreThen.subscribe(MonoIgnoreThen.java:51) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at reactor.core.publisher.Mono.subscribe(Mono.java:4495) ~[reactor-core-3.6.0-M1.jar:3.6.0-M1]
at org.springframework.http.server.reactive.ServletHttpHandlerAdapter.service(ServletHttpHandlerAdapter.java:199) ~[main/:?]
at org.eclipse.jetty.ee10.servlet.ServletHolder.handle(ServletHolder.java:736) ~[jetty-ee10-servlet-12.0.0.jar:12.0.0]
at org.eclipse.jetty.ee10.servlet.ServletHandler$ChainEnd.doFilter(ServletHandler.java:1608) ~[jetty-ee10-servlet-12.0.0.jar:12.0.0]
at org.eclipse.jetty.ee10.servlet.ServletHandler$MappedServlet.handle(ServletHandler.java:1541) ~[jetty-ee10-servlet-12.0.0.jar:12.0.0]
at org.eclipse.jetty.ee10.servlet.ServletChannel$RequestDispatchable.dispatch(ServletChannel.java:910) ~[jetty-ee10-servlet-12.0.0.jar:12.0.0]
at org.eclipse.jetty.ee10.servlet.ServletChannel.dispatch(ServletChannel.java:686) ~[jetty-ee10-servlet-12.0.0.jar:12.0.0]
at org.eclipse.jetty.ee10.servlet.ServletChannel.handle(ServletChannel.java:483) ~[jetty-ee10-servlet-12.0.0.jar:12.0.0]
at org.eclipse.jetty.ee10.servlet.ServletHandler.handle(ServletHandler.java:463) ~[jetty-ee10-servlet-12.0.0.jar:12.0.0]
at org.eclipse.jetty.server.handler.ContextHandler.handle(ContextHandler.java:808) ~[jetty-server-12.0.0.jar:12.0.0]
at org.eclipse.jetty.server.Handler$Wrapper.handle(Handler.java:596) ~[jetty-server-12.0.0.jar:12.0.0]
at org.eclipse.jetty.server.Server.handle(Server.java:177) ~[jetty-server-12.0.0.jar:12.0.0]
at org.eclipse.jetty.server.internal.HttpChannelState$HandlerInvoker.run(HttpChannelState.java:651) ~[jetty-server-12.0.0.jar:12.0.0]
at org.eclipse.jetty.server.internal.HttpConnection.onFillable(HttpConnection.java:480) ~[jetty-server-12.0.0.jar:12.0.0]
at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:322) ~[jetty-io-12.0.0.jar:12.0.0]
at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:100) ~[jetty-io-12.0.0.jar:12.0.0]
at org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53) ~[jetty-io-12.0.0.jar:12.0.0]
at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:969) ~[jetty-util-12.0.0.jar:12.0.0]
at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob(QueuedThreadPool.java:1194) ~[jetty-util-12.0.0.jar:12.0.0]
at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1149) ~[jetty-util-12.0.0.jar:12.0.0]
at java.lang.Thread.run(Thread.java:1583) [?:?]

The line in Spring Framework that causes this is here.

The line in Jetty that throws the exception is here.

This same code worked in previous version of Jetty (9.1+), so this looks like some sort of regression, as I don't think ServletInputStream::isReady is allowed to throw an IllegalArgumentException. If it is, and we are doing something wrong on our end, please inform us.

How to reproduce?

I've tried my best to create a small minimal sample that reproduces this problem, but have been unable to in a reasonable timespan.

The best luck to reproduce the issue is when running the build of my Spring Framework Jetty 12 upgrade branch that you can find here. Two tests occasionally, not predictably, show the behavior, specifically:

  • org.springframework.web.reactive.function.MultipartRouterFunctionIntegrationTests, and
  • org.springframework.web.reactive.result.method.annotation.RequestMappingMessageConversionIntegrationTests
@poutsma poutsma added the Bug For general bugs on Jetty side label Aug 15, 2023
@sbordet sbordet self-assigned this Aug 17, 2023
@sbordet sbordet moved this to 🏗 In progress in Jetty 12.0.1 - FROZEN Aug 17, 2023
@sbordet
Copy link
Contributor

sbordet commented Aug 17, 2023

@poutsma took me half day to download the internet to build Spring 😅 but I can reproduce the issue, investigating.

sbordet added a commit that referenced this issue Aug 18, 2023
…Exception.

Made sure that when HttpServletRequest.isReady() returns false, it is possible to call it again without getting an exception.

Signed-off-by: Simone Bordet <[email protected]>
@sbordet
Copy link
Contributor

sbordet commented Aug 18, 2023

@poutsma we found the problem and fixed it, will be in 12.0.1 that we plan to release end of month.

I verified that the tests you reported now pass cleanly with the Jetty fix.

lorban added a commit that referenced this issue Aug 18, 2023
…SomeRequestContentThenFailure by flipping the input state to IDLE before unblocking a blocking read

Signed-off-by: Ludovic Orban <[email protected]>
sbordet added a commit that referenced this issue Aug 21, 2023
#10343)

Made sure that when HttpServletRequest.isReady() returns false, it is possible to call it again without getting an exception.

Fix ProxyServletTest.testExpect100ContinueRespond100ContinueSomeRequestContentThenFailure by flipping the input state to IDLE before unblocking a blocking read.

Signed-off-by: Simone Bordet <[email protected]>
Signed-off-by: Ludovic Orban <[email protected]>
Co-authored-by: Ludovic Orban <[email protected]>
@sbordet
Copy link
Contributor

sbordet commented Aug 21, 2023

Fixed by #10343.

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 High Priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants