Skip to content

Commit ff46336

Browse files
artem-vartem-v
authored
Enhnanced file-upload (#931)
1 parent 9a44fcc commit ff46336

File tree

3 files changed

+149
-62
lines changed

3 files changed

+149
-62
lines changed

‎services-gateway/src/main/java/io/scalecube/services/gateway/http/HttpGateway.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.function.Function;
2525
import reactor.netty.DisposableServer;
2626
import reactor.netty.http.server.HttpServer;
27+
import reactor.netty.http.server.HttpServerFormDecoderProvider;
2728
import reactor.netty.resources.LoopResources;
2829

2930
public class HttpGateway implements Gateway {
@@ -38,6 +39,7 @@ public class HttpGateway implements Gateway {
3839
private final HttpGatewayAuthenticator authenticator;
3940
private final boolean corsEnabled;
4041
private final CorsConfigBuilder corsConfigBuilder;
42+
private final Consumer<HttpServerFormDecoderProvider.Builder> formDecoderBuilder;
4143

4244
private DisposableServer server;
4345
private LoopResources loopResources;
@@ -50,6 +52,7 @@ private HttpGateway(Builder builder) {
5052
this.authenticator = builder.authenticator;
5153
this.corsEnabled = builder.corsEnabled;
5254
this.corsConfigBuilder = builder.corsConfigBuilder;
55+
this.formDecoderBuilder = builder.formDecoderBuilder;
5356
}
5457

5558
public static Builder builder() {
@@ -80,6 +83,12 @@ public Gateway start(ServiceCall call, ServiceRegistry serviceRegistry) {
8083
.handle(
8184
new HttpGatewayAcceptor(
8285
callFactory.apply(call), serviceRegistry, errorMapper, authenticator))
86+
.httpFormDecoder(
87+
builder -> {
88+
if (formDecoderBuilder != null) {
89+
formDecoderBuilder.accept(builder);
90+
}
91+
})
8392
.bind()
8493
.toFuture()
8594
.get();
@@ -129,6 +138,8 @@ public static class Builder {
129138
.allowedRequestHeaders("*")
130139
.exposeHeaders("*")
131140
.maxAge(3600);
141+
private Consumer<HttpServerFormDecoderProvider.Builder> formDecoderBuilder =
142+
builder -> builder.maxSize(100 * 1024 * 1024);
132143

133144
private Builder() {}
134145

@@ -196,6 +207,15 @@ public Builder corsConfigBuilder(Consumer<CorsConfigBuilder> consumer) {
196207
return this;
197208
}
198209

210+
public Consumer<HttpServerFormDecoderProvider.Builder> formDecoderBuilder() {
211+
return formDecoderBuilder;
212+
}
213+
214+
public Builder formDecoderBuilder(Consumer<HttpServerFormDecoderProvider.Builder> consumer) {
215+
this.formDecoderBuilder = consumer;
216+
return this;
217+
}
218+
199219
public HttpGateway build() {
200220
return new HttpGateway(this);
201221
}

‎services-gateway/src/main/java/io/scalecube/services/gateway/http/HttpGatewayAcceptor.java‎

Lines changed: 54 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import io.netty.buffer.ByteBufAllocator;
1414
import io.netty.buffer.ByteBufOutputStream;
1515
import io.netty.buffer.Unpooled;
16-
import io.netty.handler.codec.http.HttpHeaderNames;
1716
import io.netty.handler.codec.http.HttpMethod;
1817
import io.netty.handler.codec.http.HttpResponseStatus;
1918
import io.netty.handler.codec.http.multipart.FileUpload;
@@ -52,7 +51,6 @@ public class HttpGatewayAcceptor
5251

5352
private static final String ERROR_NAMESPACE = "io.scalecube.services.error";
5453
private static final long MAX_SERVICE_MESSAGE_SIZE = 1024 * 1024;
55-
private static final long MAX_FILE_UPLOAD_SIZE = 100 * 1024 * 1024;
5654

5755
private final ServiceCall serviceCall;
5856
private final ServiceRegistry serviceRegistry;
@@ -104,35 +102,59 @@ private Mono<Void> handleFileUploadRequest(
104102
Map<String, String> principal,
105103
HttpServerRequest httpRequest,
106104
HttpServerResponse httpResponse) {
107-
return Mono.fromRunnable(() -> validateFileUploadRequest(httpRequest))
108-
.thenMany(httpRequest.receiveForm())
109-
.doOnNext(HttpGatewayAcceptor::doPostFileUploadCheck)
105+
return httpRequest
106+
.receiveForm()
107+
.map(
108+
data -> {
109+
if (!(data instanceof FileUpload file)) {
110+
throw new BadRequestException(
111+
"Non file-upload part is not allowed, name=" + data.getName());
112+
}
113+
return file.retain();
114+
})
115+
.collectList()
110116
.flatMap(
111-
httpData ->
112-
serviceCall
113-
.requestBidirectional(
114-
createFileFlux(httpData)
115-
.map(
116-
data ->
117-
toMessage(
118-
httpRequest,
119-
builder -> {
120-
final var filename =
121-
((FileUpload) httpData).getFilename();
117+
files -> {
118+
if (files.size() != 1) {
119+
return Mono.error(
120+
new BadRequestException(
121+
"Exactly one file-upload part is expected (received: "
122+
+ files.size()
123+
+ ")"));
124+
}
125+
126+
final var fileUpload = files.get(0);
127+
final var filename = fileUpload.getFilename();
128+
129+
return serviceCall
130+
.requestBidirectional(
131+
createFileFlux(fileUpload)
132+
.map(
133+
data ->
134+
toMessage(
135+
httpRequest,
136+
builder ->
122137
builder
123138
.headers(principal)
124139
.header(HEADER_UPLOAD_FILENAME, filename)
125-
.data(data);
126-
})))
127-
.last()
128-
.flatMap(
129-
response ->
130-
response.isError() // check error
131-
? error(httpResponse, response)
132-
: response.hasData() // check data
133-
? ok(httpResponse, response)
134-
: noContent(httpResponse))
135-
.doFinally(signalType -> httpData.delete()))
140+
.data(data))))
141+
.last()
142+
.flatMap(
143+
response ->
144+
response.isError() // check error
145+
? error(httpResponse, response)
146+
: response.hasData() // check data
147+
? ok(httpResponse, response)
148+
: noContent(httpResponse))
149+
.doFinally(
150+
signalType -> {
151+
try {
152+
fileUpload.delete();
153+
} finally {
154+
safestRelease(fileUpload);
155+
}
156+
});
157+
})
136158
.then()
137159
.onErrorResume(ex -> error(httpResponse, errorMapper.toMessage(ERROR_NAMESPACE, ex)));
138160
}
@@ -150,7 +172,11 @@ private Mono<Void> handleServiceRequest(
150172
final var limit = MAX_SERVICE_MESSAGE_SIZE;
151173
if (readableBytes >= limit) {
152174
throw new BadRequestException(
153-
"Service message is too large, size: " + readableBytes + ", limit: " + limit);
175+
"Service message is too large (size: "
176+
+ readableBytes
177+
+ ", limit: "
178+
+ limit
179+
+ ")");
154180
}
155181
return reduce.writeBytes(byteBuf);
156182
})
@@ -321,29 +347,4 @@ private static Flux<byte[]> createFileFlux(HttpData httpData) {
321347
throw new RuntimeException(e);
322348
}
323349
}
324-
325-
private static void validateFileUploadRequest(HttpServerRequest httpRequest) {
326-
final var contentLengthHeader =
327-
httpRequest.requestHeaders().get(HttpHeaderNames.CONTENT_LENGTH);
328-
final var contentLength =
329-
contentLengthHeader != null ? Long.parseLong(contentLengthHeader) : -1L;
330-
final var limit = MAX_FILE_UPLOAD_SIZE;
331-
if (contentLength > limit) {
332-
throw new BadRequestException(
333-
"File upload is too large, size: " + contentLength + ", limit: " + limit);
334-
}
335-
}
336-
337-
private static void doPostFileUploadCheck(HttpData httpData) {
338-
if (!(httpData instanceof FileUpload)) {
339-
throw new BadRequestException("File upload is missing or invalid");
340-
}
341-
final long fileSize = httpData.length();
342-
final var limit = MAX_FILE_UPLOAD_SIZE;
343-
if (fileSize > limit) {
344-
httpData.delete();
345-
throw new BadRequestException(
346-
"File upload is too large, size: " + fileSize + ", limit: " + limit);
347-
}
348-
}
349350
}

‎services-gateway/src/test/java/io/scalecube/services/gateway/files/FileUploadTest.java‎

Lines changed: 75 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
import java.io.IOException;
2222
import java.nio.file.Files;
2323
import java.nio.file.Path;
24+
import java.util.function.Function;
25+
import java.util.stream.Stream;
2426
import okhttp3.MediaType;
2527
import okhttp3.MultipartBody;
2628
import okhttp3.OkHttpClient;
@@ -31,11 +33,15 @@
3133
import org.junit.jupiter.api.BeforeAll;
3234
import org.junit.jupiter.api.Test;
3335
import org.junit.jupiter.params.ParameterizedTest;
36+
import org.junit.jupiter.params.provider.MethodSource;
3437
import org.junit.jupiter.params.provider.ValueSource;
3538
import reactor.core.publisher.Mono;
3639

3740
public class FileUploadTest {
3841

42+
private static final int MAX_SIZE = 15 * 1024 * 1024;
43+
private static final int SIZE_OVER_LIMIT = MAX_SIZE << 1;
44+
3945
private static Microservices gateway;
4046
private static Microservices microservices;
4147
private static Address httpAddress;
@@ -56,7 +62,12 @@ static void beforeAll() {
5662
.options(opts -> opts.metadata(serviceEndpoint)))
5763
.transport(
5864
() -> new RSocketServiceTransport().credentialsSupplier(credentialsSupplier))
59-
.gateway(() -> HttpGateway.builder().id("HTTP").build()));
65+
.gateway(
66+
() ->
67+
HttpGateway.builder()
68+
.id("HTTP")
69+
.formDecoderBuilder(builder -> builder.maxSize(MAX_SIZE))
70+
.build()));
6071

6172
microservices =
6273
Microservices.start(
@@ -85,9 +96,9 @@ static void afterAll() {
8596
}
8697
}
8798

88-
@ParameterizedTest(name = "Upload file of size {0} bytes")
89-
@ValueSource(longs = {0, 64, 512, 1024, 1024 * 1024, 10 * 1024 * 1024})
90-
public void uploadSuccessfully(long fileSize) throws Exception {
99+
@ParameterizedTest(name = "Upload successfully, size: {0} bytes")
100+
@ValueSource(longs = {0, 512, 1024, 1024 * 1024, 10 * 1024 * 1024})
101+
void uploadSuccessfully(long fileSize) throws Exception {
91102
final var client = new OkHttpClient();
92103
final var file = generateFile(Files.createTempFile("export_report_", null), fileSize);
93104

@@ -113,10 +124,11 @@ public void uploadSuccessfully(long fileSize) throws Exception {
113124
}
114125
}
115126

116-
@Test
117-
public void uploadFailed() throws Exception {
127+
@ParameterizedTest
128+
@MethodSource("uploadFailedSource")
129+
void uploadFailed(UploadFailedArgs args) throws Exception {
118130
final var client = new OkHttpClient();
119-
final var file = generateFile(Files.createTempFile("export_report_", null), 1024);
131+
final var file = generateFile(Files.createTempFile("export_report_", null), args.fileSize());
120132

121133
final var body =
122134
new MultipartBody.Builder()
@@ -135,8 +147,47 @@ public void uploadFailed() throws Exception {
135147
assertEquals(
136148
HttpResponseStatus.INTERNAL_SERVER_ERROR.code(), response.code(), "response.code");
137149
assertEquals(
138-
"{\"errorCode\":500,\"errorMessage\":\"Upload report failed: %s\"}"
139-
.formatted(file.getName()),
150+
"{\"errorCode\":%s,\"errorMessage\":\"%s\"}"
151+
.formatted(args.errorCode(), args.errorMessageFunc().apply(file)),
152+
response.body().string(),
153+
"response.body");
154+
}
155+
}
156+
157+
@Test
158+
void onlySingleFileAllowed() throws Exception {
159+
final var client = new OkHttpClient();
160+
161+
final var fileSize = 1024 * 1024;
162+
final var file1 = generateFile(Files.createTempFile("export_report_1_", null), fileSize);
163+
final var file2 = generateFile(Files.createTempFile("export_report_2_", null), fileSize);
164+
165+
final var body =
166+
new MultipartBody.Builder()
167+
.setType(MultipartBody.FORM)
168+
.addFormDataPart(
169+
"file",
170+
file1.getName(),
171+
RequestBody.create(file1, MediaType.get("application/text")))
172+
.addFormDataPart(
173+
"file",
174+
file2.getName(),
175+
RequestBody.create(file2, MediaType.get("application/text")))
176+
.build();
177+
178+
final var request =
179+
new Request.Builder()
180+
.url("http://localhost:" + httpAddress.port() + "/v1/api/uploadReportError")
181+
.post(body)
182+
.build();
183+
184+
try (Response response = client.newCall(request).execute()) {
185+
assertEquals(HttpResponseStatus.BAD_REQUEST.code(), response.code(), "response.code");
186+
assertEquals(
187+
"{"
188+
+ "\"errorCode\":400,"
189+
+ "\"errorMessage\":\"Exactly one file-upload part is expected (received: 2)\""
190+
+ "}",
140191
response.body().string(),
141192
"response.body");
142193
}
@@ -152,4 +203,19 @@ private static void assertFilesEqual(Path expected, Path actual) {
152203
throw new RuntimeException(e);
153204
}
154205
}
206+
207+
private static Stream<UploadFailedArgs> uploadFailedSource() {
208+
return Stream.of(
209+
new UploadFailedArgs(
210+
1024 * 1024,
211+
HttpResponseStatus.INTERNAL_SERVER_ERROR.code(),
212+
file -> "Upload report failed: " + file.getName()),
213+
new UploadFailedArgs(
214+
SIZE_OVER_LIMIT,
215+
HttpResponseStatus.INTERNAL_SERVER_ERROR.code(),
216+
file -> "java.io.IOException: Size exceed allowed maximum capacity"));
217+
}
218+
219+
private record UploadFailedArgs(
220+
long fileSize, int errorCode, Function<File, String> errorMessageFunc) {}
155221
}

0 commit comments

Comments
 (0)