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

Missing Content-Type header should be application/octet-stream optionally #4911

Merged
merged 4 commits into from
Nov 22, 2021
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
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
Copy link
Contributor

Choose a reason for hiding this comment

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

I would advise keeping of both profiles here -Pstaging,snapshots

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