Skip to content

Commit

Permalink
Missing Content-Type header should be application/octet-stream option…
Browse files Browse the repository at this point in the history
…ally (#4911)

Signed-off-by: jansupol <[email protected]>
  • Loading branch information
jansupol authored Nov 22, 2021
1 parent 60089c8 commit fee1c32
Show file tree
Hide file tree
Showing 47 changed files with 164 additions and 288 deletions.
2 changes: 1 addition & 1 deletion bundles/jaxrs-ri/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-shade-plugin</artifactId>
<version>3.1.0</version>
<version>3.2.4</version>
<executions>
<execution>
<phase>package</phase>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -763,35 +763,20 @@ public final class ServerProperties {

/**
* <p>
* When an HTTP request does not contain a media type, the media type should default to {@code application/octet-stream}.
* In the past, Jersey used to match any {@code @Consumes} when the HTTP {@code Content-Type} header was not set.
* This property is to preserve the behaviour. Such behaviour is potentially dangerous, though.
* The default behaviour is to set the request media type to {@code application/octet-stream} when none set.
* This is a Jakarta REST requirement.
* </p>
* <p>
* This change can be can be eminent especially for HTTP resource methods which do not expect an entity, such as HTTP GET:
* <pre>
* {@code
* @Consumes(MediaType.TEXT_PLAIN)
* @Produces(MediaType.TEXT_PLAIN)
* @Path("/")
* public class Resource {
* @GET
* public String get() {
* return ...
* }
* }
* }
* </pre>
* The client request needs to contain the {@code Content-Type} HTTP header, for instance:
* {@code
* webTarget.request().header(HttpHeaders.CONTENT_TYPE, MediaType.TEXT_PLAIN).get()
* }
* </p>
* <p>
* Set this property to true, if the empty request media type is to match any {@code @Consumes}. The default is {@code false}.
* The name of the configuration property is <tt>{@value}</tt>.
* Jakarta RESTful WebServices provides {@code @Consumes} annotation to accept only HTTP requests with compatible
* {@code Content-Type} header. However, when the header is missing a wildcard media type is used to
* match the {@code @Consumes} annotation.
* </p>
* <p><a href="https://datatracker.ietf.org/doc/html/rfc7231#section-3.1.1.5">HTTP/1.1 RFC</a>recommends that missing
* {@code Content-Type} header MAY default to {@code application/octet-stream}. This property makes Jersey consider the
* missing HTTP {@code Content-Type} header to be {@code application/octet-stream} rather than a wildcard
* media type. However, for a resource method without an entity argument, such as for HTTP GET, a wildcard media type
* is still considered to accept the HTTP request for the missing HTTP {@code Content-Type} header.
* </p>
* <p>
* Set this property to false, if the empty request media type should not to match applied {@code @Consumes} annotation
* on a resource method with an entity argument. The default is {@code true}. The name of the configuration property is
* <tt>{@value}</tt>.
* </p>
* @since 3.1.0
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@

abstract class AbstractMethodSelectingRouter extends ContentTypeDeterminer implements Router {

private static final Logger LOGGER = Logger.getLogger(MethodSelectingRouter.class.getName());
private static final Logger LOGGER = Logger.getLogger(AbstractMethodSelectingRouter.class.getName());

private static final Comparator<ConsumesProducesAcceptor> CONSUMES_PRODUCES_ACCEPTOR_COMPARATOR =
new Comparator<ConsumesProducesAcceptor>() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.glassfish.jersey.internal.routing.CombinedMediaType;
import org.glassfish.jersey.message.MessageBodyWorkers;
import org.glassfish.jersey.server.ContainerRequest;
import org.glassfish.jersey.server.model.ResourceMethod;

/**
* A single router responsible for selecting a single method from all the methods
Expand All @@ -29,11 +30,8 @@
* The method selection algorithm selects the handling method based on the HTTP request
* method name, requested media type as well as defined resource method media type
* capabilities.
*
* @author Jakub Podlesak
* @author Marek Potociar
*/
final class MethodSelectingRouter extends AbstractMethodSelectingRouter implements Router {
final class OctetStreamMethodSelectingRouter extends AbstractMethodSelectingRouter implements Router {

/**
* Create a new {@code MethodSelectingRouter} for all the methods on the same path.
Expand All @@ -44,7 +42,7 @@ final class MethodSelectingRouter extends AbstractMethodSelectingRouter implemen
* @param workers message body workers.
* @param methodRoutings [method model, method methodAcceptorPair] pairs.
*/
MethodSelectingRouter(MessageBodyWorkers workers, List<MethodRouting> methodRoutings) {
OctetStreamMethodSelectingRouter(MessageBodyWorkers workers, List<MethodRouting> methodRoutings) {
super(workers, methodRoutings);
}

Expand Down Expand Up @@ -72,8 +70,11 @@ private ConsumesProducesAcceptor(
*/
boolean isConsumable(ContainerRequest requestContext) {
MediaType contentType = requestContext.getMediaType();
contentType = contentType == null ? MediaType.APPLICATION_OCTET_STREAM_TYPE : contentType;
return consumes.getMediaType().isCompatible(contentType);
if (contentType == null && methodRouting.method.getType() != ResourceMethod.JaxrsType.SUB_RESOURCE_LOCATOR
&& methodRouting.method.getInvocable().requiresEntity()) {
contentType = MediaType.APPLICATION_OCTET_STREAM_TYPE;
}
return contentType == null || consumes.getMediaType().isCompatible(contentType);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Properties;
import java.util.function.Function;

import jakarta.ws.rs.core.Configuration;
Expand Down Expand Up @@ -57,7 +56,7 @@ final class RuntimeModelBuilder {

// SubResourceLocator Model Builder.
private final Value<RuntimeLocatorModelBuilder> locatorBuilder;
private final boolean is2xMethodSelectingRouter;
private final boolean isWildcardMethodSelectingRouter;

/**
* Create a new instance of the runtime model builder.
Expand Down Expand Up @@ -86,8 +85,8 @@ public RuntimeModelBuilder(
this.locatorBuilder = Values.lazy((Value<RuntimeLocatorModelBuilder>)
() -> new RuntimeLocatorModelBuilder(config, messageBodyWorkers, valueSuppliers, resourceContext,
RuntimeModelBuilder.this, modelProcessors, createServiceFunction));
this.is2xMethodSelectingRouter = ServerProperties.getValue(config.getProperties(),
ServerProperties.EMPTY_REQUEST_MEDIA_TYPE_MATCHES_ANY_CONSUMES, false);
this.isWildcardMethodSelectingRouter = ServerProperties.getValue(config.getProperties(),
ServerProperties.EMPTY_REQUEST_MEDIA_TYPE_MATCHES_ANY_CONSUMES, true);
}

private Router createMethodRouter(final ResourceMethod resourceMethod) {
Expand Down Expand Up @@ -154,9 +153,9 @@ public Router buildModel(final RuntimeResourceModel resourceModel, final boolean
// resource methods
if (!resource.getResourceMethods().isEmpty()) {
final List<MethodRouting> methodRoutings = createResourceMethodRouters(resource, subResourceMode);
final Router methodSelectingRouter = is2xMethodSelectingRouter
? new MethodSelectingRouter2x(messageBodyWorkers, methodRoutings)
: new MethodSelectingRouter(messageBodyWorkers, methodRoutings);
final Router methodSelectingRouter = isWildcardMethodSelectingRouter
? new WildcardMethodSelectingRouter(messageBodyWorkers, methodRoutings)
: new OctetStreamMethodSelectingRouter(messageBodyWorkers, methodRoutings);
if (subResourceMode) {
currentRouterBuilder = startNextRoute(currentRouterBuilder, PathPattern.END_OF_PATH_PATTERN)
.to(resourcePushingRouter)
Expand Down Expand Up @@ -185,9 +184,9 @@ public Router buildModel(final RuntimeResourceModel resourceModel, final boolean
srRoutedBuilder = startNextRoute(srRoutedBuilder, childClosedPattern)
.to(uriPushingRouter)
.to(childResourcePushingRouter)
.to(is2xMethodSelectingRouter
? new MethodSelectingRouter2x(messageBodyWorkers, childMethodRoutings)
: new MethodSelectingRouter(messageBodyWorkers, childMethodRoutings));
.to(isWildcardMethodSelectingRouter
? new WildcardMethodSelectingRouter(messageBodyWorkers, childMethodRoutings)
: new OctetStreamMethodSelectingRouter(messageBodyWorkers, childMethodRoutings));
}

// sub resource locator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
* @author Jakub Podlesak
* @author Marek Potociar
*/
final class MethodSelectingRouter2x extends AbstractMethodSelectingRouter implements Router {
final class WildcardMethodSelectingRouter extends AbstractMethodSelectingRouter implements Router {

/**
* Create a new {@code MethodSelectingRouter} for all the methods on the same path.
Expand All @@ -44,7 +44,7 @@ final class MethodSelectingRouter2x extends AbstractMethodSelectingRouter implem
* @param workers message body workers.
* @param methodRoutings [method model, method methodAcceptorPair] pairs.
*/
MethodSelectingRouter2x(MessageBodyWorkers workers, List<MethodRouting> methodRoutings) {
WildcardMethodSelectingRouter(MessageBodyWorkers workers, List<MethodRouting> methodRoutings) {
super(workers, methodRoutings);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ private ApplicationHandler createApplication(Class<?>... classes) {
return new ApplicationHandler(new ResourceConfig(classes));
}

private ApplicationHandler create2xApplication(Class<?>... classes) {
private ApplicationHandler createAppOctetStreamApplication(Class<?>... classes) {
return new ApplicationHandler(
new ResourceConfig(classes).property(ServerProperties.EMPTY_REQUEST_MEDIA_TYPE_MATCHES_ANY_CONSUMES, true)
new ResourceConfig(classes).property(ServerProperties.EMPTY_REQUEST_MEDIA_TYPE_MATCHES_ANY_CONSUMES, false)
);
}

Expand Down Expand Up @@ -154,7 +154,7 @@ public void testProduceSimpleBean() throws Exception {
assertEquals("XHTML",
app.apply(RequestContextBuilder.from("/a/b", "GET").accept("text/xhtml").build()).get().getEntity());

app = create2xApplication(ProduceSimpleBean.class);
app = createAppOctetStreamApplication(ProduceSimpleBean.class);

assertEquals("HTML", app.apply(RequestContextBuilder.from("/a/b", "GET").accept("text/html").build()).get().getEntity());
assertEquals("XHTML",
Expand All @@ -172,28 +172,38 @@ public void testConsumeProduceSimpleBean() throws Exception {
app.apply(RequestContextBuilder.from("/a/b", "POST").entity("").type("text/xhtml").accept("text/xhtml").build())
.get().getEntity());

assertEquals(415,
app.apply(RequestContextBuilder.from("/a/b", "GET").accept("text/html").build())
.get().getStatus()
assertEquals("HTML",
app.apply(RequestContextBuilder.from("/a/b", "GET").accept("text/html").build()).get().getEntity()
);
assertEquals("HTML",
app.apply(RequestContextBuilder.from("/a/b", "GET").type("text/html").accept("text/html").build())
.get().getEntity()
);

assertEquals(415,
app.apply(RequestContextBuilder.from("/a/b", "GET").accept("text/xhtml").build())
.get().getStatus()
assertEquals("XHTML",
app.apply(RequestContextBuilder.from("/a/b", "GET").accept("text/xhtml").build()).get().getEntity()
);
assertEquals("XHTML",
app.apply(RequestContextBuilder.from("/a/b", "GET").type("text/xhtml").accept("text/xhtml").build())
.get().getEntity()
);

app = create2xApplication(ConsumeProduceSimpleBean.class);
app = createAppOctetStreamApplication(ConsumeProduceSimpleBean.class);
assertEquals("HTML", app.apply(RequestContextBuilder.from("/a/b", "GET").accept("text/html").build()).get().getEntity());
assertEquals("XHTML",
app.apply(RequestContextBuilder.from("/a/b", "GET").accept("text/xhtml").build()).get().getEntity());

assertEquals(415,
app.apply(RequestContextBuilder.from("/a/b", "POST").entity("").accept("text/html").build()).get().getStatus());
assertEquals(415,
app.apply(RequestContextBuilder.from("/a/b", "POST").entity("").accept("text/xhtml").build()).get().getStatus());

assertEquals("HTML",
app.apply(RequestContextBuilder.from("/a/b", "POST").entity("").type("text/html").accept("text/html").build())
.get().getEntity());
assertEquals("XHTML",
app.apply(RequestContextBuilder.from("/a/b", "POST").entity("").type("text/xhtml").accept("text/xhtml").build())
.get().getEntity());
}

@Path("/")
Expand Down
22 changes: 14 additions & 8 deletions docs/src/main/docbook/appendix-properties.xml
Original file line number Diff line number Diff line change
Expand Up @@ -277,16 +277,22 @@
<entry><literal>jersey.config.server.empty.request.media.matches.any.consumes</literal></entry>
<entry>
<para>
When an HTTP request does not contain a media type, the media type should default to
<literal>application/octet-stream</literal>. In the past, Jersey used to match any
<literal>@Consumes</literal> when the HTTP <literal>Content-Type</literal> header was not set.
This property is to preserve the behaviour. Such behaviour is potentially dangerous, though.
The default behaviour is to set the request media type to
<literal>application/octet-stream</literal> when none set. This is a Jakarta REST requirement.
Jakarta RESTful WebServices provides <literal>@Consumes</literal> annotation to accept only HTTP
requests with compatible HTTP <literal>Content-Type</literal> header. However, when the header
is missing a wildcard media type is used to match the <literal>@Consumes</literal> annotation.
</para>
<para>
Set this property to <literal>true</literal>, if the empty request media type is to match any
<literal>@Consumes</literal>. The default value is <literal>false</literal>.
HTTP/1.1 RFC recommends that missing HTTP <literal>Content-Type</literal> header MAY default to
<literal>application/octet-stream</literal>. This property makes Jersey consider the missing HTTP
<literal>Content-Type</literal> header to be <literal>application/octet-stream</literal> rather
than a wildcard media type. However, for a resource method without an entity argument, such as
for HTTP GET, a wildcard media type is still considered to accept the HTTP request for
the missing HTTP <literal>Content-Type</literal> header.
</para>
<para>
Set this property to <literal>false</literal>, if the empty request media type should not to match
applied <literal>@Consumes</literal> annotation on a resource method with an entity argument. The
default is <literal>true</literal>.
</para>
</entry>
</row>
Expand Down
16 changes: 0 additions & 16 deletions docs/src/main/docbook/migration.xml
Original file line number Diff line number Diff line change
Expand Up @@ -96,22 +96,6 @@
supports JDK 8.
</para>
</listitem>
<listitem>
<para>
Since Jersey 3.1.0 when incoming the HTTP request does not contain content type, the
<literal>application/octet-stream</literal> is considered, as required by the Jakarta REST
Specification and HTTP/1.1 RFC 2616. As a consequence, a resource method with non-wildcard
<literal>@Consumes</literal> annotation is not matched with the request without the
Content-Type HTTP header. This is typically the case of HTTP requests without an entity,
such as HTTP GET requests, where the Content-Type header is not set.
</para>
<para>
When using the <literal>@Consumes</literal> annotation, the Content-Type HTTP header must be set.
The previous behaviour when the missing Content-Type HTTP header matched any resource method
with <literal>@Consumes</literal> annotation, the
&jersey.server.ServerProperties.EMPTY_REQUEST_MEDIA_TYPE_MATCHES_ANY_CONSUMES; property can be set.
</para>
</listitem>
<listitem>
<para>
&jersey.server.ServerProperties.UNWRAP_COMPLETION_STAGE_IN_WRITER_ENABLE; is by default
Expand Down
2 changes: 1 addition & 1 deletion etc/jenkins/jenkins_build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

export DEBUG=true

mvn -V -U -B -e -Psnapshots clean install glassfish-copyright:check -Dcopyright.quiet=false
mvn -V -U -B -e -Pstaging clean install glassfish-copyright:check -Dcopyright.quiet=false
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@

import jakarta.ws.rs.client.Entity;
import jakarta.ws.rs.client.WebTarget;
import jakarta.ws.rs.core.HttpHeaders;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.UriBuilder;

Expand Down Expand Up @@ -68,7 +66,7 @@ public void testWebApplicationExceptionInRequestFilter() {
@Test
public void testWebApplicationExceptionInResponseFilter() {
WebTarget t = client().target(UriBuilder.fromUri(getBaseUri()).path(App.ROOT_PATH).path("response_exception").build());
Response r = t.request("text/plain").header(HttpHeaders.CONTENT_TYPE, MediaType.TEXT_PLAIN).get();
Response r = t.request("text/plain").get();
assertEquals(200, r.getStatus());
final String entity = r.readEntity(String.class);
System.out.println("entity = " + entity);
Expand Down
Loading

0 comments on commit fee1c32

Please sign in to comment.