Skip to content
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 @@ -62,6 +62,12 @@

private static final Logger LOG = LogManager.getLogger(HttpRequestWrapper.class);

// HTTP methods whose request may legitimately carry a request body. GET, HEAD and DELETE are
// deliberately excluded: a body on those methods has no defined semantics (RFC 9110 §9.3.1),
// and parsing one would expose attacker-suppliable multipart content (including file uploads)
// to handlers not written to expect a body.
private static final Set<String> METHODS_WITH_REQUEST_BODY = Set.of("POST", "PUT", "PATCH");

private static final Path TMP_DIR;
static {
try {
Expand Down Expand Up @@ -135,10 +141,14 @@
this.pathInfo = servletRequest.getPathInfo();
this.servletPath = servletRequest.getServletPath();

// Determine if request is a multipart

// Determine if request is a multipart. A multipart/form-data body may accompany any
// body-carrying method (e.g. PUT and PATCH, not just POST), so recognize the whole
// allow-list rather than POST alone — while still excluding GET/HEAD/DELETE (see
// METHODS_WITH_REQUEST_BODY). See https://github.com/eXist-db/exist/issues/6580 and
// https://github.com/eXist-db/exist/issues/6578
@Nullable final String contentType = servletRequest.getContentType();
isMultipartContent = "POST".equalsIgnoreCase(servletRequest.getMethod()) && contentType != null && contentType.toLowerCase(Locale.ENGLISH).startsWith("multipart/");
isMultipartContent = METHODS_WITH_REQUEST_BODY.contains(servletRequest.getMethod().toUpperCase(Locale.ENGLISH))
&& contentType != null && contentType.toLowerCase(Locale.ENGLISH).startsWith("multipart/");

// Get multi-part formdata parameters when it is a mpfd request
// and when instructed to do so
Expand Down Expand Up @@ -929,7 +939,7 @@
}

@Override
protected void finalize() {

Check warning on line 942 in exist-core/src/main/java/org/exist/http/servlets/HttpRequestWrapper.java

View workflow job for this annotation

GitHub Actions / Test and Publish Container Images

finalize() in java.lang.Object has been deprecated and marked for removal

Check warning on line 942 in exist-core/src/main/java/org/exist/http/servlets/HttpRequestWrapper.java

View workflow job for this annotation

GitHub Actions / W3C XQuery Test Suite

finalize() in java.lang.Object has been deprecated and marked for removal
if (temporaryUploadedFilesPathCache == null) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,11 @@ private SourceInfo getSource(final DBBroker broker, final String moduleLoadPath)
}

private void declareVariables(final XQueryContext context, final SourceInfo sourceInfo, final URLRewrite staticRewrite, final String basePath, final RequestWrapper request, final HttpServletResponse response) throws XPathException {
final HttpRequestWrapper reqw = new HttpRequestWrapper(request, UTF_8.name(), UTF_8.name(), false);
// parseMultipart=true so that multipart/form-data uploads (including file parts)
// are exposed to controllers and RESTXQ resource functions for every HTTP method,
// not only POST. See https://github.com/eXist-db/exist/issues/6580 and
// https://github.com/eXist-db/exist/issues/6578
final HttpRequestWrapper reqw = new HttpRequestWrapper(request, UTF_8.name(), UTF_8.name(), true);
final HttpResponseWrapper respw = new HttpResponseWrapper(response);
// context.declareNamespace(RequestModule.PREFIX,
// RequestModule.NAMESPACE_URI);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
/*
* eXist-db Open Source Native XML Database
* Copyright (C) 2001 The eXist-db Authors
*
* info@exist-db.org
* http://www.exist-db.org
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 2.1 of the License, or (at your option) any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
package org.exist.http.urlrewrite;

import org.exist.TestUtils;
import org.exist.http.AbstractHttpTest;
import org.exist.test.ExistWebServer;
import org.junit.ClassRule;
import org.junit.Test;

import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.URI;
import java.net.http.HttpRequest;

import static java.nio.charset.StandardCharsets.UTF_8;
import static org.exist.http.urlrewrite.XQueryURLRewrite.LEGACY_XQUERY_CONTROLLER_FILENAME;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

/**
* Reproduction for multipart/form-data parsing on methods other than POST
* (eXist-db/exist#6580, #6578), via a controller.xql that reports what the
* {@code request:} module can see of an identical multipart body — the same
* path third-party routers (e.g. roaster) take.
*/
public class MultipartMethodControllerTest extends AbstractHttpTest {

@ClassRule
public static final ExistWebServer existWebServer = new ExistWebServer(true, false, true, true, false);

private static final String BOUNDARY = "wdbBoundary";

private static final String MULTIPART_BODY =
"--" + BOUNDARY + "\r\n"
+ "Content-Disposition: form-data; name=\"path\"\r\n"
+ "\r\n"
+ "edition/01/17410105.xml\r\n"
+ "--" + BOUNDARY + "\r\n"
+ "Content-Disposition: form-data; name=\"file\"; filename=\"17410105.xml\"\r\n"
+ "Content-Type: application/xml\r\n"
+ "\r\n"
+ "<example>hello</example>\r\n"
+ "--" + BOUNDARY + "--\r\n";

private static final String CONTROLLER =
"""
xquery version "3.1";
<result method="{request:get-method()}"
is-multipart="{request:is-multipart-content()}"
param-names="{string-join(request:get-parameter-names(), ',')}"
path="{request:get-parameter('path', ())}"
file-param="{request:get-parameter('file', ())}"
uploaded-files="{string-join(request:get-uploaded-file-name('file'), ',')}"/>
""";

@Test
public void multipartFormDataIsParsedForBodyMethods() throws IOException {
final String coll = "multipart-method-controller";
store(coll, LEGACY_XQUERY_CONTROLLER_FILENAME, "application/xquery", CONTROLLER);

// A multipart/form-data body must be parsed identically for every body-carrying method:
// both the form field ("path") and the uploaded file ("file") must be visible, and
// request:is-multipart-content() must report true. Prior to the fix, PUT/PATCH reported
// is-multipart-content()=false and exposed neither the file nor its part (#6580),
// and the controller/RESTXQ path never exposed the uploaded file at all (#6578).
for (final String method : new String[]{"POST", "PUT", "PATCH"}) {
final String body = send(coll, method);
assertTrue(method + ": is-multipart-content() should be true: " + body,
body.contains("is-multipart=\"true\""));
assertTrue(method + ": form field 'path' should be visible: " + body,
body.contains("path=\"edition/01/17410105.xml\""));
assertTrue(method + ": uploaded file 'file' should be visible: " + body,
body.contains("uploaded-files=\"17410105.xml\""));
}
}

@Test
public void multipartFormDataIsNotParsedForGet() throws IOException {
final String coll = "multipart-method-controller-get";
store(coll, LEGACY_XQUERY_CONTROLLER_FILENAME, "application/xquery", CONTROLLER);

// GET is a safe method with no defined semantics for a request body (RFC 9110 §9.3.1),
// so a multipart/form-data body on GET must not be parsed: is-multipart-content() is false
// and no uploaded file is exposed to the handler. (Non-file form fields may still leak via
// the servlet container's parameter map — a pre-existing quirk this fix does not change.)
final String body = send(coll, "GET");
assertTrue("GET: is-multipart-content() must be false: " + body,
body.contains("is-multipart=\"false\""));
assertTrue("GET: uploaded file must not be exposed: " + body,
body.contains("uploaded-files=\"\""));
}

private void store(final String coll, final String name, final String mediaType, final String content) throws IOException {
final HttpRequest request = authenticatedRequest(
URI.create(getRestUri(existWebServer) + "/db/apps/" + coll + "/" + name),
TestUtils.ADMIN_DB_USER, TestUtils.ADMIN_DB_PWD)
.header("Content-Type", mediaType)
.PUT(HttpRequest.BodyPublishers.ofString(content))
.build();
final int status = withHttpClient(client -> executeForStatus(client, request));
assertEquals(HttpURLConnection.HTTP_CREATED, status);
}

private String send(final String coll, final String method) throws IOException {
final HttpRequest request = authenticatedRequest(
URI.create(getServerUri(existWebServer) + "/apps/" + coll + "/echo"),
TestUtils.ADMIN_DB_USER, TestUtils.ADMIN_DB_PWD)
.header("Content-Type", "multipart/form-data; boundary=" + BOUNDARY)
.method(method, HttpRequest.BodyPublishers.ofString(MULTIPART_BODY, UTF_8))
.build();
return withHttpClient(client -> executeForStatusAndBody(client, request).body());
}
}
Loading