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.0 Handle HEAD requests in Handler #9953

Closed
poutsma opened this issue Jun 23, 2023 · 3 comments · Fixed by #9957
Closed

Jetty 12.0 Handle HEAD requests in Handler #9953

poutsma opened this issue Jun 23, 2023 · 3 comments · Fixed by #9957
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@poutsma
Copy link

poutsma commented Jun 23, 2023

Hi,

I work on the Spring Framework, where we are in the process of upgrading to Jetty 12 as part of our 6.1 branch. As part of this migration, I have run into an issue.

This issue occurs when an asynchronous WriteListener does not write the amount of bytes specified in Content-Length. This is fine for typical requests, but not for HEAD requests, given Section 14.13 of RFC 2616:

The Content-Length entity-header field indicates the size of the entity-body [...] sent to the recipient or, in the case of the HEAD method, the size of the entity-body that would have been sent had the request been a GET.

The following Servlet can be used to reproduce this issue:

@WebServlet(urlPatterns={"/"}, asyncSupported=true)
public class AsyncServlet extends GenericServlet {
	@Override
	public void service(ServletRequest req, ServletResponse res) throws IOException {
		HttpServletRequest request = (HttpServletRequest) req;
		if (request.getMethod().equals("GET") || request.getMethod().equals("HEAD")) {
			AsyncContext context = req.startAsync();
			res.getOutputStream().setWriteListener(new HeadWriteListener(context));
		}
		else {
			HttpServletResponse response = (HttpServletResponse) res;
			response.sendError(404);
		}
	}

	private static class HeadWriteListener implements WriteListener {
		private final AsyncContext context;

		public HeadWriteListener(AsyncContext context) {
			this.context = context;
		}

		@Override
		public void onWritePossible() throws IOException {
			HttpServletRequest req = (HttpServletRequest) this.context.getRequest();
			HttpServletResponse resp = (HttpServletResponse) this.context.getResponse();
			resp.setContentType("text/plain");
			resp.setContentLength(9);
			if (req.getMethod().equals("GET")) {
				try (ServletOutputStream outputStream = resp.getOutputStream()) {
					outputStream.write("FooBarBaz".getBytes(UTF_8));
				}
			}
			this.context.complete();
		}

		@Override
		public void onError(Throwable t) {
			t.printStackTrace();
		}
	}
}

Running this servlet on Jetty 12.0 beta 2 results in the following exception in HttpChannelState:

java.io.IOException: written 0 < 9 content-length
	at org.eclipse.jetty.server.internal.HttpChannelState$ChannelResponse.write(HttpChannelState.java:1208)
	at org.eclipse.jetty.ee10.servlet.HttpOutput.channelWrite(HttpOutput.java:215)
	at org.eclipse.jetty.ee10.servlet.HttpOutput.complete(HttpOutput.java:447)
	at org.eclipse.jetty.ee10.servlet.ServletContextResponse.completeOutput(ServletContextResponse.java:209)
	at org.eclipse.jetty.ee10.servlet.ServletChannel.handle(ServletChannel.java:625)
	at org.eclipse.jetty.ee10.servlet.ServletHandler.handle(ServletHandler.java:458)
	at org.eclipse.jetty.server.handler.ContextHandler.handle(ContextHandler.java:798)
	at org.eclipse.jetty.server.Handler$Wrapper.handle(Handler.java:611)
	at org.eclipse.jetty.server.Server.handle(Server.java:177)
	at org.eclipse.jetty.server.internal.HttpChannelState$HandlerInvoker.run(HttpChannelState.java:617)
	at org.eclipse.jetty.server.internal.HttpConnection.onFillable(HttpConnection.java:480)
	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.produce(AdaptiveExecutionStrategy.java:196)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:969)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob(QueuedThreadPool.java:1194)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1149)
	at java.base/java.lang.Thread.run(Thread.java:833)

Jetty version(s)
Jetty 12.0 beta2

Java version/vendor (use: java -version)

openjdk version "17.0.7" 2023-04-18 LTS

@poutsma poutsma added the Bug For general bugs on Jetty side label Jun 23, 2023
@gregw gregw self-assigned this Jun 23, 2023
@gregw
Copy link
Contributor

gregw commented Jun 23, 2023

Typically the container ignores content written, so you don't need to explicitly handle head like this servlet does. However, I'll look to see if we can avoid that length written check for head methods when only 0 bytes have been written... stand by...

gregw added a commit that referenced this issue Jun 23, 2023
Fix #9953 so that if a Handler self handles HEAD by not writing content, then the length checks do not fire if 0 bytes have been written.
@gregw gregw linked a pull request Jun 23, 2023 that will close this issue
@gregw
Copy link
Contributor

gregw commented Jun 23, 2023

See PR #9957 This is not actually related to Aysnc, but to any handler that self handles HEAD.

@gregw gregw changed the title Jetty 12.0 beta 2 does not allow asynchronous Servlet to handle HEAD requests Jetty 12.0 beta 2 does not Handler to handle HEAD requests themselves Jun 23, 2023
@gregw gregw changed the title Jetty 12.0 beta 2 does not Handler to handle HEAD requests themselves Jetty 12.0 Handle HEAD requests in Handler Jun 23, 2023
@gregw gregw moved this to 👀 In review in Jetty 12.0.0.beta3 Jun 23, 2023
gregw added a commit that referenced this issue Jun 23, 2023
* Fix #9953 handled HEAD

Fix #9953 so that if a Handler self handles HEAD by not writing content, then the length checks do not fire if 0 bytes have been written.

* updates from review
@gregw gregw moved this from 👀 In review to ✅ Done in Jetty 12.0.0.beta3 Jun 23, 2023
@joakime
Copy link
Contributor

joakime commented Jun 26, 2023

Merged in PR #9957

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
Development

Successfully merging a pull request may close this issue.

3 participants