From d5c95a1302230ba8e20d1fd56dc99b7a3302a256 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 29 Apr 2015 14:01:20 +1000 Subject: [PATCH] 465747 - Jetty is failing to process all HTTP OPTIONS requests. The Server handleOptions method was handling all OPTIONS * requests with a blank 200 response. This has been fixed so that this method only checks that * URI is only applied to OPTIONS method. --- .../org/eclipse/jetty/http/HttpMethod.java | 2 +- .../java/org/eclipse/jetty/server/Server.java | 10 ++--- .../jetty/server/HttpServerTestBase.java | 39 +++++++++++++++++++ .../jetty/server/HttpServerTestFixture.java | 15 +++++++ .../test/support/TestableJettyServer.java | 2 +- .../src/test/resources/BIOHttp.xml | 22 ----------- .../src/test/resources/BIOHttps.xml | 29 -------------- .../src/test/resources/NIOHttp.xml | 2 +- .../src/test/resources/NIOHttps.xml | 2 +- 9 files changed, 61 insertions(+), 62 deletions(-) delete mode 100644 tests/test-integration/src/test/resources/BIOHttp.xml delete mode 100644 tests/test-integration/src/test/resources/BIOHttps.xml diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java index 0dd075328f2..7904932637b 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java @@ -76,7 +76,7 @@ public enum HttpMethod return HEAD; break; case 'O': - if (bytes[position+1]=='O' && bytes[position+2]=='T' && bytes[position+3]=='I' && length>=8 && + if (bytes[position+1]=='P' && bytes[position+2]=='T' && bytes[position+3]=='I' && length>=8 && bytes[position+4]=='O' && bytes[position+5]=='N' && bytes[position+6]=='S' && bytes[position+7]==' ' ) return OPTIONS; break; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index fe6f847410f..96d10b64b6d 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -487,8 +487,10 @@ public class Server extends HandlerWrapper implements Attributes if (LOG.isDebugEnabled()) LOG.debug(request.getDispatcherType()+" "+request.getMethod()+" "+target+" on "+connection); - if ("*".equals(target)) + if (HttpMethod.OPTIONS.is(request.getMethod()) || "*".equals(target)) { + if (!HttpMethod.OPTIONS.is(request.getMethod())) + response.sendError(HttpStatus.BAD_REQUEST_400); handleOptions(request,response); if (!request.isHandled()) handle(target, request, request, response); @@ -505,12 +507,6 @@ public class Server extends HandlerWrapper implements Attributes */ protected void handleOptions(Request request,Response response) throws IOException { - if (!HttpMethod.OPTIONS.is(request.getMethod())) - response.sendError(HttpStatus.BAD_REQUEST_400); - request.setHandled(true); - response.setStatus(200); - response.setContentLength(0); - response.closeOutput(); } /* ------------------------------------------------------------ */ diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java index 5875c526a85..9d0dad20be8 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java @@ -131,6 +131,45 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture } } + @Test + public void testOPTIONS() throws Exception + { + configureServer(new OptionsHandler()); + + try (Socket client = newSocket(_serverURI.getHost(), _serverURI.getPort())) + { + OutputStream os = client.getOutputStream(); + + os.write(("OPTIONS * HTTP/1.1\r\n" + + "Host: "+_serverURI.getHost()+"\r\n" + + "Connection: close\r\n" + + "\r\n").getBytes(StandardCharsets.ISO_8859_1)); + os.flush(); + + // Read the response. + String response = readResponse(client); + + Assert.assertThat(response, Matchers.containsString("HTTP/1.1 200 OK")); + Assert.assertThat(response, Matchers.containsString("Allow: GET")); + } + + try (Socket client = newSocket(_serverURI.getHost(), _serverURI.getPort())) + { + OutputStream os = client.getOutputStream(); + + os.write(("GET * HTTP/1.1\r\n" + + "Host: "+_serverURI.getHost()+"\r\n" + + "Connection: close\r\n" + + "\r\n").getBytes(StandardCharsets.ISO_8859_1)); + os.flush(); + + // Read the response. + String response = readResponse(client); + + Assert.assertThat(response, Matchers.containsString("HTTP/1.1 400 ")); + Assert.assertThat(response, Matchers.not(Matchers.containsString("Allow: "))); + } + } /* * Feed a full header method diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestFixture.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestFixture.java index 82fff8f4a9c..c06034279c6 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestFixture.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestFixture.java @@ -150,6 +150,21 @@ public class HttpServerTestFixture } } + protected static class OptionsHandler extends AbstractHandler + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + baseRequest.setHandled(true); + if (request.getMethod().equals("OPTIONS")) + response.setStatus(200); + else + response.setStatus(500); + + response.setHeader("Allow", "GET"); + } + } + protected static class HelloWorldHandler extends AbstractHandler { @Override diff --git a/tests/test-integration/src/test/java/org/eclipse/jetty/test/support/TestableJettyServer.java b/tests/test-integration/src/test/java/org/eclipse/jetty/test/support/TestableJettyServer.java index 5c589d1dd6a..43876e8f4bd 100644 --- a/tests/test-integration/src/test/java/org/eclipse/jetty/test/support/TestableJettyServer.java +++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/support/TestableJettyServer.java @@ -177,7 +177,7 @@ public class TestableJettyServer _server.start(); // Find the active server port. - this._serverPort = ((NetworkConnector)_server.getConnectors()[0]).getPort(); + this._serverPort = ((NetworkConnector)_server.getConnectors()[0]).getLocalPort(); System.err.println("Server Port="+_serverPort); Assert.assertTrue("Server Port is between 1 and 65535. Actually <" + _serverPort + ">",(1 <= this._serverPort) && (this._serverPort <= 65535)); } diff --git a/tests/test-integration/src/test/resources/BIOHttp.xml b/tests/test-integration/src/test/resources/BIOHttp.xml deleted file mode 100644 index 751a3c2cd0a..00000000000 --- a/tests/test-integration/src/test/resources/BIOHttp.xml +++ /dev/null @@ -1,22 +0,0 @@ - - - - - - - - - - - - - - - 300000 - 2 - false - - - - - diff --git a/tests/test-integration/src/test/resources/BIOHttps.xml b/tests/test-integration/src/test/resources/BIOHttps.xml deleted file mode 100644 index d845957d142..00000000000 --- a/tests/test-integration/src/test/resources/BIOHttps.xml +++ /dev/null @@ -1,29 +0,0 @@ - - - - - - - - - - - - - - - 300000 - 2 - false - /keystore - OBF:1vny1zlo1x8e1vnw1vn61x8g1zlu1vn4 - OBF:1u2u1wml1z7s1z7a1wnl1u2g - - - - - - diff --git a/tests/test-integration/src/test/resources/NIOHttp.xml b/tests/test-integration/src/test/resources/NIOHttp.xml index 793da7a94d1..c766d105966 100644 --- a/tests/test-integration/src/test/resources/NIOHttp.xml +++ b/tests/test-integration/src/test/resources/NIOHttp.xml @@ -21,7 +21,7 @@ - + 0 diff --git a/tests/test-integration/src/test/resources/NIOHttps.xml b/tests/test-integration/src/test/resources/NIOHttps.xml index ff42c725fa6..5e90fdbd0d6 100644 --- a/tests/test-integration/src/test/resources/NIOHttps.xml +++ b/tests/test-integration/src/test/resources/NIOHttps.xml @@ -26,7 +26,7 @@ - + 0 30000