Skip to content

Commit 399d549

Browse files
jknackjknack
committed
netty: exceed file size are not detected #3790
- fix #3790
1 parent 37956f1 commit 399d549

4 files changed

Lines changed: 60 additions & 71 deletions

File tree

‎modules/jooby-netty/src/main/java/io/jooby/internal/netty/NettyHandler.java‎

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ public class NettyHandler extends ChannelInboundHandlerAdapter {
3131
private final boolean defaultHeaders;
3232
private final long maxRequestSize;
3333
private final int maxFormFields;
34-
private long contentLength;
3534
private long chunkSize;
3635
private final boolean http2;
3736
private NettyContext context;
@@ -83,7 +82,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
8382
router.match(context).execute(context);
8483
} else {
8584
// possibly body:
86-
contentLength = contentLength(req);
85+
long contentLength = contentLength(req);
8786
if (contentLength > 0 || isTransferEncodingChunked(req)) {
8887
context.httpDataFactory = new DefaultHttpDataFactory(bufferSize);
8988
context.httpDataFactory.setBaseDir(app.getTmpdir().toString());
@@ -93,20 +92,6 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
9392
router.match(context).execute(context);
9493
}
9594
}
96-
} else if (isLastHttpContent(msg)) {
97-
var chunk = (HttpContent) msg;
98-
try {
99-
// when decoder == null, chunk is always a LastHttpContent.EMPTY, ignore it
100-
if (context.decoder != null) {
101-
if (offer(context, chunk)) {
102-
Router.Match route = router.match(context);
103-
resetDecoderState(context, !route.matches());
104-
route.execute(context);
105-
}
106-
}
107-
} finally {
108-
release(chunk);
109-
}
11095
} else if (isHttpContent(msg)) {
11196
this.read = true;
11297
var chunk = (HttpContent) msg;
@@ -119,10 +104,13 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
119104
router.match(context).execute(context, Route.REQUEST_ENTITY_TOO_LARGE);
120105
return;
121106
}
122-
offer(context, chunk);
107+
if (offer(context, chunk) && isLastHttpContent(msg)) {
108+
Router.Match route = router.match(context);
109+
resetDecoderState(context, !route.matches());
110+
route.execute(context);
111+
}
123112
}
124113
} finally {
125-
// must be released
126114
release(chunk);
127115
}
128116
} else if (isWebSocketFrame(msg)) {
@@ -251,7 +239,6 @@ private boolean offer(NettyContext context, HttpContent chunk) {
251239

252240
private void resetDecoderState(NettyContext context, boolean destroy) {
253241
chunkSize = 0;
254-
contentLength = -1;
255242
if (destroy && context.decoder != null) {
256243
var decoder = context.decoder;
257244
var httpDataFactory = context.httpDataFactory;

‎tests/src/test/java/examples/ConcurrentTest.java‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@
1515
public class ConcurrentTest {
1616

1717
@ServerTest
18-
public void makeSureBufferWork(ServerTestRunner runner)
19-
throws ExecutionException, InterruptedException {
18+
public void makeSureBufferWork(ServerTestRunner runner) {
2019
var payload = "Hello World!";
2120
runner
2221
.define(
@@ -32,12 +31,7 @@ public void makeSureBufferWork(ServerTestRunner runner)
3231
})
3332
.ready(
3433
http -> {
35-
http.concurrent(
36-
"/plaintext",
37-
20,
38-
rsp -> {
39-
assertEquals(payload, rsp.body().string());
40-
});
34+
http.get("/plaintext").execute(50, rsp -> assertEquals(payload, rsp.body().string()));
4135
});
4236
}
4337
}

‎tests/src/test/java/io/jooby/i2806/Issue2806.java‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
*/
66
package io.jooby.i2806;
77

8+
import static io.jooby.test.TestUtil._19kb;
89
import static org.junit.jupiter.api.Assertions.assertEquals;
910

10-
import java.util.Arrays;
1111
import java.util.Map;
1212

1313
import com.google.common.collect.ImmutableMap;
@@ -23,9 +23,6 @@ public class Issue2806 {
2323

2424
@ServerTest
2525
public void renderShouldWorkFromErrorHandlerWhenLargeRequestAreSent(ServerTestRunner runner) {
26-
char[] chars = new char[19 * 1024];
27-
Arrays.fill(chars, 'S');
28-
String _19kb = new String(chars);
2926
runner
3027
.options(
3128
new ServerOptions()
@@ -43,20 +40,32 @@ public void renderShouldWorkFromErrorHandlerWhenLargeRequestAreSent(ServerTestRu
4340
ctx.render(map);
4441
});
4542

46-
app.post("/2806", ctx -> ctx.body().value(""));
43+
app.post(
44+
"/2806",
45+
ctx -> {
46+
return ctx.body().value("");
47+
});
4748

4849
app.get("/2806", ctx -> ctx.body().value(""));
4950
})
5051
.ready(
5152
client -> {
5253
// Exceeds
53-
client.post(
54-
"/2806",
55-
RequestBody.create(_19kb, MediaType.get("text/plain")),
56-
rsp -> {
57-
// assertEquals(413, rsp.code());
58-
assertEquals("{\"router\":true,\"route\":true}", rsp.body().string());
59-
});
54+
client
55+
.post("/2806", RequestBody.create(_19kb, MediaType.get("text/plain")))
56+
.execute(
57+
rsp -> {
58+
assertEquals(413, rsp.code());
59+
assertEquals("{\"router\":true,\"route\":true}", rsp.body().string());
60+
});
61+
62+
client
63+
.get("/2806")
64+
.execute(
65+
rsp -> {
66+
assertEquals(200, rsp.code());
67+
assertEquals("", rsp.body().string());
68+
});
6069
});
6170
}
6271
}

‎tests/src/test/java/io/jooby/test/WebClient.java‎

Lines changed: 31 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -134,17 +134,46 @@ public void close() {
134134

135135
public class Request {
136136
private final okhttp3.Request.Builder req;
137+
private SneakyThrows.Consumer<okhttp3.Request.Builder> configurer;
137138

138139
public Request(okhttp3.Request.Builder req) {
139140
this.req = req;
140141
}
141142

142143
public Request prepare(SneakyThrows.Consumer<okhttp3.Request.Builder> configurer) {
143-
configurer.accept(req);
144+
this.configurer = configurer;
144145
return this;
145146
}
146147

147148
public void execute(SneakyThrows.Consumer<Response> callback) {
149+
execute(1, callback);
150+
}
151+
152+
public void execute(int concurrency, SneakyThrows.Consumer<Response> callback) {
153+
if (configurer != null) {
154+
configurer.accept(req);
155+
}
156+
if (concurrency > 1) {
157+
var futures = new ArrayList<CompletableFuture<String>>();
158+
for (var i = 0; i < concurrency; i++) {
159+
futures.add(
160+
CompletableFuture.supplyAsync(
161+
() -> {
162+
executeCall(callback);
163+
return "success";
164+
}));
165+
try {
166+
CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])).join();
167+
} catch (CompletionException x) {
168+
throw SneakyThrows.propagate(x.getCause());
169+
}
170+
}
171+
} else {
172+
executeCall(callback);
173+
}
174+
}
175+
176+
private void executeCall(SneakyThrows.Consumer<Response> callback) {
148177
okhttp3.Request r = req.build();
149178
try (Response rsp = client.newCall(r).execute()) {
150179
callback.accept(rsp);
@@ -168,16 +197,12 @@ public WebClient(String scheme, int port, boolean followRedirects) {
168197
try {
169198
this.scheme = scheme;
170199
this.port = port;
171-
Dispatcher dispatcher = new Dispatcher();
172-
dispatcher.setMaxRequests(100); // Maximum 20 concurrent requests overall
173-
dispatcher.setMaxRequestsPerHost(100);
174200
OkHttpClient.Builder builder =
175201
new OkHttpClient.Builder()
176202
.connectTimeout(5, TimeUnit.MINUTES)
177203
.writeTimeout(5, TimeUnit.MINUTES)
178204
.readTimeout(5, TimeUnit.MINUTES)
179-
.followRedirects(followRedirects)
180-
.dispatcher(dispatcher);
205+
.followRedirects(followRedirects);
181206
if (scheme.equalsIgnoreCase("https")) {
182207
configureSelfSigned(builder);
183208
}
@@ -225,32 +250,6 @@ public Request get(String path) {
225250
return invoke("GET", path, null);
226251
}
227252

228-
public void concurrent(String path, int concurrency, SneakyThrows.Consumer<Response> callback) {
229-
var futures = new ArrayList<Future<String>>();
230-
try (var executor = Executors.newFixedThreadPool(concurrency + 5)) {
231-
for (var i = 0; i < concurrency; i++) {
232-
futures.add(
233-
executor.submit(
234-
() -> {
235-
okhttp3.Request.Builder req = new okhttp3.Request.Builder();
236-
req.url(scheme + "://localhost:" + port + path);
237-
new Request(req).execute(callback);
238-
return "OK";
239-
}));
240-
}
241-
for (Future<String> future : futures) {
242-
try {
243-
var result = future.get();
244-
if (!"OK".equals(result)) {
245-
throw new IllegalStateException(result);
246-
}
247-
} catch (Exception e) {
248-
SneakyThrows.propagate(e);
249-
}
250-
}
251-
}
252-
}
253-
254253
public ServerSentMessageIterator sse(String path) {
255254
okhttp3.Request.Builder req = new okhttp3.Request.Builder();
256255
setRequestHeaders(req);

0 commit comments

Comments
 (0)