-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add request temp file buffer (#3479) #3659
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ | |
import org.springframework.boot.context.properties.DeprecatedConfigurationProperty; | ||
import org.springframework.core.style.ToStringCreator; | ||
import org.springframework.http.MediaType; | ||
import org.springframework.util.unit.DataSize; | ||
|
||
@ConfigurationProperties(GatewayMvcProperties.PREFIX) | ||
public class GatewayMvcProperties { | ||
|
@@ -66,6 +67,16 @@ public class GatewayMvcProperties { | |
*/ | ||
private int streamingBufferSize = 16384; | ||
|
||
/** | ||
* Temp file buffer for request. | ||
*/ | ||
private boolean fileBufferEnabled = true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be disabled by default. |
||
|
||
/** | ||
* The size threshold which a request will be written to disk. | ||
*/ | ||
private DataSize fileBufferSizeThreshold = DataSize.ofMegabytes(1L); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's create a nest class |
||
|
||
public List<RouteProperties> getRoutes() { | ||
return routes; | ||
} | ||
|
@@ -102,13 +113,31 @@ public void setStreamingBufferSize(int streamingBufferSize) { | |
this.streamingBufferSize = streamingBufferSize; | ||
} | ||
|
||
public boolean isFileBufferEnabled() { | ||
return fileBufferEnabled; | ||
} | ||
|
||
public void setFileBufferEnabled(boolean fileBufferEnabled) { | ||
this.fileBufferEnabled = fileBufferEnabled; | ||
} | ||
|
||
public DataSize getFileBufferSizeThreshold() { | ||
return fileBufferSizeThreshold; | ||
} | ||
|
||
public void setFileBufferSizeThreshold(DataSize fileBufferSizeThreshold) { | ||
this.fileBufferSizeThreshold = fileBufferSizeThreshold; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return new ToStringCreator(this).append("httpClient", httpClient) | ||
.append("routes", routes) | ||
.append("routesMap", routesMap) | ||
.append("streamingMediaTypes", streamingMediaTypes) | ||
.append("streamingBufferSize", streamingBufferSize) | ||
.append("fileBufferEnabled", fileBufferEnabled) | ||
.append("fileBufferSizeThreshold", fileBufferSizeThreshold) | ||
.toString(); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,14 +16,18 @@ | |
|
||
package org.springframework.cloud.gateway.server.mvc.handler; | ||
|
||
import java.io.File; | ||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.io.OutputStream; | ||
import java.io.UncheckedIOException; | ||
import java.nio.file.Files; | ||
import java.nio.file.StandardCopyOption; | ||
|
||
import org.springframework.cloud.gateway.server.mvc.common.AbstractProxyExchange; | ||
import org.springframework.cloud.gateway.server.mvc.common.MvcUtils; | ||
import org.springframework.cloud.gateway.server.mvc.config.GatewayMvcProperties; | ||
import org.springframework.core.io.FileSystemResource; | ||
import org.springframework.http.client.ClientHttpResponse; | ||
import org.springframework.util.StreamUtils; | ||
import org.springframework.web.client.RestClient; | ||
|
@@ -50,9 +54,25 @@ public ServerResponse exchange(Request request) { | |
.uri(request.getUri()) | ||
.headers(httpHeaders -> httpHeaders.putAll(request.getHeaders())); | ||
if (isBodyPresent(request)) { | ||
requestSpec.body(outputStream -> copyBody(request, outputStream)); | ||
if (isWriteClientBodyToFile(request)) { | ||
requestSpec.body(copyClientBodyToFile(request)); | ||
} | ||
else { | ||
requestSpec.body(outputStream -> copyBody(request, outputStream)); | ||
} | ||
} | ||
return requestSpec.exchange((clientRequest, clientResponse) -> { | ||
ServerResponse serverResponse = doExchange(request, clientResponse); | ||
clearTempFileIfExist(request); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be guarded by the |
||
return serverResponse; | ||
}, false); | ||
} | ||
|
||
private void clearTempFileIfExist(Request request) { | ||
File tempFile = MvcUtils.getAttribute(request.getServerRequest(), MvcUtils.CLIENT_BODY_TMP_ATTR); | ||
if (tempFile != null && tempFile.exists()) { | ||
tempFile.delete(); | ||
} | ||
return requestSpec.exchange((clientRequest, clientResponse) -> doExchange(request, clientResponse), false); | ||
} | ||
|
||
private static boolean isBodyPresent(Request request) { | ||
|
@@ -68,6 +88,23 @@ private static int copyBody(Request request, OutputStream outputStream) throws I | |
return StreamUtils.copy(request.getServerRequest().servletRequest().getInputStream(), outputStream); | ||
} | ||
|
||
private static FileSystemResource copyClientBodyToFile(Request request) { | ||
File bodyTempFile = null; | ||
try { | ||
// TODO: customize temp dir | ||
bodyTempFile = File.createTempFile("gateway_client_body", ".tmp"); | ||
Files.copy(request.getServerRequest().servletRequest().getInputStream(), bodyTempFile.toPath(), StandardCopyOption.REPLACE_EXISTING); | ||
} | ||
catch (IOException e) { | ||
if (bodyTempFile != null && bodyTempFile.exists()) { | ||
bodyTempFile.delete(); | ||
} | ||
throw new UncheckedIOException(e); | ||
} | ||
MvcUtils.setBodyTempFile(request.getServerRequest(), bodyTempFile); | ||
return new FileSystemResource(bodyTempFile); | ||
} | ||
|
||
private ServerResponse doExchange(Request request, ClientHttpResponse clientResponse) throws IOException { | ||
InputStream body = clientResponse.getBody(); | ||
// put the body input stream in a request attribute so filters can read it. | ||
|
There was a problem hiding this comment.
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?