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

Static resources caching issues with ShallowEtagHeaderFilter and Jetty caching directives #32039

Closed
sebastian-steiner opened this issue Jan 16, 2024 · 3 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@sebastian-steiner
Copy link

Affects: 6.1.2 on Spring Boot 3.2.0


When the ShallowEtagHeaderFilter is enabled along with the jetty web server, static resources fail to load every second time.

If you fetch a static resource under this configuration, it will work fine for the first call. On the second request, with an ETag attached, the server responds with the following error:

<h2>HTTP ERROR 500 java.io.IOException: written 0 &lt; 4 content-length</h2>

URI: | http://localhost:8080/
-- | --
500
java.io.IOException: written 0 < 4 content-length
java.io.IOException: written 0 < 4 content-length


<h3>Caused by:</h3><pre>java.io.IOException: written 0 &lt; 4 content-length
	at org.eclipse.jetty.server.internal.HttpChannelState$ChannelResponse.write(HttpChannelState.java:1182)
	at org.eclipse.jetty.server.Response$Wrapper.write(Response.java:735)
	at org.eclipse.jetty.server.handler.ContextResponse.write(ContextResponse.java:56)
	at org.eclipse.jetty.ee10.servlet.HttpOutput.channelWrite(HttpOutput.java:204)
	at org.eclipse.jetty.ee10.servlet.HttpOutput.complete(HttpOutput.java:435)
	at org.eclipse.jetty.ee10.servlet.ServletContextResponse.completeOutput(ServletContextResponse.java:209)
	at org.eclipse.jetty.ee10.servlet.ServletChannel.handle(ServletChannel.java:553)
	at org.eclipse.jetty.ee10.servlet.ServletHandler.handle(ServletHandler.java:464)
	at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:571)
	at org.eclipse.jetty.ee10.servlet.SessionHandler.handle(SessionHandler.java:703)
	at org.eclipse.jetty.server.handler.ContextHandler.handle(ContextHandler.java:761)
	at org.eclipse.jetty.server.Server.handle(Server.java:179)
	at org.eclipse.jetty.server.internal.HttpChannelState$HandlerInvoker.run(HttpChannelState.java:594)
	at org.eclipse.jetty.server.internal.HttpConnection.onFillable(HttpConnection.java:424)
	at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:322)
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:99)
	at org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.runTask(AdaptiveExecutionStrategy.java:478)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.consumeTask(AdaptiveExecutionStrategy.java:441)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.tryProduce(AdaptiveExecutionStrategy.java:293)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.produce(AdaptiveExecutionStrategy.java:195)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:971)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob(QueuedThreadPool.java:1201)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1156)
	at java.base/java.lang.Thread.run(Thread.java:1583)
</pre>

It seems that the response is already marked as "Not Modified" and the body cleared by one caching component, while the other still expects it to be unchanged.

Repository with minimal reproducer: https://github.com/sebastian-steiner/spring-cacheissue

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 16, 2024
@jhoeller jhoeller added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jan 17, 2024
@poutsma
Copy link
Contributor

poutsma commented Jan 17, 2024

This might be related to jetty/jetty.project#9953.

@poutsma
Copy link
Contributor

poutsma commented Jan 18, 2024

The reason for this issue is that the response has a Content-Length header that does not correspond to the amount of bytes written, an error that Jetty 12 tracks more strictly. While the ShallowEtagHeaderFilter does filter out the content-length if set through setContentLength, it does not do so when setHeader("Content-Length", x) is used. I will prepare a fix.

@sebastian-steiner
Copy link
Author

Thanks for the explanation and the quick triage!

poutsma added a commit that referenced this issue Jan 18, 2024
This commit ensures that setting the Content-Length through
setHeader("Content-Length", x") has the same effect as calling
setContentLength in the ShallowEtagHeaderFilter. It also filters out
Content-Type headers similarly to Content-Length.

See gh-32039
Closes gh-32051
poutsma added a commit that referenced this issue Jan 18, 2024
This commit ensures that setting the Content-Length through
setHeader("Content-Length", x") has the same effect as calling
setContentLength in the ShallowEtagHeaderFilter. It also filters out
Content-Type headers similarly to Content-Length.

See gh-32039
Closes gh-32050
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Feb 24, 2024
Commit 375e0e6 introduced a regression in
ContentCachingResponseWrapper (CCRW). Specifically, CCRW no longer
honors Content-Type and Content-Length headers that have been set on
the wrapped response and returns null for those header values if they
have not been set directly on the CCRW.

This commit fixes this regression as follows.

- The Content-Type and Content-Length headers set in the wrapped
  response are honored in getContentType(), containsHeader(),
  getHeader(), and getHeaders() unless those headers have been set
  directly on the CCRW.

- In copyBodyToResponse(), the Content-Type in the wrapped response is
  only overridden if the the Content-Type has been set directly on the
  CCRW.

See spring-projectsgh-32039
Closes spring-projectsgh-32317
sbrannen added a commit that referenced this issue Feb 24, 2024
Commit 375e0e6 introduced a regression in
ContentCachingResponseWrapper (CCRW). Specifically, CCRW no longer
honors Content-Type and Content-Length headers that have been set on
the wrapped response and returns null for those header values if they
have not been set directly on the CCRW.

This commit fixes this regression as follows.

- The Content-Type and Content-Length headers set in the wrapped
  response are honored in getContentType(), containsHeader(),
  getHeader(), and getHeaders() unless those headers have been set
  directly on the CCRW.

- In copyBodyToResponse(), the Content-Type in the wrapped response is
  only overridden if the Content-Type has been set directly on the CCRW.

See gh-32039
Closes gh-32317
sbrannen added a commit that referenced this issue Feb 25, 2024
Prior to this commit, getHeaderNames() returned duplicates for the
Content-Type and Content-Length headers if they were set in the wrapped
response as well as in ContentCachingResponseWrapper.

This commit fixes that by returning a unique set from getHeaderNames().

In addition, this commit introduces a new test in
ContentCachingResponseWrapperTests to verify the expected behavior for
Content-Type and Content-Length headers that are set in the wrapped
response as well as in ContentCachingResponseWrapper.

See gh-32039
See gh-32317
sbrannen added a commit that referenced this issue Feb 25, 2024
Commit 375e0e6 introduced a regression in
ContentCachingResponseWrapper (CCRW). Specifically, CCRW no longer
honors Content-Type and Content-Length headers that have been set in
the wrapped response and now incorrectly returns null for those header
values if they have not been set directly in the CCRW.

This commit fixes this regression as follows.

- The Content-Type and Content-Length headers set in the wrapped
  response are honored in getContentType(), containsHeader(),
  getHeader(), and getHeaders() unless those headers have been set
  directly in the CCRW.

- In copyBodyToResponse(), the Content-Type in the wrapped response is
  only overridden if the Content-Type has been set directly in the CCRW.

Furthermore, prior to this commit, getHeaderNames() returned duplicates
for the Content-Type and Content-Length headers if they were set in the
wrapped response as well as in CCRW.

This commit fixes that by returning a unique set from getHeaderNames().

This commit also updates ContentCachingResponseWrapperTests to verify
the expected behavior for Content-Type and Content-Length headers that
are set in the wrapped response as well as in CCRW.

See gh-32039
See gh-32317
Closes gh-32321
sbrannen added a commit that referenced this issue Feb 25, 2024
Commit 375e0e6 introduced a regression in
ContentCachingResponseWrapper (CCRW). Specifically, CCRW no longer
honors Content-Type and Content-Length headers that have been set in
the wrapped response and now incorrectly returns null for those header
values if they have not been set directly in the CCRW.

This commit fixes this regression as follows.

- The Content-Type and Content-Length headers set in the wrapped
  response are honored in getContentType(), containsHeader(),
  getHeader(), and getHeaders() unless those headers have been set
  directly in the CCRW.

- In copyBodyToResponse(), the Content-Type in the wrapped response is
  only overridden if the Content-Type has been set directly in the CCRW.

Furthermore, prior to this commit, getHeaderNames() returned duplicates
for the Content-Type and Content-Length headers if they were set in the
wrapped response as well as in CCRW.

This commit fixes that by returning a unique set from getHeaderNames().

This commit also updates ContentCachingResponseWrapperTests to verify
the expected behavior for Content-Type and Content-Length headers that
are set in the wrapped response as well as in CCRW.

See gh-32039
See gh-32317
Closes gh-32322
sbrannen added a commit that referenced this issue Feb 26, 2024
Based on feedback from several members of the community, we have
decided to revert the caching of the Content-Type header that was
introduced in ContentCachingResponseWrapper in 375e0e6.

This commit therefore completely removes Content-Type caching in
ContentCachingResponseWrapper and updates the existing tests
accordingly.

To provide guards against future regressions in this area, this commit
also introduces explicit tests for the 6 ways to set the content length
in ContentCachingResponseWrapper and modifies a test in
ShallowEtagHeaderFilterTests to ensure that a Content-Type header set
directly on ContentCachingResponseWrapper is propagated to the
underlying response even if content caching is disabled for the
ShallowEtagHeaderFilter.

See gh-32039
Closes gh-32317
sbrannen added a commit that referenced this issue Feb 28, 2024
Based on feedback from several members of the community, we have
decided to revert the caching of the Content-Type header that was
introduced in ContentCachingResponseWrapper in 375e0e6.

This commit therefore completely removes Content-Type caching in
ContentCachingResponseWrapper and updates the existing tests
accordingly.

To provide guards against future regressions in this area, this commit
also introduces explicit tests for the 6 ways to set the content length
in ContentCachingResponseWrapper and modifies a test in
ShallowEtagHeaderFilterTests to ensure that a Content-Type header set
directly on ContentCachingResponseWrapper is propagated to the
underlying response even if content caching is disabled for the
ShallowEtagHeaderFilter.

See gh-32039
See gh-32317
Closes gh-32321
sbrannen added a commit that referenced this issue Feb 28, 2024
Based on feedback from several members of the community, we have
decided to revert the caching of the Content-Type header that was
introduced in ContentCachingResponseWrapper in 375e0e6.

This commit therefore completely removes Content-Type caching in
ContentCachingResponseWrapper and updates the existing tests
accordingly.

To provide guards against future regressions in this area, this commit
also introduces explicit tests for the 6 ways to set the content length
in ContentCachingResponseWrapper and modifies a test in
ShallowEtagHeaderFilterTests to ensure that a Content-Type header set
directly on ContentCachingResponseWrapper is propagated to the
underlying response even if content caching is disabled for the
ShallowEtagHeaderFilter.

See gh-32039
See gh-32317
Closes gh-32322
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants