Skip to content

Commit ac2f9f3

Browse files
jknackjknack
committed
fix: Reading body() before or after form() is not consistent across all servers
fixes #4031
1 parent 28cc2d7 commit ac2f9f3

4 files changed

Lines changed: 236 additions & 10 deletions

File tree

‎modules/jooby-jetty/src/main/java/io/jooby/internal/jetty/JettyContext.java‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import org.eclipse.jetty.io.ByteBufferPool;
4343
import org.eclipse.jetty.io.Content;
4444
import org.eclipse.jetty.io.content.InputStreamContentSource;
45+
import org.eclipse.jetty.server.FormFields;
4546
import org.eclipse.jetty.server.Request;
4647
import org.eclipse.jetty.server.Response;
4748
import org.eclipse.jetty.util.BufferUtil;
@@ -145,9 +146,12 @@ public Map<String, String> cookieMap() {
145146

146147
@Override
147148
public Body body() {
149+
if (isFormLike()) {
150+
return Body.empty(this);
151+
}
148152
InputStream in = Content.Source.asInputStream(request);
149153
long len = request.getLength();
150-
if (maxRequestSize > 0) {
154+
if (maxRequestSize > 0 && len > maxRequestSize) {
151155
in = new LimitedInputStream(in, maxRequestSize);
152156
}
153157
return Body.of(this, in, len);
@@ -210,6 +214,23 @@ public QueryString query() {
210214
return query;
211215
}
212216

217+
private boolean isFormLike() {
218+
var contentType = request.getHeaders().get(HttpHeader.CONTENT_TYPE);
219+
return isFormEncoded(contentType) || isMultipart(contentType);
220+
}
221+
222+
private boolean isFormEncoded(String contentType) {
223+
return MimeTypes.Type.FORM_ENCODED.is(contentType);
224+
}
225+
226+
private boolean isMultipart() {
227+
return isMultipart(request.getHeaders().get(HttpHeader.CONTENT_TYPE));
228+
}
229+
230+
private boolean isMultipart(String contentType) {
231+
return MimeTypes.Type.MULTIPART_FORM_DATA.is(HttpField.getValueParameters(contentType, null));
232+
}
233+
213234
@Override
214235
public Formdata form() {
215236
if (formdata == null) {
@@ -218,10 +239,8 @@ public Formdata form() {
218239
formParam(request, formdata);
219240

220241
// Files:
221-
String contentType = request.getHeaders().get(HttpHeader.CONTENT_TYPE);
222-
if (contentType != null
223-
&& MimeTypes.Type.MULTIPART_FORM_DATA.is(
224-
HttpField.getValueParameters(contentType, null))) {
242+
if (isMultipart()) {
243+
String contentType = request.getHeaders().get(HttpHeader.CONTENT_TYPE);
225244
// Is a multipart... make sure isn't empty
226245
String boundary = MultiPart.extractBoundary(contentType);
227246
if (boundary != null) {
@@ -724,7 +743,7 @@ private void register(DeleteFileTask deleteFileTask) {
724743

725744
private void formParam(Request request, Formdata form) {
726745
try {
727-
var params = Request.getParameters(request);
746+
var params = FormFields.getFields(request);
728747
for (Fields.Field param : params) {
729748
String name = param.getName();
730749
var values = param.getValues();

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -375,10 +375,21 @@ public Value header() {
375375

376376
@Override
377377
public Body body() {
378-
if (decoder != null && decoder.hasNext()) {
379-
return new NettyBody(this, (HttpData) decoder.next(), HttpUtil.getContentLength(req, -1L));
380-
}
381-
return Body.empty(this);
378+
return switch (decoder) {
379+
case HttpRawPostRequestDecoder rawDecoder -> {
380+
try {
381+
if (rawDecoder.hasNext()) {
382+
yield new NettyBody(
383+
this, (HttpData) rawDecoder.next(), HttpUtil.getContentLength(req, -1L));
384+
}
385+
} catch (HttpPostRequestDecoder.EndOfDataDecoderException ignored) {
386+
// ignore
387+
}
388+
yield Body.empty(this);
389+
}
390+
// no body or form-like
391+
default -> Body.empty(this);
392+
};
382393
}
383394

384395
@Override

‎tests/src/test/java/io/jooby/i4031/Issue4031.java‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
/*
2+
* Jooby https://jooby.io
3+
* Apache License Version 2.0 https://jooby.io/LICENSE.txt
4+
* Copyright 2014 Edgar Espina
5+
*/
6+
package io.jooby.i4031;
7+
8+
import java.util.Map;
9+
10+
import org.junit.jupiter.api.Assertions;
11+
12+
import io.jooby.exception.MissingValueException;
13+
import io.jooby.junit.ServerTest;
14+
import io.jooby.junit.ServerTestRunner;
15+
import okhttp3.FormBody;
16+
import okhttp3.MultipartBody;
17+
18+
class Issue4031 {
19+
20+
@ServerTest
21+
public void formDataAndQueryParameters(ServerTestRunner runner) {
22+
runner
23+
.define(
24+
app ->
25+
app.post(
26+
"/",
27+
ctx -> {
28+
try {
29+
Assertions.assertAll(
30+
() -> {
31+
Assertions.assertEquals(
32+
1, ctx.form().size(), "form contains one entry");
33+
Assertions.assertTrue(ctx.form().isPresent());
34+
Assertions.assertFalse(ctx.form().isMissing());
35+
Assertions.assertEquals("bar", ctx.form().get("foo").value());
36+
Assertions.assertEquals("bar", ctx.form("foo").value());
37+
Assertions.assertEquals("bar", ctx.form().toMap().get("foo"));
38+
Assertions.assertEquals("bar", ctx.formMap().get("foo"));
39+
40+
Assertions.assertEquals(
41+
"?parameter1=value1", ctx.query().queryString());
42+
Assertions.assertEquals("parameter1=value1", ctx.query().value());
43+
Assertions.assertEquals("value1", ctx.query("parameter1").value());
44+
Assertions.assertEquals("value1", ctx.queryMap().get("parameter1"));
45+
Assertions.assertEquals(1, ctx.queryMap().size());
46+
});
47+
return "OK";
48+
} catch (Exception ex) {
49+
ctx.setResponseCode(500);
50+
return ex.getMessage();
51+
}
52+
}))
53+
.ready(
54+
http -> {
55+
http.post(
56+
"/",
57+
new FormBody.Builder().add("foo", "bar").build(),
58+
Map.of("parameter1", "value1"),
59+
rsp -> {
60+
Assertions.assertEquals("OK", rsp.body().string());
61+
Assertions.assertEquals(200, rsp.code());
62+
});
63+
64+
http.post(
65+
"/",
66+
new MultipartBody.Builder()
67+
.addFormDataPart("foo", "bar")
68+
.setType(MultipartBody.FORM)
69+
.build(),
70+
Map.of("parameter1", "value1"),
71+
rsp -> {
72+
Assertions.assertEquals("OK", rsp.body().string());
73+
Assertions.assertEquals(200, rsp.code());
74+
});
75+
});
76+
}
77+
78+
@ServerTest
79+
public void readFormDataBeforeBody(ServerTestRunner runner) {
80+
runner
81+
.define(
82+
app ->
83+
app.post(
84+
"/",
85+
ctx -> {
86+
try {
87+
Assertions.assertAll(
88+
() -> {
89+
Assertions.assertEquals(
90+
1, ctx.form().size(), "form contains one entry");
91+
Assertions.assertTrue(ctx.form().isPresent());
92+
Assertions.assertFalse(ctx.form().isMissing());
93+
Assertions.assertEquals("bar", ctx.form().get("foo").value());
94+
Assertions.assertEquals("bar", ctx.form("foo").value());
95+
Assertions.assertEquals("bar", ctx.form().toMap().get("foo"));
96+
Assertions.assertEquals("bar", ctx.formMap().get("foo"));
97+
98+
// get body().value() after calling form methods
99+
Assertions.assertThrows(
100+
MissingValueException.class, () -> ctx.body().value());
101+
});
102+
return "OK";
103+
} catch (Exception ex) {
104+
ctx.setResponseCode(500);
105+
return ex.getMessage();
106+
}
107+
}))
108+
.ready(
109+
http -> {
110+
http.post(
111+
"/",
112+
new FormBody.Builder().add("foo", "bar").build(),
113+
rsp -> {
114+
Assertions.assertEquals("OK", rsp.body().string());
115+
Assertions.assertEquals(200, rsp.code());
116+
});
117+
118+
http.post(
119+
"/",
120+
new MultipartBody.Builder()
121+
.addFormDataPart("foo", "bar")
122+
.setType(MultipartBody.FORM)
123+
.build(),
124+
rsp -> {
125+
Assertions.assertEquals("OK", rsp.body().string());
126+
Assertions.assertEquals(200, rsp.code());
127+
});
128+
});
129+
}
130+
131+
@ServerTest
132+
public void readFormDataAfterBody(ServerTestRunner runner) {
133+
runner
134+
.define(
135+
app ->
136+
app.post(
137+
"/",
138+
ctx -> {
139+
try {
140+
Assertions.assertAll(
141+
() -> {
142+
// get body().value() before calling form methods
143+
Assertions.assertThrows(
144+
MissingValueException.class, () -> ctx.body().value());
145+
146+
Assertions.assertEquals(
147+
1, ctx.form().size(), "form contains one entry");
148+
Assertions.assertTrue(ctx.form().isPresent());
149+
Assertions.assertFalse(ctx.form().isMissing());
150+
Assertions.assertEquals("bar", ctx.form().get("foo").value());
151+
Assertions.assertEquals("bar", ctx.form("foo").value());
152+
Assertions.assertEquals("bar", ctx.form().toMap().get("foo"));
153+
Assertions.assertEquals("bar", ctx.formMap().get("foo"));
154+
});
155+
return "OK";
156+
} catch (Exception ex) {
157+
ex.printStackTrace();
158+
ctx.setResponseCode(500);
159+
return ex.getMessage();
160+
}
161+
}))
162+
.ready(
163+
http -> {
164+
http.post(
165+
"/",
166+
new FormBody.Builder().add("foo", "bar").build(),
167+
rsp -> {
168+
Assertions.assertEquals("OK", rsp.body().string());
169+
Assertions.assertEquals(200, rsp.code());
170+
});
171+
172+
http.post(
173+
"/",
174+
new MultipartBody.Builder()
175+
.setType(MultipartBody.FORM)
176+
.addFormDataPart("foo", "bar")
177+
.build(),
178+
rsp -> {
179+
Assertions.assertEquals("OK", rsp.body().string());
180+
Assertions.assertEquals(200, rsp.code());
181+
});
182+
});
183+
}
184+
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,18 @@ public void post(String path, SneakyThrows.Consumer<Response> callback) {
366366
post(path).execute(callback);
367367
}
368368

369+
public Request post(String path, RequestBody body, Map<String, Object> query) {
370+
return invoke("POST", path, query, body);
371+
}
372+
373+
public void post(
374+
String path,
375+
RequestBody form,
376+
Map<String, Object> query,
377+
SneakyThrows.Consumer<Response> callback) {
378+
post(path, form, query).execute(callback);
379+
}
380+
369381
public Request post(String path, RequestBody body) {
370382
return invoke("POST", path, body);
371383
}

0 commit comments

Comments
 (0)