Skip to content

Commit

Permalink
Merge pull request #1541 from gaol/UNDERTOW-2337
Browse files Browse the repository at this point in the history 
[UNDERTOW-2337] Multipart form-data larger than 16KiB is not available through Servlet getParameter API
  • Loading branch information
@fl4via
fl4via authored Feb 13, 2024
2 parents 489e145 + 765f69b commit 70cb55e
Show file tree
Hide file tree
Showing 9 changed files with 161 additions and 25 deletions.
2 changes: 1 addition & 1 deletion core/src/main/java/io/undertow/server/handlers/RequestDumpingHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ private void dumpRequestBody(HttpServerExchange exchange, StringBuilder sb) {
sb.append(formField)
.append("=");
for (FormData.FormValue formValue : formValues) {
sb.append(formValue.isFileItem() ? "[file-content]" : formValue.getValue());
sb.append(formValue.isFileItem() && !formValue.isBigField() ? "[file-content]" : formValue.getValue());
sb.append("\n");

if (formValue.getHeaders() != null) {
Expand Down
37 changes: 34 additions & 3 deletions core/src/main/java/io/undertow/server/handlers/form/FormData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.util.Map;

import io.undertow.UndertowMessages;
import io.undertow.util.FileUtils;
import io.undertow.util.HeaderMap;

/**
Expand Down Expand Up @@ -102,11 +103,15 @@ public void add(String name, String value, String charset, final HeaderMap heade
}

public void add(String name, Path value, String fileName, final HeaderMap headers) {
add(name, value, fileName, headers, false, null);
}

public void add(String name, Path value, String fileName, final HeaderMap headers, boolean bigField, String charset) {
Deque<FormValue> values = this.values.get(name);
if (values == null) {
this.values.put(name, values = new ArrayDeque<>(1));
}
values.add(new FormValueImpl(value, fileName, headers));
values.add(new FormValueImpl(value, fileName, headers, bigField, charset));
if (values.size() > maxValues) {
throw new RuntimeException(UndertowMessages.MESSAGES.tooManyParameters(maxValues));
}
Expand Down Expand Up @@ -211,6 +216,16 @@ public interface FormValue {
* @return The headers that were present in the multipart request, or null if this was not a multipart request
*/
HeaderMap getHeaders();

/**
* @return true if size of the FormValue comes from a multipart request exceeds the fieldSizeThreshold of
* {@link MultiPartParserDefinition} without filename specified.
*
* A big field is stored in disk, it is a file based FileItem.
*
* getValue() returns getCharset() decoded string from the file if it is true.
*/
boolean isBigField();
}

public static class FileItem {
Expand Down Expand Up @@ -284,13 +299,15 @@ static class FormValueImpl implements FormValue {
private final HeaderMap headers;
private final FileItem fileItem;
private final String charset;
private final boolean bigField;

FormValueImpl(String value, HeaderMap headers) {
this.value = value;
this.headers = headers;
this.fileName = null;
this.fileItem = null;
this.charset = null;
this.bigField = false;
}

FormValueImpl(String value, String charset, HeaderMap headers) {
Expand All @@ -299,14 +316,16 @@ static class FormValueImpl implements FormValue {
this.headers = headers;
this.fileName = null;
this.fileItem = null;
this.bigField = false;
}

FormValueImpl(Path file, final String fileName, HeaderMap headers) {
FormValueImpl(Path file, final String fileName, HeaderMap headers, boolean bigField, String charset) {
this.fileItem = new FileItem(file);
this.headers = headers;
this.fileName = fileName;
this.value = null;
this.charset = null;
this.charset = charset;
this.bigField = bigField;
}

FormValueImpl(byte[] data, String fileName, HeaderMap headers) {
Expand All @@ -315,12 +334,20 @@ static class FormValueImpl implements FormValue {
this.headers = headers;
this.value = null;
this.charset = null;
this.bigField = false;
}


@Override
public String getValue() {
if (value == null) {
if (bigField) {
try (InputStream inputStream = getFileItem().getInputStream()) {
return FileUtils.readFile(inputStream, this.charset);
} catch (IOException e) {
// ignore
}
}
throw UndertowMessages.MESSAGES.formValueIsAFile();
}
return value;
Expand Down Expand Up @@ -360,6 +387,10 @@ public FileItem getFileItem() {
return fileItem;
}

public boolean isBigField() {
return bigField;
}

@Override
public boolean isFileItem() {
return fileItem != null;
Expand Down
27 changes: 14 additions & 13 deletions core/src/main/java/io/undertow/server/handlers/form/MultiPartParserDefinition.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,8 +354,7 @@ public void endPart() {
if (fileName != null) {
data.add(currentName, file, fileName, headers);
} else {
final Path fileNamePath = file.getFileName();
data.add(currentName, file, fileNamePath != null ? fileNamePath.toString() : "", headers);
data.add(currentName, file, null, headers, true, getCharset());
}
file = null;
contentBytes.reset();
Expand All @@ -369,18 +368,8 @@ public void endPart() {
data.add(currentName, Arrays.copyOf(contentBytes.toByteArray(), contentBytes.size()), fileName, headers);
contentBytes.reset();
} else {


try {
String charset = defaultEncoding;
String contentType = headers.getFirst(Headers.CONTENT_TYPE);
if (contentType != null) {
String cs = Headers.extractQuotedValueFromHeader(contentType, "charset");
if (cs != null) {
charset = cs;
}
}

String charset = getCharset();
data.add(currentName, new String(contentBytes.toByteArray(), charset), charset, headers);
} catch (UnsupportedEncodingException e) {
throw new RuntimeException(e);
Expand All @@ -389,6 +378,18 @@ public void endPart() {
}
}

private String getCharset() {
String charset = defaultEncoding;
String contentType = headers.getFirst(Headers.CONTENT_TYPE);
if (contentType != null) {
String cs = Headers.extractQuotedValueFromHeader(contentType, "charset");
if (cs != null) {
charset = cs;
}
}
return charset;
}

public List<Path> getCreatedFiles() {
return createdFiles;
}
Expand Down
20 changes: 19 additions & 1 deletion core/src/main/java/io/undertow/util/FileUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.nio.charset.UnsupportedCharsetException;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
Expand Down Expand Up @@ -51,16 +53,32 @@ public static String readFile(URL url) {
}
}

public static String readFile(InputStream file, String charset) {
try {
Charset charSet = charset != null ? Charset.forName(charset) : StandardCharsets.UTF_8;
return readFile(file, charSet);
} catch (UnsupportedCharsetException e) {
throw new RuntimeException(e);
}
}

/**
* Reads the {@link InputStream file} and converting it to {@link String} using UTF-8 encoding.
*/
public static String readFile(InputStream file) {
return readFile(file, StandardCharsets.UTF_8);
}

/**
* Reads the {@link InputStream file} and converting it to {@link String} using <code>charSet</code> encoding.
*/
public static String readFile(InputStream file, Charset charSet) {
try (BufferedInputStream stream = new BufferedInputStream(file)) {
byte[] buff = new byte[1024];
StringBuilder builder = new StringBuilder();
int read;
while ((read = stream.read(buff)) != -1) {
builder.append(new String(buff, 0, read, StandardCharsets.UTF_8));
builder.append(new String(buff, 0, read, charSet));
}
return builder.toString();
} catch (IOException e) {
Expand Down
3 changes: 1 addition & 2 deletions core/src/test/java/io/undertow/server/handlers/form/MultipartFormDataParserTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,7 @@ public void testLargeContentWithoutFileNameWithSmallFileSizeThreshold() throws E

Assert.assertEquals("false", parsedResponse.get("in_memory"));
Assert.assertEquals(DigestUtils.md5Hex(Files.newInputStream(file.toPath())), parsedResponse.get("hash"));
Assert.assertTrue(parsedResponse.get("file_name").startsWith("undertow"));
Assert.assertTrue(parsedResponse.get("file_name").endsWith("upload"));
Assert.assertEquals(parsedResponse.get("file_name"), "null");
} finally {
if (!file.delete()) {
file.deleteOnExit();
Expand Down
10 changes: 5 additions & 5 deletions servlet/src/main/java/io/undertow/servlet/spec/HttpServletRequestImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ public String getParameter(final String name) {
final FormData parsedFormData = parseFormData();
if (parsedFormData != null) {
FormData.FormValue res = parsedFormData.getFirst(name);
if (res == null || res.isFileItem()) {
if (res == null || res.isFileItem() && !res.isBigField()) {
return null;
} else {
return res.getValue();
Expand All @@ -781,7 +781,7 @@ public Enumeration<String> getParameterNames() {
while (it.hasNext()) {
String name = it.next();
for(FormData.FormValue param : parsedFormData.get(name)) {
if(!param.isFileItem()) {
if(!param.isFileItem() || param.isBigField()) {
parameterNames.add(name);
break;
}
Expand All @@ -808,7 +808,7 @@ public String[] getParameterValues(final String name) {
Deque<FormData.FormValue> res = parsedFormData.get(name);
if (res != null) {
for (FormData.FormValue value : res) {
if(!value.isFileItem()) {
if(!value.isFileItem() || value.isBigField()) {
ret.add(value.getValue());
}
}
Expand Down Expand Up @@ -839,14 +839,14 @@ public Map<String, String[]> getParameterMap() {
if (arrayMap.containsKey(name)) {
ArrayList<String> existing = arrayMap.get(name);
for (final FormData.FormValue v : val) {
if(!v.isFileItem()) {
if(!v.isFileItem() || v.isBigField()) {
existing.add(v.getValue());
}
}
} else {
final ArrayList<String> values = new ArrayList<>();
for (final FormData.FormValue v : val) {
if(!v.isFileItem()) {
if(!v.isFileItem() || v.isBigField()) {
values.add(v.getValue());
}
}
Expand Down
4 changes: 4 additions & 0 deletions servlet/src/test/java/io/undertow/servlet/test/multipart/AddMultipartServetListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ public void contextInitialized(ServletContextEvent sce) {
ServletRegistration.Dynamic reg = sce.getServletContext().addServlet("added", new MultiPartServlet());
reg.addMapping("/added");
reg.setMultipartConfig(new MultipartConfigElement(System.getProperty("java.io.tmpdir")));

reg = sce.getServletContext().addServlet("getParam", new MultiPartServlet(true));
reg.addMapping("/getParam");
reg.setMultipartConfig(new MultipartConfigElement(System.getProperty("java.io.tmpdir")));
}

@Override
Expand Down
31 changes: 31 additions & 0 deletions servlet/src/test/java/io/undertow/servlet/test/multipart/MultiPartServlet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.Enumeration;
import java.util.TreeSet;

import io.undertow.util.Headers;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
Expand All @@ -37,6 +38,17 @@
*/
public class MultiPartServlet extends HttpServlet {

private final boolean getParam;
public MultiPartServlet() {
super();
this.getParam = false;
}

public MultiPartServlet(boolean getParam) {
super();
this.getParam = getParam;
}

@Override
protected void doPost(final HttpServletRequest req, final HttpServletResponse resp) throws ServletException, IOException {
try {
Expand All @@ -54,10 +66,29 @@ protected void doPost(final HttpServletRequest req, final HttpServletResponse re
Collection<String> headerNames = new TreeSet<>(part.getHeaderNames());
for (String header : headerNames) {
writer.println(header + ": " + part.getHeader(header));
if (header.equals(Headers.CONTENT_DISPOSITION_STRING)) {
final String parameterValue = part.getHeader(header);
String parameterName = parameterValue.substring(parameterValue.indexOf("name=") + "name=\"".length());
parameterName = parameterName.substring(0, parameterName.indexOf('\"'));
final String[] values = req.getParameterValues(parameterName);
if (values != null) {
for (String value: values) {
writer.println("value: " + value);
}
}
}
}
writer.println("size: " + part.getSize());
writer.println("content: " + FileUtils.readFile(part.getInputStream()));
}
if (getParam) {
Enumeration<String> paramNames = req.getParameterNames();
while (paramNames.hasMoreElements()) {
String name = paramNames.nextElement();
writer.println("param name: " + name);
writer.println("param value: " + req.getParameter(name));
}
}
} catch (Exception e) {
resp.getWriter().write("EXCEPTION: " + e.getClass());
}
Expand Down
Loading

0 comments on commit 70cb55e

Please sign in to comment.