Skip to content

Commit abe448c

Browse files
jknackjknack
committed
Headers: Fix parsing of multiple accept headers
- Fix parsing of multiple types on Accept header - Fix content negotation when multiple types are present in the Accept header - Fix jooby-project#2529
1 parent 2332bd5 commit abe448c

File tree

11 files changed

+156
-73
lines changed

11 files changed

+156
-73
lines changed

‎jooby/src/main/java/io/jooby/DefaultContext.java‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66
package io.jooby;
77

8+
import static java.util.Collections.singletonList;
89
import static java.util.Optional.ofNullable;
910

1011
import java.io.FileInputStream;
@@ -23,6 +24,7 @@
2324
import java.util.List;
2425
import java.util.Map;
2526
import java.util.Optional;
27+
import java.util.stream.Collectors;
2628

2729
import javax.annotation.Nonnull;
2830
import javax.annotation.Nullable;
@@ -203,23 +205,34 @@ public interface DefaultContext extends Context {
203205
}
204206

205207
@Override default boolean accept(@Nonnull MediaType contentType) {
206-
Value accept = header(ACCEPT);
207-
return accept.isMissing() || contentType.matches(accept.value());
208+
return accept(singletonList(contentType)) == contentType;
208209
}
209210

210211
@Override default MediaType accept(@Nonnull List<MediaType> produceTypes) {
211-
List<MediaType> acceptTypes = MediaType.parse(header(ACCEPT).valueOrNull());
212+
Value accept = header(ACCEPT);
213+
if (accept.isMissing()) {
214+
// NO header? Pick first, which is the default.
215+
return produceTypes.isEmpty() ? null : produceTypes.get(0);
216+
}
217+
218+
// Sort accept by most relevant/specific first:
219+
List<MediaType> acceptTypes = accept.toList().stream()
220+
.flatMap(value -> MediaType.parse(value).stream())
221+
.distinct()
222+
.sorted()
223+
.collect(Collectors.toList());
224+
225+
// Find most appropriated type:
226+
int idx = Integer.MAX_VALUE;
212227
MediaType result = null;
213-
for (MediaType acceptType : acceptTypes) {
214-
for (MediaType produceType : produceTypes) {
228+
for (MediaType produceType : produceTypes) {
229+
for (int i = 0; i < acceptTypes.size(); i++) {
230+
MediaType acceptType = acceptTypes.get(i);
215231
if (produceType.matches(acceptType)) {
216-
if (result == null) {
232+
if (i < idx) {
217233
result = produceType;
218-
} else {
219-
MediaType max = MediaType.MOST_SPECIFIC.apply(result, produceType);
220-
if (max != result) {
221-
result = max;
222-
}
234+
idx = i;
235+
break;
223236
}
224237
}
225238
}

‎jooby/src/main/java/io/jooby/MediaType.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -843,7 +843,6 @@ static boolean matches(@Nonnull String expected, @Nonnull String contentType) {
843843
default:
844844
return octetStream;
845845
}
846-
847846
}
848847

849848
private static boolean matchOne(String expected, int len1, String contentType) {

‎jooby/src/main/java/io/jooby/MessageEncoder.java‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,4 @@ public interface MessageEncoder {
3737
*/
3838
@Nullable byte[] encode(@Nonnull Context ctx, @Nonnull Object value) throws Exception;
3939

40-
/**
41-
* Execute this renderer only if the <code>Accept</code> header matches the content-type
42-
* parameter.
43-
*
44-
* @param contentType Mediatype to test.
45-
* @return A new renderer with accept header matching.
46-
*/
47-
@Nonnull default MessageEncoder accept(@Nonnull MediaType contentType) {
48-
return (ctx, value) -> {
49-
if (ctx.accept(contentType)) {
50-
return encode(ctx, value);
51-
}
52-
return null;
53-
};
54-
}
55-
5640
}

‎jooby/src/main/java/io/jooby/internal/HttpMessageEncoder.java‎

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,35 +5,42 @@
55
*/
66
package io.jooby.internal;
77

8-
import io.jooby.Context;
9-
import io.jooby.FileDownload;
10-
import io.jooby.MessageEncoder;
11-
import io.jooby.ModelAndView;
12-
import io.jooby.StatusCode;
13-
import io.jooby.TemplateEngine;
14-
15-
import javax.annotation.Nonnull;
168
import java.io.File;
179
import java.io.InputStream;
1810
import java.nio.ByteBuffer;
1911
import java.nio.channels.FileChannel;
2012
import java.nio.charset.StandardCharsets;
2113
import java.nio.file.Path;
2214
import java.util.ArrayList;
23-
import java.util.Iterator;
15+
import java.util.LinkedHashMap;
2416
import java.util.List;
17+
import java.util.Map;
18+
19+
import javax.annotation.Nonnull;
20+
21+
import io.jooby.Context;
22+
import io.jooby.FileDownload;
23+
import io.jooby.MediaType;
24+
import io.jooby.MessageEncoder;
25+
import io.jooby.ModelAndView;
26+
import io.jooby.StatusCode;
27+
import io.jooby.TemplateEngine;
2528

2629
public class HttpMessageEncoder implements MessageEncoder {
2730

28-
private List<MessageEncoder> encoderList = new ArrayList<>(2);
31+
private Map<MediaType, MessageEncoder> encoders;
2932

3033
private List<TemplateEngine> templateEngineList = new ArrayList<>(2);
3134

32-
public HttpMessageEncoder add(MessageEncoder encoder) {
35+
public HttpMessageEncoder add(MediaType type, MessageEncoder encoder) {
3336
if (encoder instanceof TemplateEngine) {
37+
// media type is ignored for template engines. They have a custom object type
3438
templateEngineList.add((TemplateEngine) encoder);
3539
} else {
36-
this.encoderList.add(encoder);
40+
if (encoders == null) {
41+
encoders = new LinkedHashMap<>();
42+
}
43+
encoders.put(type, encoder);
3744
}
3845
return this;
3946
}
@@ -91,13 +98,13 @@ public HttpMessageEncoder add(MessageEncoder encoder) {
9198
ctx.send((ByteBuffer) value);
9299
return null;
93100
}
94-
Iterator<MessageEncoder> iterator = encoderList.iterator();
95-
/** NOTE: looks like an infinite loop but there is a default renderer at the end of iterator. */
96-
byte[] result = null;
97-
while (result == null) {
98-
MessageEncoder next = iterator.next();
99-
result = next.encode(ctx, value);
101+
if (encoders != null) {
102+
// Content negotiation, find best:
103+
MediaType type = ctx.accept(new ArrayList<>(encoders.keySet()));
104+
MessageEncoder encoder = encoders.getOrDefault(type, MessageEncoder.TO_STRING);
105+
return encoder.encode(ctx, value);
106+
} else {
107+
return MessageEncoder.TO_STRING.encode(ctx, value);
100108
}
101-
return result;
102109
}
103110
}

‎jooby/src/main/java/io/jooby/internal/RouterImpl.java‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -325,18 +325,14 @@ public <T> Router mvc(@Nonnull Class<T> router, @Nonnull Provider<T> provider) {
325325
}
326326

327327
@Nonnull @Override public Router encoder(@Nonnull MessageEncoder encoder) {
328-
this.encoder.add(encoder);
328+
this.encoder.add(MediaType.all, encoder);
329329
return this;
330330
}
331331

332332
@Nonnull @Override
333333
public Router encoder(@Nonnull MediaType contentType, @Nonnull MessageEncoder encoder) {
334-
if (encoder instanceof TemplateEngine) {
335-
// Mime-Type is ignored for TemplateEngine due they depends on specific object type and file
336-
// extension.
337-
return encoder(encoder);
338-
}
339-
return encoder(encoder.accept(contentType));
334+
this.encoder.add(contentType, encoder);
335+
return this;
340336
}
341337

342338
@Nonnull @Override public Router decoder(@Nonnull MediaType contentType, @Nonnull
@@ -529,7 +525,6 @@ private Route newRoute(@Nonnull String method, @Nonnull String pattern,
529525
} else {
530526
err = err.then(ErrorHandler.create());
531527
}
532-
encoder.add(MessageEncoder.TO_STRING);
533528

534529
// Must be last, as fallback
535530
ValueConverters.addFallbackConverters(converters);

‎jooby/src/test/java/io/jooby/MediaTypeTest.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,10 @@ public void precedence() {
143143
assertEquals("text/*;q=0.3", types.get(3).toString());
144144
assertEquals("*/*;q=0.5", types.get(4).toString());
145145
});
146+
147+
accept("text/html, application/xhtml+xml, application/xml;q=0.9, image/webp, */*;q=0.8, application/json", types -> {
148+
System.out.println(types);
149+
});
146150
}
147151

148152
public static void accept(String value, Consumer<List<MediaType>> consumer) {

‎modules/jooby-bom/pom.xml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
<!-- THIS FILE IS AUTO GENERATED. DON'T EDIT -->
1717

1818
<properties>
19-
<jooby.version>2.13.0</jooby.version>
19+
<jooby.version>2.13.1-SNAPSHOT</jooby.version>
2020
<HikariCP.version>4.0.3</HikariCP.version>
2121
<archetype-packaging.version>3.2.0</archetype-packaging.version>
2222
<asm.version>9.1</asm.version>
@@ -60,8 +60,8 @@
6060
<jetty.version>9.4.44.v20210927</jetty.version>
6161
<jfiglet.version>0.0.8</jfiglet.version>
6262
<jmespath-java.version>1.12.57</jmespath-java.version>
63-
<jooby-maven-plugin.version>2.13.0</jooby-maven-plugin.version>
64-
<jooby.version>2.13.0</jooby.version>
63+
<jooby-maven-plugin.version>2.13.1-SNAPSHOT</jooby-maven-plugin.version>
64+
<jooby.version>2.13.1-SNAPSHOT</jooby.version>
6565
<jsonwebtoken.version>0.11.2</jsonwebtoken.version>
6666
<jsr305.version>3.0.2</jsr305.version>
6767
<junit.version>5.8.1</junit.version>

‎tests/src/test/java/io/jooby/WebClient.java‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ public void execute(SneakyThrows.Consumer<Response> callback) {
140140
private String scheme;
141141
private final int port;
142142
private OkHttpClient client;
143-
private Map<String, String> headers;
143+
private Headers.Builder headers;
144144

145145
public WebClient(String scheme, int port, boolean followRedirects) {
146146
try {
@@ -155,18 +155,18 @@ public WebClient(String scheme, int port, boolean followRedirects) {
155155
configureSelfSigned(builder);
156156
}
157157
this.client = builder.build();
158-
header("Accept",
159-
"text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8");
160158
} catch (Exception x) {
161159
throw SneakyThrows.propagate(x);
162160
}
163161
}
164162

165163
public WebClient header(String name, String value) {
166164
if (headers == null) {
167-
headers = new HashMap<>();
165+
headers = new Headers.Builder();
166+
}
167+
if (value != null && value.trim().length() > 0) {
168+
headers.add(name, value);
168169
}
169-
headers.put(name, value);
170170
return this;
171171
}
172172

@@ -183,11 +183,15 @@ public Request invoke(String method, String path, RequestBody body) {
183183
}
184184

185185
private void setRequestHeaders(okhttp3.Request.Builder req) {
186-
if (headers != null) {
187-
req.headers(Headers.of(headers));
188-
headers = null;
186+
if (headers == null) {
187+
// set default headers:
189188
header("Accept",
190189
"text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8");
190+
191+
}
192+
if (headers != null) {
193+
req.headers(headers.build());
194+
headers = null;
191195
}
192196
}
193197

‎tests/src/test/java/io/jooby/i2413/Issue2413.java‎

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,37 @@ public void shouldGenerateJsonAndXmlDefaultsJson(ServerTestRunner runner) {
2323

2424
http.header("Accept", "application/json")
2525
.get("/2413", rsp -> {
26-
assertEquals("application/json;charset=utf-8", rsp.body().contentType().toString().toLowerCase());
26+
assertEquals("application/json;charset=utf-8",
27+
rsp.body().contentType().toString().toLowerCase());
2728
assertEquals("{\"id\":\"someId\",\"name\":\"someName\"}", rsp.body().string());
2829
});
2930

3031
http.header("Accept", "application/xml")
3132
.get("/2413", rsp -> {
32-
assertEquals("application/xml;charset=utf-8", rsp.body().contentType().toString().toLowerCase());
33+
assertEquals("application/xml;charset=utf-8",
34+
rsp.body().contentType().toString().toLowerCase());
3335
assertEquals("<B2413><id>someId</id><name>someName</name></B2413>",
3436
rsp.body().string());
3537
});
3638

37-
http
39+
http.header("Accept", "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8")
3840
.get("/2413", rsp -> {
39-
assertEquals("application/json;charset=utf-8", rsp.body().contentType().toString().toLowerCase());
41+
assertEquals("application/xml;charset=utf-8",
42+
rsp.body().contentType().toString().toLowerCase());
43+
assertEquals("<B2413><id>someId</id><name>someName</name></B2413>", rsp.body().string());
44+
});
45+
46+
http.header("Accept", "*/*")
47+
.get("/2413", rsp -> {
48+
assertEquals("application/json;charset=utf-8",
49+
rsp.body().contentType().toString().toLowerCase());
50+
assertEquals("{\"id\":\"someId\",\"name\":\"someName\"}", rsp.body().string());
51+
});
52+
53+
http.header("Accept", null)
54+
.get("/2413", rsp -> {
55+
assertEquals("application/json;charset=utf-8",
56+
rsp.body().contentType().toString().toLowerCase());
4057
assertEquals("{\"id\":\"someId\",\"name\":\"someName\"}", rsp.body().string());
4158
});
4259
});
@@ -54,23 +71,47 @@ public void shouldGenerateJsonAndXmlDefaultsXml(ServerTestRunner runner) {
5471

5572
http.header("Accept", "application/json")
5673
.get("/2413", rsp -> {
57-
assertEquals("application/json;charset=utf-8", rsp.body().contentType().toString().toLowerCase());
74+
assertEquals("application/json;charset=utf-8",
75+
rsp.body().contentType().toString().toLowerCase());
5876
assertEquals("{\"id\":\"someId\",\"name\":\"someName\"}", rsp.body().string());
5977
});
6078

6179
http.header("Accept", "application/xml")
6280
.get("/2413", rsp -> {
63-
assertEquals("application/xml;charset=utf-8", rsp.body().contentType().toString().toLowerCase());
81+
assertEquals("application/xml;charset=utf-8",
82+
rsp.body().contentType().toString().toLowerCase());
6483
assertEquals("<B2413><id>someId</id><name>someName</name></B2413>",
6584
rsp.body().string());
6685
});
6786

6887
http
6988
.get("/2413", rsp -> {
70-
assertEquals("application/xml;charset=utf-8", rsp.body().contentType().toString().toLowerCase());
89+
assertEquals("application/xml;charset=utf-8",
90+
rsp.body().contentType().toString().toLowerCase());
7191
assertEquals("<B2413><id>someId</id><name>someName</name></B2413>",
7292
rsp.body().string());
7393
});
94+
95+
http.header("Accept", "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8")
96+
.get("/2413", rsp -> {
97+
assertEquals("application/xml;charset=utf-8",
98+
rsp.body().contentType().toString().toLowerCase());
99+
assertEquals("<B2413><id>someId</id><name>someName</name></B2413>", rsp.body().string());
100+
});
101+
102+
http.header("Accept", "*/*")
103+
.get("/2413", rsp -> {
104+
assertEquals("application/xml;charset=utf-8",
105+
rsp.body().contentType().toString().toLowerCase());
106+
assertEquals("<B2413><id>someId</id><name>someName</name></B2413>", rsp.body().string());
107+
});
108+
109+
http.header("Accept", null)
110+
.get("/2413", rsp -> {
111+
assertEquals("application/xml;charset=utf-8",
112+
rsp.body().contentType().toString().toLowerCase());
113+
assertEquals("<B2413><id>someId</id><name>someName</name></B2413>", rsp.body().string());
114+
});
74115
});
75116
}
76117
}

‎tests/src/test/java/io/jooby/i2529/Controller2529.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package io.jooby.i2529;
2+
3+
import io.jooby.annotations.GET;
4+
import io.jooby.annotations.Path;
5+
import io.jooby.annotations.Produces;
6+
7+
public class Controller2529 {
8+
@GET
9+
@Path("/2529")
10+
@Produces("text/plain")
11+
public String hello() {
12+
return "Hello world";
13+
}
14+
}

0 commit comments

Comments
 (0)