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

Don't assume that multipart part without filename is always text #38714

Merged
merged 1 commit into from
Feb 12, 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
@@ -0,0 +1,67 @@
package io.quarkus.resteasy.reactive.server.test.multipart;

import static io.restassured.RestAssured.given;
import static org.assertj.core.api.Assertions.assertThat;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.util.function.Supplier;

import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.core.MediaType;

import org.jboss.resteasy.reactive.PartType;
import org.jboss.resteasy.reactive.RestForm;
import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;

public class MultipartBinaryWithoutFilenameTest {

@RegisterExtension
static QuarkusUnitTest test = new QuarkusUnitTest()
.setArchiveProducer(new Supplier<>() {
@Override
public JavaArchive get() {
return ShrinkWrap.create(JavaArchive.class)
.addClasses(MultipartDataInputTest.Resource.class, MultipartDataInputTest.Item.class,
MultipartDataInputTest.Result.class);
}
});
private final File IMAGE_FILE = new File("./src/test/resources/image.png");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the nice picture, but given that it's 5MB added to the source archives and history, perhaps we want to generate an arbitrary binary array in the code as a slimmer alternative?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I'll make it a lot smaller

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if something's gone wrong with refreshing this PR, but I don't see the changes :(

Copy link
Contributor Author

@geoand geoand Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the image, from 5MB to 30k :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Github UI does not agree…
Screenshot from 2024-02-12 11-43-23

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about now?


@Test
public void test() throws IOException {
byte[] bytes = given()
.contentType("multipart/form-data")
.multiPart("bytes", IMAGE_FILE, "application/png")
.when()
.post("/test")
.then()
.statusCode(200)
.extract().body().asByteArray();

assertThat(bytes).isEqualTo(Files.readAllBytes(IMAGE_FILE.toPath()));
}

@Path("/test")
public static class Resource {

@POST
public byte[] testMultipart(Input input) {
return input.bytes;
}
}

public static class Input {
@RestForm("bytes")
@PartType(MediaType.APPLICATION_OCTET_STREAM)
public byte[] bytes;

}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import jakarta.ws.rs.WebApplicationException;
import jakarta.ws.rs.container.CompletionCallback;
import jakarta.ws.rs.core.HttpHeaders;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;

import org.jboss.logging.Logger;
Expand Down Expand Up @@ -339,24 +340,35 @@ public void endPart() {
contentBytes.reset();
} else {

try {
String charset = defaultEncoding;
String contentType = headers.getFirst(HttpHeaders.CONTENT_TYPE);
if (contentType != null) {
String cs = HeaderUtil.extractQuotedValueFromHeader(contentType, "charset");
String contentType = headers.getFirst(HttpHeaders.CONTENT_TYPE);
if (isText(contentType)) {
try {
String charset = defaultEncoding;
String cs = contentType != null ? HeaderUtil.extractQuotedValueFromHeader(contentType, "charset")
: null;
if (cs != null) {
charset = cs;
}
}

data.add(currentName, contentBytes.toString(charset), charset, headers);
} catch (UnsupportedEncodingException e) {
throw new RuntimeException(e);
data.add(currentName, contentBytes.toString(charset), charset, headers);
} catch (UnsupportedEncodingException e) {
throw new RuntimeException(e);
}
} else {
data.add(currentName, Arrays.copyOf(contentBytes.toByteArray(), contentBytes.size()), null, headers);
}

contentBytes.reset();
}
}

private boolean isText(String contentType) {
if (contentType == null || contentType.isEmpty()) { // https://www.rfc-editor.org/rfc/rfc7578.html#section-4.4 says the default content-type if missing is text/plain
return true;
}
return MediaType.TEXT_PLAIN_TYPE.isCompatible(MediaType.valueOf(contentType));
}

public List<Path> getCreatedFiles() {
return createdFiles;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.jboss.resteasy.reactive.server.core.ResteasyReactiveRequestContext;
import org.jboss.resteasy.reactive.server.core.ServerSerialisers;
import org.jboss.resteasy.reactive.server.handlers.RequestDeserializeHandler;
import org.jboss.resteasy.reactive.server.multipart.FileItem;
import org.jboss.resteasy.reactive.server.multipart.FormValue;
import org.jboss.resteasy.reactive.server.multipart.MultipartFormDataInput;
import org.jboss.resteasy.reactive.server.multipart.MultipartPartReadingException;
Expand Down Expand Up @@ -90,7 +91,7 @@ private static Object read(MessageBodyReader<?> reader, String attributeName, Fo
@Override
public InputStream get() {
try {
return Files.newInputStream(value.getFileItem().getFile());
return value.getFileItem().getInputStream();
} catch (IOException e) {
throw new UncheckedIOException(e);
}
Expand Down Expand Up @@ -248,7 +249,11 @@ public static byte[] getByteArray(String formName, ResteasyReactiveRequestContex
}
if (value.isFileItem()) {
try {
return Files.readAllBytes(value.getFileItem().getFile());
FileItem fileItem = value.getFileItem();
if (fileItem.isInMemory()) {
return fileItem.getInputStream().readAllBytes();
}
return Files.readAllBytes(fileItem.getFile());
} catch (IOException e) {
throw new MultipartPartReadingException(e);
}
Expand Down
Loading