Skip to content

Add request temp file buffer (#3479)#3659

Open
Milkdove wants to merge 6 commits intospring-cloud:mainfrom
Milkdove:add_proxy_temp_file
Open

Add request temp file buffer (#3479)#3659
Milkdove wants to merge 6 commits intospring-cloud:mainfrom
Milkdove:add_proxy_temp_file

Conversation

@Milkdove
Copy link
Copy Markdown
Contributor

@Milkdove Milkdove commented Jan 7, 2025

Add the feature of using temp file as intermediaries when forwarding large requests, which can effectively solve the OOM, and added two configs to control it:

spring.cloud.gateway.mvc.file-buffer-enabled 
spring.cloud.gateway.mvc.file-buffer-size-threshold

I'm not sure if this solution is appropriate. It may cause some large requests to be slowed down.But I don’t have any other ideas at the moment. This is the solution our team uses. In addition, should the default value of file-buffer-enabled be true?

@ryanjbaxter
Copy link
Copy Markdown
Contributor

Shouldn't the gateway stream the file? Or are you using filters that may be reading the body?

@Milkdove
Copy link
Copy Markdown
Contributor Author

Strictly speaking, the current MVC version of the Gateway doesn't truly support streaming transmission. Its process involves reading the request, caching it, and then sending it out. This approach can lead to Out Of Memory (OOM) issues, as seen in some cases.
To achieve streaming forwarding, you'd need to modify the underlying network framework to use asynchronous I/O, like with a WebFlux Gateway. However, this means moving away from the traditional MVC architecture.

@spencergibb
Copy link
Copy Markdown
Member

This version was built specifically for Spring WebMVC and blocking IO is part of that. When Zuul was supported, there was an option to install it as a servlet outside the spring dispatcher servlet, and that's how large uploads were handled. We don't really have either of those options. This seems like a good middle ground.

@@ -75,4 +75,9 @@ private int copyResponseBodyWithFlushing(InputStream inputStream, OutputStream o
return totalReadBytes;
}


protected boolean isWriteClientBodyToFile(Request request) {
return properties.isFileBufferEnabled() && request.getServerRequest().servletRequest().getContentLength() >= properties.getFileBufferSizeThreshold().toBytes();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this should include a content type check as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is mainly about OOM protection.
Whether we write to disk isn’t really tied to content-type, but to request size and memory safety.
We could add content-type based config later if there’s a real need, but I’d prefer to keep this logic simple for now.

@Milkdove Milkdove requested a review from spencergibb January 8, 2026 06:33
@Milkdove Milkdove force-pushed the add_proxy_temp_file branch 2 times, most recently from b4a7fd0 to cb85a78 Compare January 8, 2026 09:40
Signed-off-by: Milkdove <pollux.li@outlook.com>
… logic (spring-cloud#3479)

Signed-off-by: Milkdove <pollux.li@outlook.com>
Signed-off-by: Milkdove <pollux.li@outlook.com>
… logic (spring-cloud#3479)

Signed-off-by: Milkdove <pollux.li@outlook.com>
@Milkdove Milkdove force-pushed the add_proxy_temp_file branch from cb85a78 to 207e667 Compare January 8, 2026 10:30
Signed-off-by: Milkdove <49242304+Milkdove@users.noreply.github.com>
Signed-off-by: Milkdove <pollux.li@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants