Skip to content

Commit cf33599

Browse files
triberraartriberraar
authored
Fix bug where you can't do @param List<Path> (#6430)
Motivation: This pr solves the bug where annotated services can't handle multiple files with the same form field name in a multi part form upload. This would solve this issue: #6419 Modifications: Use reflection in the annotatedValueResolver to allow list/sets of File/Path/MultipartFile. Result: - Closes #6419.
1 parent 21e4000 commit cf33599

File tree

2 files changed

+178
-1
lines changed

2 files changed

+178
-1
lines changed

‎core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,8 @@ protected EnumConverter<?> computeValue(Class<?> type) {
128128

129129
private static final List<RequestObjectResolver> defaultRequestObjectResolvers;
130130

131+
private static final Set<Type> fileTypes = ImmutableSet.of(File.class, Path.class, MultipartFile.class);
132+
131133
static {
132134
final ImmutableList.Builder<RequestObjectResolver> builder = ImmutableList.builderWithExpectedSize(4);
133135
builder.add((resolverContext, expectedResultType, expectedParameterizedResultType, beanFactoryId) -> {
@@ -473,7 +475,7 @@ private static AnnotatedValueResolver of(AnnotatedElement annotatedElement,
473475
}
474476
return ofQueryParamMap(name, annotatedElement, typeElement, type, description);
475477
}
476-
if (type == File.class || type == Path.class || type == MultipartFile.class) {
478+
if (fileTypes.contains(type) || isListOfFiles(typeElement)) {
477479
return ofFileParam(name, annotatedElement, typeElement, type, description);
478480
}
479481
if (pathParams.contains(name)) {
@@ -529,6 +531,31 @@ private static AnnotatedValueResolver of(AnnotatedElement annotatedElement,
529531
return null;
530532
}
531533

534+
private static boolean isListOfFiles(AnnotatedElement typeElement) {
535+
if (!(typeElement instanceof Parameter)) {
536+
return false;
537+
}
538+
final Type parameter = ((Parameter) typeElement).getParameterizedType();
539+
if (!(parameter instanceof ParameterizedType)) {
540+
return false;
541+
}
542+
final ParameterizedType parameterizedType = (ParameterizedType) parameter;
543+
final Type raw = parameterizedType.getRawType();
544+
if (!(raw instanceof Class<?>)) {
545+
return false;
546+
}
547+
final Class<?> rawClass = (Class<?>) raw;
548+
if (!List.class.isAssignableFrom(rawClass) && !Set.class.isAssignableFrom(rawClass)) {
549+
return false;
550+
}
551+
final Type[] args = parameterizedType.getActualTypeArguments();
552+
if (args.length != 1) {
553+
return false;
554+
}
555+
final Type arg = args[0];
556+
return fileTypes.contains(arg);
557+
}
558+
532559
static List<RequestObjectResolver> addToFirstIfExists(List<RequestObjectResolver> resolvers,
533560
List<RequestConverter> converters,
534561
DependencyInjector dependencyInjector) {

‎core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedServiceMultipartTest.java‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,23 @@
1616

1717
package com.linecorp.armeria.internal.server.annotation;
1818

19+
import static com.google.common.collect.ImmutableList.toImmutableList;
1920
import static com.google.common.collect.ImmutableMap.toImmutableMap;
2021
import static net.javacrumbs.jsonunit.fluent.JsonFluentAssert.assertThatJson;
2122
import static org.assertj.core.api.Assertions.assertThat;
2223

2324
import java.io.File;
2425
import java.io.IOException;
2526
import java.nio.charset.StandardCharsets;
27+
import java.util.List;
2628
import java.util.function.Function;
2729

2830
import org.junit.jupiter.api.Test;
2931
import org.junit.jupiter.api.extension.RegisterExtension;
3032
import org.junit.jupiter.params.ParameterizedTest;
3133
import org.junit.jupiter.params.provider.ValueSource;
3234

35+
import com.google.common.collect.ImmutableList;
3336
import com.google.common.collect.ImmutableMap;
3437
import com.google.common.io.Files;
3538

@@ -89,6 +92,80 @@ void testUploadFile(String path) throws Exception {
8992
"\"param1\":\"armeria\"}");
9093
}
9194

95+
@Test
96+
void testUploadMultipleFilesWithSameNameMultipart() {
97+
final Multipart multipart = Multipart.of(
98+
BodyPart.of(ContentDisposition.of("form-data", "files", "bar.txt"), "bar"),
99+
BodyPart.of(ContentDisposition.of("form-data", "files", "qux.txt"), "qux"),
100+
BodyPart.of(ContentDisposition.of("form-data", "files", "quz.txt"), MediaType.PLAIN_TEXT,
101+
"quz"),
102+
BodyPart.of(ContentDisposition.of("form-data", "params"), "hello"),
103+
BodyPart.of(ContentDisposition.of("form-data", "params"), "armeria")
104+
);
105+
106+
final AggregatedHttpResponse response =
107+
server.blockingWebClient().execute(
108+
multipart.toHttpRequest("/uploadWithFileParamSameNameMultipart")
109+
);
110+
111+
final ImmutableMap<String, List<String>> expected = ImmutableMap.of(
112+
"files", ImmutableList.of("fileName bar.txt data bar contentType application/octet-stream",
113+
"fileName qux.txt data qux contentType application/octet-stream",
114+
"fileName quz.txt data quz contentType text/plain"),
115+
"params", ImmutableList.of("hello",
116+
"armeria"
117+
)
118+
);
119+
assertThatJson(response.contentUtf8()).isEqualTo(expected);
120+
}
121+
122+
@Test
123+
void testUploadMultipleFilesWithSameNamePath() {
124+
final Multipart multipart = Multipart.of(
125+
BodyPart.of(ContentDisposition.of("form-data", "files", "bar.txt"), "bar"),
126+
BodyPart.of(ContentDisposition.of("form-data", "files", "qux.txt"), "qux"),
127+
BodyPart.of(ContentDisposition.of("form-data", "files", "quz.txt"), MediaType.PLAIN_TEXT,
128+
"quz"),
129+
BodyPart.of(ContentDisposition.of("form-data", "params"), "hello"),
130+
BodyPart.of(ContentDisposition.of("form-data", "params"), "armeria")
131+
);
132+
133+
final AggregatedHttpResponse response =
134+
server.blockingWebClient().execute(
135+
multipart.toHttpRequest("/uploadWithFileParamSameNamePath")
136+
);
137+
138+
final ImmutableMap<String, List<String>> expected = ImmutableMap.of(
139+
"files", ImmutableList.of("data bar", "data qux", "data quz"),
140+
"params", ImmutableList.of("hello", "armeria"
141+
)
142+
);
143+
assertThatJson(response.contentUtf8()).isEqualTo(expected);
144+
}
145+
146+
@Test
147+
void testUploadMultipleFilesWithSameNameFile() {
148+
final Multipart multipart = Multipart.of(
149+
BodyPart.of(ContentDisposition.of("form-data", "files", "bar.txt"), "bar"),
150+
BodyPart.of(ContentDisposition.of("form-data", "files", "qux.txt"), "qux"),
151+
BodyPart.of(ContentDisposition.of("form-data", "files", "quz.txt"), MediaType.PLAIN_TEXT,
152+
"quz"),
153+
BodyPart.of(ContentDisposition.of("form-data", "params"), "hello"),
154+
BodyPart.of(ContentDisposition.of("form-data", "params"), "armeria")
155+
);
156+
157+
final AggregatedHttpResponse response =
158+
server.blockingWebClient().execute(
159+
multipart.toHttpRequest("/uploadWithFileParamSameNameFile")
160+
);
161+
162+
final ImmutableMap<String, List<String>> expected = ImmutableMap.of(
163+
"files", ImmutableList.of("data bar", "data qux", "data quz"),
164+
"params", ImmutableList.of("hello", "armeria")
165+
);
166+
assertThatJson(response.contentUtf8()).isEqualTo(expected);
167+
}
168+
92169
@Test
93170
void emptyBodyPart() {
94171
final Multipart multipart = Multipart.of();
@@ -190,5 +267,78 @@ public HttpResponse uploadWithMultipartObject(Multipart multipart) {
190267
return HttpResponse.ofJson(body);
191268
}));
192269
}
270+
271+
@Blocking
272+
@Post
273+
@Path("/uploadWithFileParamSameNameMultipart")
274+
public HttpResponse uploadWithFileParamSameNameMultipart(
275+
@Param("params") List<String> params,
276+
@Param("files") List<MultipartFile> files) {
277+
final List<String> fileData = files
278+
.stream()
279+
.map(multipartFile -> {
280+
try {
281+
final String data = Files.asCharSource(multipartFile.file(),
282+
StandardCharsets.UTF_8)
283+
.read();
284+
return String.format("fileName %s data %s contentType %s",
285+
multipartFile.filename(), data,
286+
multipartFile.headers().contentType());
287+
} catch (IOException e) {
288+
throw new RuntimeException(e);
289+
}
290+
}).collect(toImmutableList());
291+
final ImmutableMap<String, List<String>> content =
292+
ImmutableMap.of("files", fileData,
293+
"params", params);
294+
return HttpResponse.ofJson(content);
295+
}
296+
297+
@Blocking
298+
@Post
299+
@Path("/uploadWithFileParamSameNamePath")
300+
public HttpResponse uploadWithFileParamSameNamePath(
301+
@Param("params") List<String> params,
302+
@Param("files") List<java.nio.file.Path> files) {
303+
final List<String> fileData = files
304+
.stream()
305+
.map(path -> {
306+
try {
307+
final String data = Files.asCharSource(path.toFile(), StandardCharsets.UTF_8)
308+
.read();
309+
return String.format("data %s", data);
310+
} catch (IOException e) {
311+
throw new RuntimeException(e);
312+
}
313+
}).collect(toImmutableList());
314+
final ImmutableMap<String, List<String>> content =
315+
ImmutableMap.of("files", fileData,
316+
"params", params);
317+
return HttpResponse.ofJson(content);
318+
}
319+
320+
@Blocking
321+
@Post
322+
@Path("/uploadWithFileParamSameNameFile")
323+
public HttpResponse uploadWithFileParamSameNameFile(
324+
@Param("params") List<String> params,
325+
@Param("files") List<File> files,
326+
RequestHeaders headers) {
327+
final List<String> fileData = files
328+
.stream()
329+
.map(file -> {
330+
try {
331+
final String data = Files.asCharSource(file, StandardCharsets.UTF_8)
332+
.read();
333+
return String.format("data %s", data);
334+
} catch (IOException e) {
335+
throw new RuntimeException(e);
336+
}
337+
}).collect(toImmutableList());
338+
final ImmutableMap<String, List<String>> content =
339+
ImmutableMap.of("files", fileData,
340+
"params", params);
341+
return HttpResponse.ofJson(content);
342+
}
193343
}
194344
}

0 commit comments

Comments
 (0)