Skip to content

Commit

Permalink
[RESTEASY-1763] (#1355)
Browse files Browse the repository at this point in the history
Impose spec mandated restriction on resource methods considered in
resource matching algorithm.
  • Loading branch information
ronsigal committed Dec 8, 2017
1 parent 0355f9b commit 9d97488
Show file tree
Hide file tree
Showing 10 changed files with 253 additions and 10 deletions.
Expand Up @@ -552,6 +552,20 @@ public class MyApplication extends Application
Imposes maximum size on decompressed gzipped .
</entry>
</row>
<row>
<entry>
resteasy.loose.step2.request.matching
</entry>
<entry>
false
</entry>
<entry>
In searching for a matching resource method with which to respond to a request,
consider all resource methods that match the request path. See
<link linkend='Migration_to_3_0_19_SP6'>Migrating to 3.0.19.SP6</link> for discussion.
</entry>
</row>

</tbody>
</tgroup>
</table>
Expand Down
Expand Up @@ -8,6 +8,67 @@

<chapter id="Migration_from_older_versions">
<title>Migration from older versions</title>
<sect1 id="Migration_to_3_0_19_SP6">
<title>Migrating to 3.0.19.SP6</title>

<para>
A bug discovered in the resource method matching algorithm in Resteasy versions 3.0.19.SPx prior to 3.0.19.SP6
allowed Resteasy to consider too many resource methods in responding to requests. There are three stages in
the matching algorithm:
</para>

<orderedlist>
<listitem>Use the request path to choose possible resource classes.</listitem>
<listitem>Use the request path to choose possible resource methods.</listitem>
<listitem>Use the HTTP verb and media types, coming and going, to choose a final resource method.</listitem>
</orderedlist>

<para>
In step 2, the set of potential resource methods are sorted, based on properties of @Path values like number
of literals, and only the maximal elements are passed on to step 3. However, the Resteasy implementation prior
to 3.0.19.SP6 passes ALL methods to step 3. For example,
</para>

<programlisting>
@Path("/")
public static class TestResource
{
@GET
@Path("complex/match")
public String get()
{
return "content";
}

@POST
@Path("complex/{param}")
public String post(@PathParam("param") String param)
{
return "&lt;" + param + "/&gt;";
}
}
</programlisting>

<para>
Both methods can match a request with path "complex/match", but <methodname>get()</methodname> comes
out ahead of <methodname>post()</methodname> because it has more literal characters, and
only <methodname>get()</methodname> is considered in step 3. [For more details about the sort, see
the specification for JAX-RS 2.0.]
</para>

<para>
Prior to version 3.0.19.SP6, Resteasy passes both methods to step 3. In fact, passing both methods might be considered
preferable, since, if the request verb is POST, no matching method will be found. In fact, the matching
algorithm is changed in the specification of JAX-RS 2.1 so that it passes all matching resource methods
to step 3. However, Resteasy 3.0.x adheres to JAX-RS 2.0, and the bug is fixed in release 3.0.19.SP6.
</para>

<para>
In order to make the older, if incorrect, behavior available, a context-param configuration switch,
"resteasy.loose.step2.request.matching", is introduced in version 3.0.19.SP6. It defaults to "false",
but, if it is set to "true", the older, looser behavior is enabled.
</para>
</sect1>
<sect1>
<title>Migrating from 3.0.7 to 3.0.9</title>
<itemizedlist>
Expand Down
@@ -0,0 +1,103 @@
package org.jboss.resteasy.test.nextgen.finegrain.resource;


import java.util.HashSet;
import java.util.Set;

import javax.ws.rs.ApplicationPath;
import javax.ws.rs.GET;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.client.Entity;
import javax.ws.rs.client.Invocation;
import javax.ws.rs.core.Application;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;

import org.jboss.resteasy.client.jaxrs.ResteasyClient;
import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder;
import org.jboss.resteasy.core.Dispatcher;
import org.jboss.resteasy.core.ResourceMethodRegistry;
import org.jboss.resteasy.spi.ResteasyDeployment;
import org.jboss.resteasy.test.EmbeddedContainer;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;


/**
*
* @author <a href="ron.sigal@jboss.com">Ron Sigal</a>
* @version $Revision: 1.1 $
*
* Created December 6, 2017
*/
public class Step2ResourceMatchingTest
{
protected static ResteasyDeployment deployment;
protected static Dispatcher dispatcher;

@ApplicationPath("")
public static class TestApp extends Application
{
@Override
public Set<Class<?>> getClasses()
{
HashSet<Class<?>> classes = new HashSet<Class<?>>();
classes.add(TestResource.class);
return classes;
}
}

@Path("")
public static class TestResource
{
@GET
@Path("/test/abc")
public Response m1()
{
return Response.ok("m1()").build();
}

@POST
@Path("/test/{path}")
public Response m2(@PathParam("path") String path, String s) {
return Response.ok("m2()").build();
}
}

//////////////////////////////////////////////////////////////////////////////
@BeforeClass
public static void before() throws Exception
{
deployment = EmbeddedContainer.start();
((ResourceMethodRegistry)deployment.getRegistry()).setWiderMatching(true);
dispatcher = deployment.getDispatcher();
deployment.getRegistry().addPerRequestResource(TestResource.class);
}

@AfterClass
public static void after() throws Exception
{
EmbeddedContainer.stop();
dispatcher = null;
deployment = null;
Thread.sleep(100);
}

//////////////////////////////////////////////////////////////////////////////

@Test
public void testResourceMethodChoice() throws Exception
{
ResteasyClient client = new ResteasyClientBuilder().build();
Invocation.Builder request = client.target("http://localhost:8081/test/abc").request();
Response response = request.post(Entity.entity("xyz", MediaType.TEXT_PLAIN));
String s = response.readEntity(String.class);
Assert.assertEquals(200, response.getStatus());
Assert.assertEquals("m2()", s);
client.close();
}
}
Expand Up @@ -29,10 +29,15 @@ public int compareTo(Expression expression)
if (s != 0) return s;

MethodExpression me = (MethodExpression)expression;
if (invoker.getClass().equals(me.invoker.getClass())) return 0;

if (invoker instanceof ResourceMethodInvoker) return -1;
else return 1;
if (this.invoker instanceof ResourceMethodInvoker && me.invoker instanceof ResourceLocatorInvoker)
{
return -1;
}
if (this.invoker instanceof ResourceLocatorInvoker && me.invoker instanceof ResourceMethodInvoker)
{
return 1;
}
return 0;
}

public MethodExpression(SegmentNode parent, String segment, ResourceInvoker invoker)
Expand Down
Expand Up @@ -3,10 +3,14 @@
import org.jboss.resteasy.core.ResourceInvoker;
import org.jboss.resteasy.core.ResourceLocatorInvoker;
import org.jboss.resteasy.core.ResourceMethodInvoker;
import org.jboss.resteasy.core.ResourceMethodRegistry;
import org.jboss.resteasy.resteasy_jaxrs.i18n.LogMessages;
import org.jboss.resteasy.resteasy_jaxrs.i18n.Messages;
import org.jboss.resteasy.spi.DefaultOptionsMethodException;
import org.jboss.resteasy.spi.HttpRequest;
import org.jboss.resteasy.spi.Registry;
import org.jboss.resteasy.spi.ResteasyDeployment;
import org.jboss.resteasy.spi.ResteasyProviderFactory;
import org.jboss.resteasy.spi.ResteasyUriInfo;
import org.jboss.resteasy.util.HttpHeaderNames;
import org.jboss.resteasy.util.HttpResponseCodes;
Expand Down Expand Up @@ -52,6 +56,9 @@ public class SegmentNode
protected String segment;
protected Map<String, SegmentNode> children = new HashMap<String, SegmentNode>();
protected List<MethodExpression> targets = new ArrayList<MethodExpression>();
protected boolean requestMatchingSet = false;
protected boolean looseStep2RequestMatching = false;
protected boolean widerRequestMatching = false;

public SegmentNode(String segment)
{
Expand All @@ -78,20 +85,35 @@ public ResourceInvoker match(HttpRequest request, int start)
potentials(path, start, potentials);
Collections.sort(potentials);

boolean expressionMatched = false;
if (!requestMatchingSet)
{
Map<Class<?>, Object> contextDataMap = ResteasyProviderFactory.getContextDataMap();
ResteasyDeployment deployment = (ResteasyDeployment) contextDataMap.get(ResteasyDeployment.class);
if (deployment != null)
{
looseStep2RequestMatching = deployment.isLooseStep2RequestMatching();
widerRequestMatching = deployment.isWiderRequestMatching();
requestMatchingSet = true;
}
ResourceMethodRegistry registry = (ResourceMethodRegistry) contextDataMap.get(Registry.class);
if (deployment != null)
{
widerRequestMatching = registry.isWiderMatching();
}
}
MethodExpression matchedExpression = null;
List<Match> matches = new ArrayList<Match>();
for (MethodExpression expression : potentials)
{
// We ignore locators if the first match was a resource method as per the spec Section 3, Step 2(h)
if (expressionMatched && expression.isLocator()) continue;
if (matchedExpression != null && expression.isLocator()) continue;

Pattern pattern = expression.getPattern();
Matcher matcher = pattern.matcher(path);
matcher.region(start, path.length());

if (matcher.matches())
{
expressionMatched = true;
ResourceInvoker invoker = expression.getInvoker();
if (invoker instanceof ResourceLocatorInvoker)
{
Expand Down Expand Up @@ -128,7 +150,23 @@ public ResourceInvoker match(HttpRequest request, int start)
}
else
{
matches.add(new Match(expression, matcher));
if (looseStep2RequestMatching || widerRequestMatching)
{
matches.add(new Match(expression, matcher));
}
else if (matchedExpression == null)
{
matchedExpression = expression;
matches.add(new Match(expression, matcher));
}
else if (matchedExpression.compareTo(expression) == 0)
{
matches.add(new Match(expression, matcher));
}
else
{
break;
}
}
}
}
Expand Down
Expand Up @@ -239,7 +239,13 @@ public ResteasyDeployment createDeployment()
deployment.setWiderRequestMatching(wider);
}


String looseStep2Matching = getParameter(ResteasyContextParameters.RESTEASY_LOOSE_STEP2_REQUEST_MATCHING);
if (looseStep2Matching != null)
{
boolean looseStep2 = parseBooleanParam(ResteasyContextParameters.RESTEASY_LOOSE_STEP2_REQUEST_MATCHING, looseStep2Matching);
deployment.setLooseStep2RequestMatching(looseStep2);
}


String injectorFactoryClass = getParameter("resteasy.injector.factory");
if (injectorFactoryClass != null)
Expand Down
Expand Up @@ -33,6 +33,7 @@ public ResteasyDeployment createDeployment()
{
ResteasyDeployment deployment = (ResteasyDeployment) servletContext.getAttribute(ResteasyDeployment.class.getName());
if (deployment == null) deployment = super.createDeployment();
deployment.getDefaultContextObjects().put(ResteasyDeployment.class, deployment);
deployment.getDefaultContextObjects().put(ServletContext.class, servletContext);
deployment.getDefaultContextObjects().put(ResteasyConfiguration.class, this);
String servletMappingPrefix = getParameter(ResteasyContextParameters.RESTEASY_SERVLET_MAPPING_PREFIX);
Expand Down
Expand Up @@ -35,6 +35,7 @@ public interface ResteasyContextParameters
String RESTEASY_SECURE_PROCESSING_FEATURE = "resteasy.document.secure.processing.feature";
String RESTEASY_DISABLE_DTDS = "resteasy.document.secure.disableDTDs";
String RESTEASY_GZIP_MAX_INPUT = "resteasy.gzip.max.input";
String RESTEASY_LOOSE_STEP2_REQUEST_MATCHING = "resteasy.loose.step2.request.matching";

// these scanned variables are provided by a deployer
String RESTEASY_SCANNED_RESOURCES = "resteasy.scanned.resources";
Expand Down
Expand Up @@ -39,6 +39,7 @@
public class ResteasyDeployment
{
protected boolean widerRequestMatching;
protected boolean looseStep2RequestMatching = false;
protected boolean useContainerFormParams = false;
protected boolean deploymentSensitiveFactoryEnabled = false;
protected boolean asyncJobServiceEnabled = false;
Expand Down Expand Up @@ -935,4 +936,14 @@ public void setWiderRequestMatching(boolean widerRequestMatching)
{
this.widerRequestMatching = widerRequestMatching;
}

public boolean isLooseStep2RequestMatching()
{
return looseStep2RequestMatching;
}

public void setLooseStep2RequestMatching(boolean looseStep2RequestMatching)
{
this.looseStep2RequestMatching = looseStep2RequestMatching;
}
}
Expand Up @@ -9,6 +9,7 @@
import org.jboss.resteasy.client.ClientResponse;
import org.jboss.resteasy.core.Dispatcher;
import org.jboss.resteasy.plugins.delegates.MediaTypeHeaderDelegate;
import org.jboss.resteasy.spi.ResteasyDeployment;
import org.jboss.resteasy.test.EmbeddedContainer;
import org.jboss.resteasy.util.HttpHeaderNames;
import org.jboss.resteasy.util.HttpResponseCodes;
Expand Down Expand Up @@ -84,7 +85,9 @@ public void noreturn(String entity)
@BeforeClass
public static void before() throws Exception
{
dispatcher = EmbeddedContainer.start().getDispatcher();
ResteasyDeployment deployment = EmbeddedContainer.start();
deployment.setLooseStep2RequestMatching(true);
dispatcher = deployment.getDispatcher();
dispatcher.getRegistry().addPerRequestResource(WebResourceUnsupportedMediaType.class);
}

Expand Down

0 comments on commit 9d97488

Please sign in to comment.