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 throws Exception handling static files when using response wrapper #10463

Closed
chiqiu opened this issue Sep 1, 2023 · 6 comments
Closed
Labels
Bug For general bugs on Jetty side

Comments

@chiqiu
Copy link

chiqiu commented Sep 1, 2023

Jetty version(s)

Jetty 12
Jetty Environment

EE8
Java version/vendor (use: java -version)
openjdk 17
OS type/version

Description

How to reproduce?
Wheh user uses response wrapper to wrap the response for static files, there will be an exception:
java.lang.NumberFormatException: For input string: "Wed, 31 May 2023 16:15:38 GMT"
at java.base/java.lang.NumberFormatException.forInputString(NumberFormatException.java:67)
at java.base/java.lang.Long.parseLong(Long.java:711)
at java.base/java.lang.Long.parseLong(Long.java:836)
at org.eclipse.jetty.http.HttpField.getLongValue(HttpField.java:355)
at org.eclipse.jetty.ee8.nested.Response.putHeaders(Response.java:1198)
at org.eclipse.jetty.ee8.nested.ResourceService.putHeaders(ResourceService.java:717)
at org.eclipse.jetty.ee8.nested.ResourceService.sendData(ResourceService.java:569)
at org.eclipse.jetty.ee8.nested.ResourceService.doGet(ResourceService.java:279)
at org.eclipse.jetty.ee8.servlet.DefaultServlet.doGet(DefaultServlet.java:432)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:503)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:590)
at org.eclipse.jetty.ee8.servlet.ServletHolder$NotAsync.service(ServletHolder.java:1145)
at org.eclipse.jetty.ee8.servlet.ServletHolder.handle(ServletHolder.java:640)
at org.eclipse.jetty.ee8.servlet.ServletHandler$ChainEnd.doFilter(ServletHandler.java:1373)

The cause is that I used a response wrapper to wrap the response before it is returned.
In ResourceService.putHeaders, if the response is not an instance of Response , Response.putHeaders will be called:

protected void putHeaders(HttpServletResponse response, HttpContent content, long contentLength) {
if (response instanceof Response r) {
r.putHeaders(content, contentLength, _etags);
HttpFields.Mutable fields = r.getHttpFields();
if (_acceptRanges && !fields.contains(HttpHeader.ACCEPT_RANGES))
fields.add(ACCEPT_RANGES);
if (_cacheControl != null && !fields.contains(HttpHeader.CACHE_CONTROL))
fields.add(_cacheControl);
} else {
Response.putHeaders(response, content, contentLength, _etags);<================================
if (_acceptRanges && !response.containsHeader(HttpHeader.ACCEPT_RANGES.asString()))
response.setHeader(ACCEPT_RANGES.getName(), ACCEPT_RANGES.getValue());
if (_cacheControl != null && !response.containsHeader(HttpHeader.CACHE_CONTROL.asString()))
response.setHeader(_cacheControl.getName(), _cacheControl.getValue());
}
}

This line:

public static void putHeaders(HttpServletResponse response, HttpContent content, long contentLength, boolean etag) {
long lml = content.getLastModified().getLongValue();<================================

Throws an exception if the getLastModified() returns a Date String instead of a long String.

Could you please help check if this needs to be fixed?
Thanks!

@chiqiu chiqiu added the Bug For general bugs on Jetty side label Sep 1, 2023
@joakime
Copy link
Contributor

joakime commented Sep 1, 2023

You had a Response.Wrapper where you set the response header Content-Length to the value Wed, 31 May 2023 16:15:38 GMT, and you got an Exception when something needed the numerical value of Content-Length.

Seems like an expected outcome / result in this situation.

What were you expecting to happen?

@chiqiu
Copy link
Author

chiqiu commented Sep 4, 2023

@joakime I didn't set the content-length to a date string in the response wrapper. It is the method content.getLastModified() returned a value 'Wed, 31 May 2023 16:15:38 GMT' .
in the method public static void putHeaders(HttpServletResponse response, HttpContent content, long contentLength, boolean etag)
The content length here is -2. The content entity here is an object of class CompressedFormatsHttpContent which contains a delegate ResourceHttpContent.

@chiqiu
Copy link
Author

chiqiu commented Sep 4, 2023

image

@janbartel
Copy link
Contributor

@chiqiu yup, looks like a bug to me.

Raised PR #10556 to fix it.

@chiqiu
Copy link
Author

chiqiu commented Sep 20, 2023

@janbartel Thanks!

janbartel added a commit that referenced this issue Sep 21, 2023
…apper (#10556)

* Issue #10463 Fix lastModified header when using HttpServletResponseWrapper
@janbartel
Copy link
Contributor

Fixed via #10556

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

No branches or pull requests

3 participants