Skip to content
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

[UNDERTOW-2337] Multipart form-data larger than 16KiB is not available through Servlet getParameter API #1541

Merged
merged 2 commits into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
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
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
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
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
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
Loading