diff --git a/PR-STATUS.md b/PR-STATUS.md index 576e46b24b..218aa41741 100644 --- a/PR-STATUS.md +++ b/PR-STATUS.md @@ -66,6 +66,15 @@ This is the current status for each PR: * :green_circle: **MERGED**: HTTP 2 Support Feature request (Major 3.0 candidate) * perwendel/spark#1183 opened on Jul 21, 2020 by luneo7 +### Merged (Release 4) +* :green_circle: **MERGED**: Fix perwendel/spark/issues/1077 : Solve the bad route selection based on acceptType + * perwendel/spark#1238 opened on May 21, 2021 by Chauncey-Xxy +* :green_circle: **MERGED AND FIXED**: fix issue perwendel/spark/issues/1204 + * perwendel/spark#1236 opened on May 21, 2021 by FhToday +* :green_circle: **CHERRY PICKED**: Solve the problem of non-ASCII characters in URL. Try to fix #1026 + * perwendel/spark#1222 opened on Apr 23, 2021 by Bugjudger + + ### Rejected * :red_circle: **REJECTED (Travis was removed)**: Improve Travis CI build Performance @@ -83,12 +92,6 @@ This is the current status for each PR: ### To Fix / To Discuss -* :yellow_circle: **TEST FAILING**: Fix perwendel/spark/issues/1077 : Solve the bad route selection based on acceptType - * perwendel/spark#1238 opened on May 21, 2021 by Chauncey-Xxy -* :yellow_circle: **TEST FAILING**: fix issue perwendel/spark/issues/1204 - * perwendel/spark#1236 opened on May 21, 2021 by FhToday -* :yellow_circle: **TEST FAILING**: Solve the problem of non-ASCII characters in URL. Try to fix #1026 - * perwendel/spark#1222 opened on Apr 23, 2021 by Bugjudger * :yellow_circle: **MAY BREAK CODE (package name will change)**: Provide Automatic-Module-Name attribute in MANIFEST.MF (issue perwendel/spark/issues/961) * perwendel/spark#1212 opened on Mar 10, 2021 by apssouza22 * :yellow_circle: **MERGE FAILS (#1030 was closed as not-wanted)**: Try to Fix issue perwendel/spark/issues/1022 & perwendel/spark/issues/1030 bug-fix diff --git a/README.md b/README.md index 350aaca40f..7b9d7eda8a 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ In order to use this fork version, you need to change your spark dependency. com.intellisrc spark-core - 2.9.4-unofficial-3 + 2.9.4-unofficial-4 ``` @@ -32,7 +32,7 @@ In order to use this fork version, you need to change your spark dependency. ```groovy dependencies { - implementation 'com.intellisrc:spark-core:2.9.4-unofficial-3' + implementation 'com.intellisrc:spark-core:2.9.4-unofficial-4' } ``` @@ -56,6 +56,8 @@ Security: ## Release 2 +These are the patches included in `unofficial-2`: + Bug fixes: * Initiate the servlet instance in exception mapper (perwendel#1137) @@ -68,6 +70,8 @@ Improvements: ## Release 3 +These are the patches included in `unofficial-3`: + Bug fixes: * Fixed GZip content-length problem - Issue: perwendel#1157 (also perwendel#459, perwendel#742 and perwendel#937) @@ -78,7 +82,19 @@ Improvements: * Regex support in paths * HTTP/2 support (perwendel#1183) -More details on these features: [DIFFERENCES.md](DIFFERENCES.md) +## Release4 + +These are the patches included in `unofficial-4`: + +Bug fixes: +* Fixed optional trailing slash when used with params +* Fixed incorrect response based on acceptType (perwendel#1077) (PR: perwendel/spark#1238) +* Fixed incorrect handling when using invalid methods (perwendel/spark#1236) (PR: perwendel#1204) + +Improvements: +* Added unicode support in paths (issue perwendel#1026) (PR: perwendel/spark#1222) + +More details and examples on the differences between the Official version and this one: [DIFFERENCES.md](DIFFERENCES.md) ---------------------------------------- diff --git a/pom.xml b/pom.xml index 90327fb70d..34326f8656 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ com.intellisrc spark-core bundle - 2.9.4-unofficial-3-SNAPSHOT + 2.9.4-unofficial-4-SNAPSHOT Spark A micro framework for creating web applications in Kotlin, Groovy and Java with minimal effort http://www.sparkjava.com diff --git a/src/main/java/spark/embeddedserver/jetty/JettyHandler.java b/src/main/java/spark/embeddedserver/jetty/JettyHandler.java index 16b90729ee..bed476ae09 100644 --- a/src/main/java/spark/embeddedserver/jetty/JettyHandler.java +++ b/src/main/java/spark/embeddedserver/jetty/JettyHandler.java @@ -24,6 +24,8 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.HttpMethod; +import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.session.SessionHandler; @@ -44,18 +46,24 @@ public JettyHandler(Filter filter) { @Override public void doHandle( - String target, - Request baseRequest, - HttpServletRequest request, - HttpServletResponse response) throws IOException, ServletException { + String target, + Request baseRequest, + HttpServletRequest request, + HttpServletResponse response) throws IOException, ServletException { HttpRequestWrapper wrapper = new HttpRequestWrapper(request); + HttpMethod method = HttpMethod.fromString(request.getMethod().trim().toUpperCase()); + if(method == null) { + response.sendError(HttpStatus.METHOD_NOT_ALLOWED_405); + return; + } + if(consume!=null && consume.contains(baseRequest.getRequestURI())){ wrapper.notConsumed(true); - } - else { + } else { filter.doFilter(wrapper, response, null); } + baseRequest.setHandled(!wrapper.notConsumed()); } diff --git a/src/main/java/spark/http/matching/MatcherFilter.java b/src/main/java/spark/http/matching/MatcherFilter.java index 66ce8a0f53..529ba37ec4 100644 --- a/src/main/java/spark/http/matching/MatcherFilter.java +++ b/src/main/java/spark/http/matching/MatcherFilter.java @@ -17,6 +17,8 @@ package spark.http.matching; import java.io.IOException; +import java.util.List; +import java.net.URLDecoder; import javax.servlet.Filter; import javax.servlet.FilterChain; @@ -33,6 +35,7 @@ import spark.RequestResponseFactory; import spark.Response; import spark.route.HttpMethod; +import spark.routematch.RouteMatch; import spark.serialization.SerializerChain; import spark.staticfiles.StaticFilesConfiguration; @@ -104,8 +107,21 @@ public void doFilter(ServletRequest servletRequest, String httpMethodStr = method.toLowerCase(); String uri = httpRequest.getRequestURI(); + uri = URLDecoder.decode(uri, "UTF-8"); String acceptType = httpRequest.getHeader(ACCEPT_TYPE_REQUEST_MIME_HEADER); + List routes = routeMatcher.findAll(); + String firstAcceptType = null; + for (RouteMatch rm : routes) { + if (rm.getMatchUri().equals(uri)) { + firstAcceptType = rm.getAcceptType(); + break; + } + } + if ("*/*".equals(acceptType) && firstAcceptType != null) { + acceptType = firstAcceptType; + } + Body body = Body.create(); RequestWrapper requestWrapper = RequestWrapper.create(); diff --git a/src/test/java/spark/GenericIntegrationTest.java b/src/test/java/spark/GenericIntegrationTest.java index da1a7012cb..2b686190f6 100644 --- a/src/test/java/spark/GenericIntegrationTest.java +++ b/src/test/java/spark/GenericIntegrationTest.java @@ -18,7 +18,6 @@ import org.eclipse.jetty.websocket.client.WebSocketClient; import org.junit.AfterClass; import org.junit.Assert; -import org.junit.Ignore; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -328,24 +327,14 @@ public void testEchoParam3() throws Exception { Assert.assertEquals("echo: " + polyglot, response.body); } + // NOTE: To support multiple languages in paths, we are using URLDecode, which in this case, + // it will convert '+' into ' ' @Test public void testPathParamsWithPlusSign() throws Exception { String pathParamWithPlusSign = "not+broken+path+param"; UrlResponse response = doMethod("GET", "/param/" + pathParamWithPlusSign, null); Assert.assertEquals(200, response.status); - Assert.assertEquals("echo: " + pathParamWithPlusSign, response.body); - } - - // FIXME: http2 is failing due to: https://github.com/eclipse/jetty.project/issues/6132 - // also: https://www.eclipse.org/jetty/javadoc/jetty-10/org/eclipse/jetty/http/UriCompliance.html - // to fix it: update to Jetty 10+ ? - @Ignore @Test - public void testParamWithEncodedSlash() throws Exception { - String polyglot = "te/st"; - String encoded = URLEncoder.encode(polyglot, "UTF-8"); - UrlResponse response = doMethod("GET", "/param/" + encoded, null); - Assert.assertEquals(200, response.status); - Assert.assertEquals("echo: " + polyglot, response.body); + Assert.assertEquals("echo: " + pathParamWithPlusSign.replace("+"," "), response.body); } @Test @@ -354,32 +343,31 @@ public void testParamWithEncodedSpace() throws Exception { String encoded = URLEncoder.encode(polyglot, "UTF-8"); UrlResponse response = doMethod("GET", "/param/" + encoded, null); Assert.assertEquals(200, response.status); - Assert.assertEquals("echo: " + polyglot.replace(" ", "+"), response.body); + Assert.assertEquals("echo: " + polyglot, response.body); } - // FIXME: http2 FIXME comment above - @Ignore @Test - public void testSplatWithEncodedSlash() throws Exception { - String param = "fo/shizzle"; + @Test + public void testSplatWithEncodedSpace() throws Exception { + String param = "fo shizzle"; String encodedParam = URLEncoder.encode(param, "UTF-8"); - String splat = "mah/FRIEND"; + String splat = "mah FRIEND"; String encodedSplat = URLEncoder.encode(splat, "UTF-8"); UrlResponse response = doMethod("GET", "/paramandwild/" + encodedParam + "/stuff/" + encodedSplat, null); Assert.assertEquals(200, response.status); - Assert.assertEquals("paramandwild: " + param.replace(" ", "+") + splat, response.body); + Assert.assertEquals("paramandwild: " + param + splat, response.body); } @Test - public void testSplatWithEncodedSpace() throws Exception { - String param = "fo shizzle"; + public void testSplatWithUTF8() throws Exception { + String param = "私のもの"; String encodedParam = URLEncoder.encode(param, "UTF-8"); - String splat = "mah FRIEND"; + String splat = "何でも大丈夫です。"; String encodedSplat = URLEncoder.encode(splat, "UTF-8"); UrlResponse response = doMethod("GET", "/paramandwild/" + encodedParam + "/stuff/" + encodedSplat, null); Assert.assertEquals(200, response.status); - Assert.assertEquals("paramandwild: " + param.replace(" ", "+") + splat, response.body); + Assert.assertEquals("paramandwild: " + param + splat, response.body); } @Test diff --git a/src/test/java/spark/InvalidRequestTest.java b/src/test/java/spark/InvalidRequestTest.java new file mode 100644 index 0000000000..cebf1fb8db --- /dev/null +++ b/src/test/java/spark/InvalidRequestTest.java @@ -0,0 +1,88 @@ +package spark; + +import org.apache.http.HttpResponse; +import org.apache.http.client.methods.HttpRequestBase; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.junit.Before; +import org.junit.Test; + +import java.net.URI; + +import spark.util.SparkTestUtil; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static spark.Spark.awaitInitialization; +import static spark.Spark.get; +import static spark.Spark.staticFileLocation; + +public class InvalidRequestTest { + + public static class HttpFoo extends HttpRequestBase { + public final static String METHOD_NAME = "FOO"; + @Override + public String getMethod() { + return METHOD_NAME; + } + public HttpFoo(final String uri) { + super(); + setURI(URI.create(uri)); + } + public String getName() { + return "FOO"; + } + } + + public static final String FILE = "/page.html"; + + public static final String SERVICE="/test"; + + public static final int PORT = 4567; + + private static final SparkTestUtil http = new SparkTestUtil(PORT); + + @Before + public void setup() { + staticFileLocation("/public"); + get(SERVICE, (request, response) -> { + assertTrue(request.requestMethod().equalsIgnoreCase("GET")); + return "Hello"; + }); + + awaitInitialization(); + } + + public int requestPathWithInvalidMethod(String path) { + int code = 0; + try { + CloseableHttpClient httpClient = HttpClients.createDefault(); + HttpFoo fooMethod = new HttpFoo("http://localhost:" + PORT + path); + HttpResponse response = httpClient.execute(fooMethod); + code = response.getStatusLine().getStatusCode(); + } catch (Exception e) { + fail("Unexpected exception: " + e.getMessage()); + } + return code; + } + + @Test + public void invalidRequestTest(){ + // Testing that file and service is up: + try { + SparkTestUtil.UrlResponse response = http.doMethod("GET", SERVICE, ""); + assertEquals(200, response.status); + assertEquals("Hello", response.body); + + response = http.doMethod("GET", FILE, ""); + assertEquals(200, response.status); + } catch (Exception e) { + e.printStackTrace(); + fail("Unexpected exception: " + e.getMessage()); + } + // Testing wrong method (we cannot use http.doMethod as it can not handle invalid methods) + assertEquals(405, requestPathWithInvalidMethod(FILE)); + assertEquals(405, requestPathWithInvalidMethod(SERVICE)); + } +} diff --git a/src/test/java/spark/Issue1026Test.java b/src/test/java/spark/Issue1026Test.java new file mode 100644 index 0000000000..48780a0032 --- /dev/null +++ b/src/test/java/spark/Issue1026Test.java @@ -0,0 +1,28 @@ +package spark; +import org.junit.Before; +import org.junit.Test; + +import spark.util.SparkTestUtil; + +import static org.junit.Assert.*; +import static spark.Spark.*; +/** + * @since 2022/08/11. + */ +public class Issue1026Test { + private static final String ROUTE = "/api/v1/管理者/"; + private static SparkTestUtil http; + + @Before + public void setup() { + http = new SparkTestUtil(4567); + get(ROUTE, (q,a)-> "Get filter matched"); + awaitInitialization(); + } + + @Test + public void testUrl() throws Exception { + SparkTestUtil.UrlResponse response = http.get(ROUTE); + assertEquals(200,response.status); + } +} diff --git a/src/test/java/spark/Issue1077Test.java b/src/test/java/spark/Issue1077Test.java new file mode 100644 index 0000000000..ec53bdbae8 --- /dev/null +++ b/src/test/java/spark/Issue1077Test.java @@ -0,0 +1,107 @@ +package spark; + +import org.junit.Before; +import org.junit.Test; + +import spark.util.SparkTestUtil; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.Assert.*; +import static spark.Spark.*; + +// Try to fix issue 1077: https://github.com/perwendel/spark/issues/1077 +// I am not sure whether it is a bug, because it is tagged as Bug ..? +// But I think it conflict with the documentation, so I try to fix it +// In short, we expect the input and output are: +// curl -i -H "Accept: application/json" http://localhost:4567/hello : Hello application json +// curl -i -H "Accept: text/html" http://localhost:4567/hello : Go Away!!! +// curl http://localhost:4567/hello : Hello application json +// The first and second are right, but now the last command will get output: Go Away!!! +// I think it is not reasonable because the empty acceptType should match every possibilities +// so with the earliest match, it should match the first possible acceptType +// TestWheteherMatchRight() checks whether it matchs right +// TestWhetherMatchFirst() checks whether it matchs first + +public class Issue1077Test { + + public static final String HELLO = "/hello"; + + public static final String TEST="/test"; + + public static final int PORT = 4567; + + private static final SparkTestUtil http = new SparkTestUtil(PORT); + + @Before + public void setup() { + + get(HELLO,"application/json", (request, response) -> "Hello application json"); + + get(HELLO,"text/json", (request, response) -> "Hello text json"); + + get(HELLO, (request, response) -> { + response.status(406); + return "Go Away!!!"; + }); + + get(TEST,"text/json", (request, response) -> "Hello text json"); + + get(TEST,"application/json", (request, response) -> "Hello application json"); + + get(TEST, (request, response) -> { + response.status(406); + return "Go Away!!!"; + }); + + awaitInitialization(); + } + + // CS304 Issue link: https://github.com/perwendel/spark/issues/1077 + @Test + public void testWheteherMatchRight() { + + try { + Map requestHeader = new HashMap<>(); + requestHeader.put("Host", "localhost:" + PORT); + requestHeader.put("User-Agent", "curl/7.55.1"); + requestHeader.put("x-forwarded-host", "proxy.mydomain.com"); + + SparkTestUtil.UrlResponse response = http.doMethod("GET",HELLO, "", false, "*/*", requestHeader); + assertEquals(200, response.status); + assertEquals("Hello application json", response.body); + + response = http.doMethod("GET",HELLO, "", false, "text/json", requestHeader); + assertEquals(200, response.status); + assertEquals("Hello text json", response.body); + + response = http.doMethod("GET",HELLO, "", false, "text/html*", requestHeader); + assertEquals(406, response.status); + assertEquals("Go Away!!!", response.body); + + } catch (Exception e) { + e.printStackTrace(); + } + } + + // CS304 Issue link: https://github.com/perwendel/spark/issues/1077 + @Test + public void testWhetherMatchFirst() { + try { + Map requestHeader = new HashMap<>(); + requestHeader.put("Host", "localhost:" + PORT); + requestHeader.put("User-Agent", "curl/7.55.1"); + requestHeader.put("x-forwarded-host", "proxy.mydomain.com"); + + SparkTestUtil.UrlResponse response = http.doMethod("GET",TEST, "", false, "*/*", requestHeader); + assertEquals(200, response.status); + assertEquals("Hello text json", response.body); + + response = http.doMethod("GET",HELLO, "", false, "*/*", requestHeader); + assertEquals(200, response.status); + assertEquals("Hello application json", response.body); + } catch (Exception e) { + e.printStackTrace(); + } + } +} diff --git a/src/test/java/spark/examples/hello/HelloWorld.java b/src/test/java/spark/examples/hello/HelloWorld.java index d3e7ef8985..66fb7bb259 100644 --- a/src/test/java/spark/examples/hello/HelloWorld.java +++ b/src/test/java/spark/examples/hello/HelloWorld.java @@ -20,14 +20,13 @@ /** * Minimal example - * * You can test from command with: * > curl -i 'http://localhost:4567/' */ +@SuppressWarnings("JavadocLinkAsPlainText") public class HelloWorld { public static void main(String[] args) { - get("/", (request, response) -> "Hello World!"); }