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

ShallowEtagHeaderFilter throws a NumberFormatException for responses bigger than 2Gb #33256

Closed
geobreze opened this issue Jul 22, 2024 · 4 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Milestone

Comments

@geobreze
Copy link

Affects: spring-web 6.1.10

Hello, we're observing an issue with ShallowEtagHeaderFilter when the downloaded file is bigger than 2Gb, the ContentCachingResponseWrapper throws a NumberFormatException due to int overflow when trying to save the Content-Length value.

Is it possible to update this class to support long values for the Content-Length value?

Here is a stack trace of an exception

java.lang.NumberFormatException: For input string: "2148532224"
	at java.base/java.lang.NumberFormatException.forInputString(Unknown Source)
	at java.base/java.lang.Integer.parseInt(Unknown Source)
	at java.base/java.lang.Integer.valueOf(Unknown Source)
	at org.springframework.web.util.ContentCachingResponseWrapper.addHeader(ContentCachingResponseWrapper.java:173)
	at jakarta.servlet.http.HttpServletResponseWrapper.addHeader(HttpServletResponseWrapper.java:141)
.......
// Configuration.java

  @Bean
  public ShallowEtagHeaderFilter shallowEtagHeaderFilter() {
    return new ShallowEtagHeaderFilter();
  }

and the sample code we use in @RestController

// Controller.java

    return ResponseEntity.ok()
        .eTag(file.getEtag())
        .contentLength(file.getLength()) // long 
        .header(HttpHeaders.CONTENT_TYPE, file.getContentType())
        .build();

It would be great If you could share suggestions on how to avoid this issue when content length overflows int.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 22, 2024
@bclozel bclozel added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jul 22, 2024
@bclozel
Copy link
Member

bclozel commented Jul 22, 2024

@geobreze we'll look into this, thanks for the report. Are you aware that using the ShallowEtagHeaderFilter is effectively caching the entire response in memory? This is putting huge pressure on your GC and I wouldn't advise this approach if your application is serving very large files.

Instead, I would advise using the static resources features with the strategy that fits your use case.

@snicoll snicoll added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jul 23, 2024
@snicoll snicoll added this to the 6.1.12 milestone Jul 23, 2024
@snicoll snicoll self-assigned this Jul 23, 2024
@geobreze
Copy link
Author

Hi @bclozel, thanks for pointing this out, will take a look into it.

ShallowEtagHeaderFilter is effectively caching the entire response in memory

Is there an option not to cache the response if etag is already provided in the controller?

@bclozel
Copy link
Member

bclozel commented Jul 23, 2024

Is there an option not to cache the response if etag is already provided in the controller?

This is already the case for this filter: https://github.com/spring-projects/spring-framework/blob/main/spring-web/src/main/java/org/springframework/web/filter/ShallowEtagHeaderFilter.java#L218

@snicoll
Copy link
Member

snicoll commented Jul 23, 2024

ContentCachingResponseWrapper is really designed to limit to 2GB given it's buffering the content in memory. We have a dedicated exception that states as much:

if (len > Integer.MAX_VALUE) {
throw new IllegalArgumentException("Content-Length exceeds ContentCachingResponseWrapper's maximum (" +
Integer.MAX_VALUE + "): " + len);
}

We'd like to revisit this class so that it throws the exception consistently.

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) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants