Upgrade to Jetty 9.4.21.v20190926 #18494

wilkinsona · 2019-10-03T07:11:21Z

No description provided.

wilkinsona · 2019-10-03T08:14:19Z

9.4.21 appears to contain some changes in behaviour that cause some test failures:

[ERROR] Failures: 
[ERROR]   JettyServletWebServerFactoryTests>AbstractServletWebServerFactoryTests.errorPage:339 
Expecting:
 <"<html>
<head>
<meta http-equiv="Content-Type" content="text/html;charset=utf-8"/>
<title>Error 500 java.lang.RuntimeException: Planned</title>
</head>
<body><h2>HTTP ERROR 500 java.lang.RuntimeException: Planned</h2>
<table>
<tr><th>URI:</th><td>/bang</td></tr>
<tr><th>STATUS:</th><td>500</td></tr>
<tr><th>MESSAGE:</th><td>java.lang.RuntimeException: Planned</td></tr>
<tr><th>SERVLET:</th><td>error</td></tr>
<tr><th>CAUSED BY:</th><td>java.lang.RuntimeException: Planned</td></tr>
</table>
<pre>java.lang.RuntimeException: Planned
	at org.springframework.boot.web.servlet.server.AbstractServletWebServerFactoryTests$4.service(AbstractServletWebServerFactoryTests.java:1109)
	at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:760)
	at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:547)
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
	at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:536)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:235)
	at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1589)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233)
	at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1296)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188)
	at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:485)
	at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1559)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186)
	at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1211)
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
	at org.eclipse.jetty.server.Server.handle(Server.java:500)
	at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:386)
	at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:560)
	at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:378)
	at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:268)
	at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:103)
	at org.eclipse.jetty.io.ChannelEndPoint$2.run(ChannelEndPoint.java:117)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:336)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:313)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:171)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.produce(EatWhatYouKill.java:135)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:782)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:914)
	at java.lang.Thread.run(Thread.java:748)
</pre>

</body>
</html>
">
to be equal to:
 <"Hello World">
but was not.
[ERROR]   JettyServletWebServerFactoryTests>AbstractServletWebServerFactoryTests.errorPageFromPutRequest:349 
Expecting:
 <"">
to be equal to:
 <"Hello World">
but was not.
[ERROR]   JettyServletWebServerFactoryTests>AbstractServletWebServerFactoryTests.persistSession:652 [Session error s1=null:1570086848108 s2=null:1570086848116 s3=null:1570086848227] 
Expecting:
 <"null">
to be equal to:
 <"1570086848108">
but was not.

Further investigation is needed to see if we can work around things in Boot or if there's a regression in Jetty that will need to be fixed there.

dreis2211 · 2019-10-03T12:45:25Z

If it helps: I think the changes that make the errorPage tests fail are likely located in this commit: jetty/jetty.project@bde8646. I didn't get to the exact root cause yet, but Boot's JettyEmbeddedErrorHandler isn't functional anymore at least and I can trace this back to the mentioned commit. I tried overwriting errorPageForMethod to allow for PUT requests again, but it still breaks the current tests.

I didn't get to the session tests yet, but unlike before a new session seems to be created every time which breaks the logic in AbstractServletWebServerFactoryTests.java#L1124 as it can't find the session attribute anymore. I haven't found the commit yet that causes it, but there were definitely changes in the session handling (e.g. jetty/jetty.project#4049)

I initially hoped to fix the issue, but wanted to at least share the findings.

wilkinsona · 2019-10-03T13:20:00Z

Thanks very much for sharing your analysis, @dreis2211.

dreis2211 · 2019-10-03T15:05:27Z

So here's a follow-up on the issues:

Error pages

With the following minimal patch I get the error page related tests to run again (a more complete version would get rid of the custom ErrorHttpServletRequest that seems redundant now):

--- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyEmbeddedErrorHandler.java
+++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyEmbeddedErrorHandler.java
@@ -26,6 +26,7 @@ import javax.servlet.http.HttpServletResponse;
 import org.eclipse.jetty.http.HttpMethod;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.handler.ErrorHandler;
+import org.eclipse.jetty.server.handler.ErrorHandler.ErrorPageMapper;

 /**
  * Variation of Jetty's {@link ErrorHandler} that supports all {@link HttpMethod
@@ -36,7 +37,7 @@ import org.eclipse.jetty.server.handler.ErrorHandler;
  *
  * @author Phillip Webb
  */
-class JettyEmbeddedErrorHandler extends ErrorHandler {
+class JettyEmbeddedErrorHandler extends ErrorHandler implements ErrorPageMapper {

        private final ErrorHandler delegate;

@@ -54,6 +55,19 @@ class JettyEmbeddedErrorHandler extends ErrorHandler {
                this.delegate.handle(target, baseRequest, request, response);
        }

+       @Override
+       public boolean errorPageForMethod(String method) {
+               return true;
+       }
+
+       @Override
+       public String getErrorPage(HttpServletRequest request) {
+               if (this.delegate instanceof ErrorPageMapper) {
+                       return ((ErrorPageMapper) this.delegate).getErrorPage(request);
+               }
+               return null;
+       }
+
        private static class ErrorHttpServletRequest extends HttpServletRequestWrapper {

                private boolean simulateGetMethod = true;

Sessions

That leaves us with persistSession to fix. Unfortunately, I can't narrow it down via building Jetty locally as the build fails on my machine in the (sadly wide) range of commits I wanted to check. The analysis from yesterday still holds true, though. There seems to be a new session created every time via HttpSession session = ((HttpServletRequest) request).getSession(true); instead of reusing it. Initially I thought this might be only a problem between server restarts, but the following test fails as well:

	@Test
	void persistSessionWithoutRestart() throws Exception {
		AbstractServletWebServerFactory factory = getFactory();
		factory.getSession().setPersistent(true);
		this.webServer = factory.getWebServer(sessionServletRegistration());
		this.webServer.start();
		String s1 = getResponse(getLocalUrl("/session"));
		String s2 = getResponse(getLocalUrl("/session"));
		assertThat(s2.split(":")[0]).isEqualTo(s1.split(":")[1]);
		this.webServer.stop();
	}

I'm out of ideas (and time to generate new ideas ;-) ) at the moment, but I hope this helps with finding the root cause. Is there value in reporting this already to the Jetty team?

dreis2211 · 2019-10-04T08:40:57Z

For the error page stuff there is an issue already that might be of interest: jetty/jetty.project#4154

wilkinsona · 2019-10-04T08:55:40Z

Thanks again, @dreis2211. The session problem may well be worth reporting. Let me see if I can find some time to dig into it a bit so that we have something concrete to report. Next week's SpringOne Platform is looming large at the moment so it may take a little while.

dreis2211 · 2019-10-04T10:36:52Z

Have fun at SpringOne Platform then. At some point I hope we can meet up at one of those conferences and have a chat in person.

dreis2211 · 2019-10-05T07:53:48Z

There was a bug opened at Jetty that seemed interesting enough as it's also a regression in the session handling. Unfortunately, when building the branch and testing the proposed fix this doesn't have an impact on the persistSession test - it still fails. For full reference though: jetty/jetty.project#4156

dreis2211 · 2019-10-05T12:07:49Z

I could finally reproduce the behavior change (which appears to be connected to using a cookie with a JSESSIONID). I reported this already to Jetty and hope that's okay. Let's see what comes out of it. If it turns out not to be a bug, I have at least found a fix for the tests.

wilkinsona · 2019-10-07T13:07:50Z

Thanks again, @dreis2211. Judging by the Jetty team's response, it looks reasonable to me to just update our tests not to set the JSESSIONID=123 Cookie header. It was added in 08d1f6d#diff-cad85c99cd5100a102ff8195b58caa73R679 for #2490 but I don't believe it's needed.

dreis2211 · 2019-10-07T13:17:39Z

Already tried it before reporting this to Jetty and it's indeed not needed (hence my comment that I at least found a fix already). ;-) There's still a request for feedback if that change was intentional, so I'd suggest to wait a bit still before finishing this up. But I can already provide a PR with the necessary changes, if you want me to.

dreis2211 · 2019-10-07T14:19:05Z

As the change in the error page handling is likely not working for versions <= 9.4.20 (because of the unknown errorPageForMethod method) I'm currently thinking if we should provide a JettyEmbeddedLegacyErrorHandler to provide a smooth upgrade path for people. At the same time, Spring Boot 2.2.x is probably the best opportunity to raise supported versions of dependencies. What do you think?

dreis2211 · 2019-10-08T12:33:07Z

I opened #18536, so it's easier to talk about potential code changes.

philwebb · 2019-10-22T21:54:13Z

Closing in favor of PR #18536

wilkinsona added the type: dependency-upgrade A dependency upgrade label Oct 3, 2019

wilkinsona modified the milestones: 2.2.0.RC1, 2.2.x Oct 3, 2019

dreis2211 mentioned this issue Oct 5, 2019

Behaviour change in session handling in 9.4.21.v20190926 jetty/jetty.project#4158

Closed

dreis2211 mentioned this issue Oct 8, 2019

Upgrade to Jetty 9.4.21.v20190926 #18536

Closed

wilkinsona modified the milestones: 2.2.x, 2.1.x Oct 14, 2019

philwebb closed this as completed Oct 22, 2019

philwebb added status: superseded An issue that has been superseded by another and removed type: dependency-upgrade A dependency upgrade labels Oct 22, 2019

philwebb removed this from the 2.1.x milestone Oct 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to Jetty 9.4.21.v20190926 #18494

Upgrade to Jetty 9.4.21.v20190926 #18494

wilkinsona commented Oct 3, 2019

wilkinsona commented Oct 3, 2019

dreis2211 commented Oct 3, 2019

wilkinsona commented Oct 3, 2019

dreis2211 commented Oct 3, 2019 •

edited

dreis2211 commented Oct 4, 2019

wilkinsona commented Oct 4, 2019

dreis2211 commented Oct 4, 2019

dreis2211 commented Oct 5, 2019

dreis2211 commented Oct 5, 2019 •

edited

wilkinsona commented Oct 7, 2019

dreis2211 commented Oct 7, 2019

dreis2211 commented Oct 7, 2019 •

edited

dreis2211 commented Oct 8, 2019 •

edited

philwebb commented Oct 22, 2019

Upgrade to Jetty 9.4.21.v20190926 #18494

Upgrade to Jetty 9.4.21.v20190926 #18494

Comments

wilkinsona commented Oct 3, 2019

wilkinsona commented Oct 3, 2019

dreis2211 commented Oct 3, 2019

wilkinsona commented Oct 3, 2019

dreis2211 commented Oct 3, 2019 • edited

Error pages

Sessions

dreis2211 commented Oct 4, 2019

wilkinsona commented Oct 4, 2019

dreis2211 commented Oct 4, 2019

dreis2211 commented Oct 5, 2019

dreis2211 commented Oct 5, 2019 • edited

wilkinsona commented Oct 7, 2019

dreis2211 commented Oct 7, 2019

dreis2211 commented Oct 7, 2019 • edited

dreis2211 commented Oct 8, 2019 • edited

philwebb commented Oct 22, 2019

dreis2211 commented Oct 3, 2019 •

edited

dreis2211 commented Oct 5, 2019 •

edited

dreis2211 commented Oct 7, 2019 •

edited

dreis2211 commented Oct 8, 2019 •

edited