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

[Netty4] Correct handling for absolute URIs in HTTP request lines #639

Merged
merged 1 commit into from Sep 7, 2015

Conversation

avram
Copy link
Contributor

@avram avram commented Apr 7, 2015

Correctly handle requests with an absolute URI in the request line, like:

GET http://www.example.com/testContent HTTP/1.1
Host: www.example.com

This is required per the spec: http://tools.ietf.org/html/rfc2616#section-5, specifically:

To allow for transition to absoluteURIs in all requests in future versions of HTTP, all HTTP/1.1 servers MUST accept the absoluteURI form in requests, even though HTTP/1.1 clients will only generate them in requests to proxies.

And:

If Request-URI is an absoluteURI, the host is part of the Request-URI. Any Host header field value in the request MUST be ignored.

Issue: https://issues.jboss.org/browse/RESTEASY-1193

public void testAbsoluteURI() throws Exception {
String uri = generateURL("/test");

Socket client = new Socket(getHost(), getPort());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This tests uses sockets directly since the Resteasy client builders don't support making requests with absolute URIs in the request line.

@avram
Copy link
Contributor Author

avram commented Apr 17, 2015

@patriot1burke Can you take a look at this change? It looked to me like a divergence from the HTTP spec, and reproducible easily with RxNetty.

@ronsigal
Copy link
Collaborator

ronsigal commented Jul 5, 2015

Hi Avram,

I have a suggestion for a test:

  @GET
  @Path("/test/absolute")
  @Produces("text/plain")
  public String absolute(@Context UriInfo info)
  {
      return "uri: " + info.getRequestUri().toString();
  }

and

@Test
public void testAbsoluteURI() throws Exception {
   String uri = generateURL("/test/absolute");

   Socket client = new Socket(getHost(), getPort());
   PrintWriter out = new PrintWriter(client.getOutputStream(), true);
   BufferedReader in = new BufferedReader(new InputStreamReader(client.getInputStream()));
   out.printf(Locale.US, "GET %s HTTP/1.1\nHost: %s:%d\n\n", uri, "127.0.0.1", getPort());
   String statusLine = in.readLine();
   String response = in.readLine();
   while (!response.startsWith("uri")) 
   {
       response = in.readLine();
   }
   client.close();
   Assert.assertEquals("HTTP/1.1 200 OK", statusLine);
   Assert.assertEquals(uri, response.subSequence(5, response.length()));

}

This test proves that NettyUtil is using the given absolute URI in the GET line rather than constructing a URI.

@ronsigal
Copy link
Collaborator

ronsigal commented Jul 5, 2015

Other than that, the patch looks good. Avram, could you create a JIRA issue here: https://jira.jboss.org/browse/RESTEASY ?

@avram
Copy link
Contributor Author

avram commented Jul 6, 2015

Fixed and rebased vs master. Issue incoming.

@avram
Copy link
Contributor Author

avram commented Jul 6, 2015

@ronsigal
Copy link
Collaborator

ronsigal commented Jul 8, 2015

Thanks, Avram. Looks good to merge.

liweinan added a commit that referenced this pull request Sep 7, 2015
[Netty4] Correct handling for absolute URIs in HTTP request lines
@liweinan liweinan merged commit b11d11f into resteasy:master Sep 7, 2015
@avram avram deleted the fix-request-line branch September 8, 2015 04:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants