From 640823ed5b3e9960254551c360372ad41bbb84cd Mon Sep 17 00:00:00 2001 From: Maksimenko Vladimir Date: Sat, 26 Dec 2015 14:20:42 +0300 Subject: [PATCH 01/11] Refactoring for `RqHref.java:83-86: RqMethod already validates Request-Line and... #473` --- src/main/java/org/takes/rq/RqHref.java | 33 +-- src/main/java/org/takes/rq/RqMethod.java | 19 +- src/main/java/org/takes/rq/RqRequestLine.java | 161 +++++++++++ .../java/org/takes/rq/RqRequestLineTest.java | 256 ++++++++++++++++++ 4 files changed, 421 insertions(+), 48 deletions(-) create mode 100644 src/main/java/org/takes/rq/RqRequestLine.java create mode 100644 src/test/java/org/takes/rq/RqRequestLineTest.java diff --git a/src/main/java/org/takes/rq/RqHref.java b/src/main/java/org/takes/rq/RqHref.java index f52b61321..4f2895fd9 100644 --- a/src/main/java/org/takes/rq/RqHref.java +++ b/src/main/java/org/takes/rq/RqHref.java @@ -27,8 +27,6 @@ import java.io.InputStream; import java.net.HttpURLConnection; import java.util.Iterator; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import lombok.EqualsAndHashCode; import org.takes.HttpException; import org.takes.Request; @@ -63,15 +61,6 @@ public interface RqHref extends Request { @EqualsAndHashCode(callSuper = true) final class Base extends RqWrap implements RqHref { - /** - * HTTP Request-Line pattern. - * @see RFC 2616 - */ - private static final Pattern REQUEST_PATTERN = Pattern.compile( - "([!-~]+) ([^ ]+)( [^ ]+)?" - ); - /** * Ctor. * @param req Original request @@ -80,28 +69,10 @@ public Base(final Request req) { super(req); } - // @todo #445:30min/DEV RqMethod already validates Request-Line and - // extracts HTTP Method from it. We should extract all important - // information from Request-Line (HTTP method, URI and HTTP version) - // in one place to enforce DRY principle. @Override public Href href() throws IOException { - if (!this.head().iterator().hasNext()) { - throw new HttpException( - HttpURLConnection.HTTP_BAD_REQUEST, - "HTTP Request should have Request-Line" - ); - } - final String line = this.head().iterator().next(); - final Matcher matcher = - REQUEST_PATTERN.matcher(line); - if (!matcher.matches()) { - throw new HttpException( - HttpURLConnection.HTTP_BAD_REQUEST, - String.format("Illegal Request-Line: %s", line) - ); - } - final String uri = matcher.group(2); + final String uri = new RqRequestLine.Base(this) + .requestLineHeaderToken(RqRequestLine.URI); return new Href( String.format( "http://%s%s", diff --git a/src/main/java/org/takes/rq/RqMethod.java b/src/main/java/org/takes/rq/RqMethod.java index 7818c4db4..96201fbbc 100644 --- a/src/main/java/org/takes/rq/RqMethod.java +++ b/src/main/java/org/takes/rq/RqMethod.java @@ -25,7 +25,6 @@ import java.io.IOException; import java.util.Locale; -import java.util.regex.Matcher; import java.util.regex.Pattern; import lombok.EqualsAndHashCode; import org.takes.Request; @@ -112,14 +111,6 @@ final class Base extends RqWrap implements RqMethod { "[()<>@,;:\\\"/\\[\\]?={}]" ); - /** - * HTTP method line pattern. - * [!-~] is for method or extension-method token (octets 33 - 126). - */ - private static final Pattern PATTERN = Pattern.compile( - "([!-~]+) [^ ]+( [^ ]+){0,1}" - ); - /** * Ctor. * @param req Original request @@ -130,14 +121,8 @@ public Base(final Request req) { @Override public String method() throws IOException { - final String line = this.head().iterator().next(); - final Matcher matcher = PATTERN.matcher(line); - if (!matcher.matches()) { - throw new IOException( - String.format("Invalid HTTP method line: %s", line) - ); - } - final String method = matcher.group(1); + final String method = new RqRequestLine.Base(this) + .requestLineHeaderToken(RqRequestLine.METHOD); if (SEPARATORS.matcher(method).find()) { throw new IOException( String.format("Invalid HTTP method: %s", method) diff --git a/src/main/java/org/takes/rq/RqRequestLine.java b/src/main/java/org/takes/rq/RqRequestLine.java new file mode 100644 index 000000000..ab9fe53de --- /dev/null +++ b/src/main/java/org/takes/rq/RqRequestLine.java @@ -0,0 +1,161 @@ +/** + * The MIT License (MIT) + * + * Copyright (c) 2015 Yegor Bugayenko + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package org.takes.rq; + +import java.io.IOException; +import java.net.HttpURLConnection; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import lombok.EqualsAndHashCode; +import org.takes.HttpException; +import org.takes.Request; + +/** + * HTTP Request-Line parsing. + * + *

All implementations of this interface must be immutable and thread-safe. + * + * @author Vladimir Maksimenko (xupypr@xupypr.com) + * @version $Id$ + * @since 1.0 + */ +@EqualsAndHashCode(callSuper = true) +public interface RqRequestLine extends Request { + + /** + * METHOD token. + */ + int METHOD = 1; + + /** + * URI token. + */ + int URI = 2; + + /** + * HTTPVERSION token. + */ + int HTTPVERSION = 3; + + /** + * Get Request-Line header. + * @return HTTP Request-Line header + * @throws IOException If fails + */ + String requestLineHeader() throws IOException; + + /** + * Get Request-Line header token by index number. + * @param index Token index number + * @return HTTP Request-Line header token specified by index number + * @throws IOException If fails + */ + String requestLineHeaderToken(int index) throws IOException; + + /** + * Request decorator for Request-Line header validation + * + *

The class is immutable and thread-safe. + * @author Vladimir Maksimenko (xupypr@xupypr.com) + * @version $Id$ + * @since 1.0 + */ + @EqualsAndHashCode(callSuper = true) + final class Base extends RqWrap implements RqRequestLine { + /** + * HTTP Request-line pattern. + * [!-~] is for method or extension-method token (octets 33 - 126). + * @see RFC 2616 + */ + static final Pattern REQUEST_LINE_PATTERN = Pattern.compile( + "([!-~]+) ([^ ]+)( [^ ]+)?" + ); + + /** + * Ctor. + * @param req Original request + */ + public Base(final Request req) { + super(req); + } + + @Override + public String requestLineHeader() throws IOException { + return this.extractFromRequestLineHeader(null); + } + + @Override + public String requestLineHeaderToken(final int index) + throws IOException { + // @checkstyle MagicNumberCheck (1 lines) + if (index < 1 || index > 3) { + throw new IllegalArgumentException( + String.format("Illegel indexNumber %d", index) + ); + } + return this.extractFromRequestLineHeader(index); + } + + /** + * Extract Request-Line or Request-Line token from Request. + * + * @param index Token index number + * @return Request-Line if index is null, token as string otherwise + * @throws IOException If fails + */ + private String extractFromRequestLineHeader(final Integer index) + throws IOException { + if (!this.head().iterator().hasNext()) { + throw new HttpException( + HttpURLConnection.HTTP_BAD_REQUEST, + "HTTP Request should have Request-Line" + ); + } + final String line = this.head().iterator().next(); + final Matcher matcher = REQUEST_LINE_PATTERN.matcher(line); + if (!matcher.matches()) { + throw new HttpException( + HttpURLConnection.HTTP_BAD_REQUEST, + String.format( + "Invalid HTTP Request-Line header: %s", + line + ) + ); + } + if (index != null) { + final String token = matcher.group(index); + final String result; + if (token != null) { + result = token.trim(); + } else { + result = token; + } + return result; + } else { + return line; + } + } + } +} diff --git a/src/test/java/org/takes/rq/RqRequestLineTest.java b/src/test/java/org/takes/rq/RqRequestLineTest.java new file mode 100644 index 000000000..8a48a2a1e --- /dev/null +++ b/src/test/java/org/takes/rq/RqRequestLineTest.java @@ -0,0 +1,256 @@ +/** + * The MIT License (MIT) + * + * Copyright (c) 2015 Yegor Bugayenko + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package org.takes.rq; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; +import org.junit.Test; +import org.takes.HttpException; + +/** + * Test case for {@link RqRequestLine.Base}. + * @author Vladimir Maksimenko (xupypr@xupypr.com) + * @version $Id$ + * @since 1.0 + */ +public final class RqRequestLineTest { + + /** + * RqRequestLine.Base should throw {@link HttpException} when + * we call requestLineHeader with + * Request without Request-Line. + * @throws IOException If some problem inside + */ + @Test(expected = HttpException.class) + public void failsOnAbsentRequestLine() throws IOException { + new RqRequestLine.Base( + new RqSimple(Collections.emptyList(), null) + ).requestLineHeader(); + } + + /** + * RqRequestLine.Base should throw {@link HttpException} when + * we call requestLineHeader with + * Request with illegal Request-Line. + * @throws IOException If some problem inside + */ + @Test(expected = HttpException.class) + public void failsOnIllegalRequestLine() throws IOException { + new RqRequestLine.Base( + new RqFake( + Arrays.asList( + "GIVE/contacts2", + "Host: 1.example.com" + ), + "" + ) + ).requestLineHeader(); + } + + /** + * RqRequestLine.Base can return Request-Line header + * we call requestLineHeader with valid Request-Line. + * @throws IOException If some problem inside + */ + @Test + public void extractsParams() throws IOException { + final String requestline = "GET /hello?a=6&b=7&c&d=9%28x%29&ff"; + MatcherAssert.assertThat( + new RqRequestLine.Base( + new RqFake( + Arrays.asList( + requestline, + "Host: a.example.com", + "Content-type: text/xml" + ), + "" + ) + ).requestLineHeader(), + Matchers.equalToIgnoringCase(requestline) + ); + } + + /** + * RqRequestLine.Base should throw {@link HttpException} when + * we call requestLineHeaderToken with + * Request without Request-Line. + * @throws IOException If some problem inside + */ + @Test(expected = HttpException.class) + public void failsOnAbsentRequestLineToken() throws IOException { + new RqRequestLine.Base( + new RqSimple(Collections.emptyList(), null) + ).requestLineHeaderToken(1); + } + + /** + * RqRequestLine.Base should throw {@link HttpException} when + * we call requestLineHeaderToken with + * Request with illegal Request-Line. + * @throws IOException If some problem inside + */ + @Test(expected = HttpException.class) + public void failsOnIllegalRequestLineToken() throws IOException { + new RqRequestLine.Base( + new RqFake( + Arrays.asList( + "GIVE/contacts", + "Host: 3.example.com" + ), + "" + ) + ).requestLineHeaderToken(1); + } + + /** + * RqRequestLine.Base should throw {@link IllegalArgumentException} when + * we call requestLineHeaderToken with index number less then 1. + * @throws IOException If some problem inside + */ + @Test(expected = IllegalArgumentException.class) + public void failsOnIllegalRequestLineTokenIndexLessOne() + throws IOException { + new RqRequestLine.Base( + new RqFake( + Arrays.asList( + "GET /hello?a=4&b=7&c&d=9%28x%29&ff", + "Host: 4.example.com" + ), + "" + ) + ).requestLineHeaderToken(0); + } + + /** + * RqRequestLine.Base should throw {@link IllegalArgumentException} when + * we call requestLineHeaderToken with index number greater then 3. + * @throws IOException If some problem inside + */ + @Test(expected = IllegalArgumentException.class) + public void failsOnIllegalRequestLineTokenIndexGreaterThree() + throws IOException { + new RqRequestLine.Base( + new RqFake( + Arrays.asList( + "GET /hello?a=5&b=7&c&d=9%28x%29&ff", + "Host: 5.example.com" + ), + "" + ) + // @checkstyle MagicNumberCheck (1 lines) + ).requestLineHeaderToken(4); + } + + /** + * RqRequestLine.Base can extract first token (METHOD) + * when we call requestLineHeaderToken + * with valid Request-Line. + * @throws IOException If some problem inside + */ + @Test + public void extractsFirstParam() throws IOException { + MatcherAssert.assertThat( + new RqRequestLine.Base( + new RqFake( + Arrays.asList( + "GET /hello?since=3431", + "Host: f1.example.com" + ), + "" + ) + ).requestLineHeaderToken(RqRequestLine.METHOD), + Matchers.equalToIgnoringCase("GET") + ); + } + + /** + * RqRequestLine.Base can extract second token (URI) + * when we call requestLineHeaderToken + * with valid Request-Line. + * @throws IOException If some problem inside + */ + @Test + public void extractsSecondParam() throws IOException { + MatcherAssert.assertThat( + new RqRequestLine.Base( + new RqFake( + Arrays.asList( + "GET /hello?since=3432", + "Host: f2.example.com" + ), + "" + ) + ).requestLineHeaderToken(RqRequestLine.URI), + Matchers.equalToIgnoringCase("/hello?since=3432") + ); + } + + /** + * RqRequestLine.Base can extract third token (HTTP VERSION) + * when we call requestLineHeaderToken + * with valid Request-Line. + * @throws IOException If some problem inside + */ + @Test + public void extractsThirdParam() throws IOException { + MatcherAssert.assertThat( + new RqRequestLine.Base( + new RqFake( + Arrays.asList( + "GET /hello?since=343 HTTP/1.1", + "Host: f3.example.com" + ), + "" + ) + ).requestLineHeaderToken(RqRequestLine.HTTPVERSION), + Matchers.equalToIgnoringCase("HTTP/1.1") + ); + } + + /** + * RqRequestLine.Base can extract third token (HTTP VERSION) + * when we call requestLineHeaderToken + * even for Request-Line without HTTP VERSION + * with valid Request-Line. + * @throws IOException If some problem inside + */ + @Test + public void extractsEmptyThirdParam() throws IOException { + MatcherAssert.assertThat( + new RqRequestLine.Base( + new RqFake( + Arrays.asList( + "GET /hello?since=3433", + "Host: f4.example.com" + ), + "" + ) + ).requestLineHeaderToken(RqRequestLine.HTTPVERSION), + Matchers.equalTo(null) + ); + } +} From 20c68060bb1f1c9878fe1807095031ce24e16b1a Mon Sep 17 00:00:00 2001 From: Maksimenko Vladimir Date: Sat, 26 Dec 2015 14:29:38 +0300 Subject: [PATCH 02/11] annotation fix --- src/main/java/org/takes/rq/RqRequestLine.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/takes/rq/RqRequestLine.java b/src/main/java/org/takes/rq/RqRequestLine.java index ab9fe53de..998208d63 100644 --- a/src/main/java/org/takes/rq/RqRequestLine.java +++ b/src/main/java/org/takes/rq/RqRequestLine.java @@ -40,7 +40,6 @@ * @version $Id$ * @since 1.0 */ -@EqualsAndHashCode(callSuper = true) public interface RqRequestLine extends Request { /** From f43f53b39297089e3c86e0bedd7cb075b047d80c Mon Sep 17 00:00:00 2001 From: Maksimenko Vladimir Date: Sat, 26 Dec 2015 14:45:10 +0300 Subject: [PATCH 03/11] checkstyle modifications --- src/main/java/org/takes/rq/RqRequestLine.java | 13 ++++++------- src/test/java/org/takes/rq/RqRequestLineTest.java | 1 + 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/takes/rq/RqRequestLine.java b/src/main/java/org/takes/rq/RqRequestLine.java index 998208d63..fc8c535d0 100644 --- a/src/main/java/org/takes/rq/RqRequestLine.java +++ b/src/main/java/org/takes/rq/RqRequestLine.java @@ -88,7 +88,7 @@ final class Base extends RqWrap implements RqRequestLine { * @see RFC 2616 */ - static final Pattern REQUEST_LINE_PATTERN = Pattern.compile( + private static final Pattern PATTERN = Pattern.compile( "([!-~]+) ([^ ]+)( [^ ]+)?" ); @@ -133,7 +133,7 @@ private String extractFromRequestLineHeader(final Integer index) ); } final String line = this.head().iterator().next(); - final Matcher matcher = REQUEST_LINE_PATTERN.matcher(line); + final Matcher matcher = PATTERN.matcher(line); if (!matcher.matches()) { throw new HttpException( HttpURLConnection.HTTP_BAD_REQUEST, @@ -146,15 +146,14 @@ private String extractFromRequestLineHeader(final Integer index) if (index != null) { final String token = matcher.group(index); final String result; - if (token != null) { - result = token.trim(); - } else { + if (token == null) { result = token; + } else { + result = token.trim(); } return result; - } else { - return line; } + return line; } } } diff --git a/src/test/java/org/takes/rq/RqRequestLineTest.java b/src/test/java/org/takes/rq/RqRequestLineTest.java index 8a48a2a1e..c44129e92 100644 --- a/src/test/java/org/takes/rq/RqRequestLineTest.java +++ b/src/test/java/org/takes/rq/RqRequestLineTest.java @@ -37,6 +37,7 @@ * @version $Id$ * @since 1.0 */ +@SuppressWarnings("PMD.TooManyMethods") public final class RqRequestLineTest { /** From 6665b3d0487acdb51af74a306a75d7df5c0e11c5 Mon Sep 17 00:00:00 2001 From: Maksimenko Vladimir Date: Mon, 28 Dec 2015 20:27:43 +0300 Subject: [PATCH 04/11] #473 refactoring according to CR --- src/main/java/org/takes/http/BkReuse.java | 51 ----- src/main/java/org/takes/rq/RqHref.java | 3 +- src/main/java/org/takes/rq/RqMethod.java | 3 +- src/main/java/org/takes/rq/RqRequestLine.java | 129 +++++++----- src/test/java/org/takes/http/BkReuseTest.java | 143 ------------- .../java/org/takes/rq/RqRequestLineTest.java | 193 +++++++----------- 6 files changed, 157 insertions(+), 365 deletions(-) delete mode 100644 src/main/java/org/takes/http/BkReuse.java delete mode 100644 src/test/java/org/takes/http/BkReuseTest.java diff --git a/src/main/java/org/takes/http/BkReuse.java b/src/main/java/org/takes/http/BkReuse.java deleted file mode 100644 index 80eb2d3dd..000000000 --- a/src/main/java/org/takes/http/BkReuse.java +++ /dev/null @@ -1,51 +0,0 @@ -/** - * The MIT License (MIT) - * - * Copyright (c) 2015 Yegor Bugayenko - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included - * in all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - */ -package org.takes.http; - -import java.io.IOException; -import java.net.Socket; - -/** - * Reusable back-end. - * - * @author Piotr Pradzynski (prondzyn@gmail.com) - * @version $Id$ - */ -public final class BkReuse extends BkWrap { - - /** - * Constructor of BkReuse. - * @param back Origin back-end. - */ - @SuppressWarnings("PMD.UnusedFormalParameter") - public BkReuse(final Back back) { - super(new Back() { - @Override - public void accept(final Socket socket) throws IOException { - throw new UnsupportedOperationException(); - } - }); - } - -} diff --git a/src/main/java/org/takes/rq/RqHref.java b/src/main/java/org/takes/rq/RqHref.java index 4f2895fd9..1963ed829 100644 --- a/src/main/java/org/takes/rq/RqHref.java +++ b/src/main/java/org/takes/rq/RqHref.java @@ -31,6 +31,7 @@ import org.takes.HttpException; import org.takes.Request; import org.takes.misc.Href; +import org.takes.rq.RqRequestLine.Token; /** * HTTP URI query parsing. @@ -72,7 +73,7 @@ public Base(final Request req) { @Override public Href href() throws IOException { final String uri = new RqRequestLine.Base(this) - .requestLineHeaderToken(RqRequestLine.URI); + .requestLineHeaderToken(Token.URI); return new Href( String.format( "http://%s%s", diff --git a/src/main/java/org/takes/rq/RqMethod.java b/src/main/java/org/takes/rq/RqMethod.java index 96201fbbc..45f7502c9 100644 --- a/src/main/java/org/takes/rq/RqMethod.java +++ b/src/main/java/org/takes/rq/RqMethod.java @@ -28,6 +28,7 @@ import java.util.regex.Pattern; import lombok.EqualsAndHashCode; import org.takes.Request; +import org.takes.rq.RqRequestLine.Token; /** * HTTP method parsing. @@ -122,7 +123,7 @@ public Base(final Request req) { @Override public String method() throws IOException { final String method = new RqRequestLine.Base(this) - .requestLineHeaderToken(RqRequestLine.METHOD); + .requestLineHeaderToken(Token.METHOD); if (SEPARATORS.matcher(method).find()) { throw new IOException( String.format("Invalid HTTP method: %s", method) diff --git a/src/main/java/org/takes/rq/RqRequestLine.java b/src/main/java/org/takes/rq/RqRequestLine.java index fc8c535d0..16eb3f40d 100644 --- a/src/main/java/org/takes/rq/RqRequestLine.java +++ b/src/main/java/org/takes/rq/RqRequestLine.java @@ -38,24 +38,39 @@ * * @author Vladimir Maksimenko (xupypr@xupypr.com) * @version $Id$ - * @since 1.0 + * @since 0.29.1 */ public interface RqRequestLine extends Request { - /** - * METHOD token. - */ - int METHOD = 1; + public static enum Token { + /** + * METHOD token. + */ + METHOD(1), - /** - * URI token. - */ - int URI = 2; + /** + * URI token. + */ + URI(2), - /** - * HTTPVERSION token. - */ - int HTTPVERSION = 3; + /** + * HTTPVERSION token. + */ + HTTPVERSION(3); + + /** + * Value. + */ + private final int value; + + /** + * Ctor. + * @param val Value + */ + private Token(final int val) { + this.value = val; + } + } /** * Get Request-Line header. @@ -65,12 +80,12 @@ public interface RqRequestLine extends Request { String requestLineHeader() throws IOException; /** - * Get Request-Line header token by index number. - * @param index Token index number - * @return HTTP Request-Line header token specified by index number + * Get Request-Line header token. + * @param token Token + * @return HTTP Request-Line header token * @throws IOException If fails */ - String requestLineHeaderToken(int index) throws IOException; + String requestLineHeaderToken(Token token) throws IOException; /** * Request decorator for Request-Line header validation @@ -89,8 +104,8 @@ final class Base extends RqWrap implements RqRequestLine { * .w3.org/Protocols/rfc2616/rfc2616-sec5.html#sec5.1">RFC 2616 */ private static final Pattern PATTERN = Pattern.compile( - "([!-~]+) ([^ ]+)( [^ ]+)?" - ); + "([!-~]+) ([^ ]+)( [^ ]+)?" + ); /** * Ctor. @@ -102,58 +117,66 @@ public Base(final Request req) { @Override public String requestLineHeader() throws IOException { - return this.extractFromRequestLineHeader(null); + final String requestLine = this.getRequestLineHeader(); + this.validateRequestLine(requestLine); + return requestLine; } @Override - public String requestLineHeaderToken(final int index) + public String requestLineHeaderToken(final Token token) throws IOException { - // @checkstyle MagicNumberCheck (1 lines) - if (index < 1 || index > 3) { + final String requestLine = this.getRequestLineHeader(); + final Matcher matcher = this.validateRequestLine(requestLine); + final String result = matcher.group(token.value); + if (result == null) { throw new IllegalArgumentException( - String.format("Illegel indexNumber %d", index) - ); + String.format( + "There is no token %s in Request-Line header: %s", + token.toString(), + requestLine + ) + ); } - return this.extractFromRequestLineHeader(index); + return result.trim(); } /** - * Extract Request-Line or Request-Line token from Request. + * Get Request-Line header. * - * @param index Token index number - * @return Request-Line if index is null, token as string otherwise + * @return Valid Request-Line header * @throws IOException If fails */ - private String extractFromRequestLineHeader(final Integer index) - throws IOException { + private String getRequestLineHeader() throws IOException { if (!this.head().iterator().hasNext()) { throw new HttpException( - HttpURLConnection.HTTP_BAD_REQUEST, - "HTTP Request should have Request-Line" - ); + HttpURLConnection.HTTP_BAD_REQUEST, + "HTTP Request should have Request-Line" + ); } - final String line = this.head().iterator().next(); - final Matcher matcher = PATTERN.matcher(line); + final String requestLine = this.head().iterator().next(); + return requestLine; + } + + /** + * Validate Request-Line according to PATTERN. + * + * @param requestline Request-Line header + * @return Matcher that can be used to extract tokens + * @throws HttpException If fails + */ + private Matcher validateRequestLine(final String requestline) + throws HttpException { + final Matcher matcher = PATTERN.matcher(requestline); if (!matcher.matches()) { throw new HttpException( - HttpURLConnection.HTTP_BAD_REQUEST, - String.format( - "Invalid HTTP Request-Line header: %s", - line - ) - ); - } - if (index != null) { - final String token = matcher.group(index); - final String result; - if (token == null) { - result = token; - } else { - result = token.trim(); - } - return result; + HttpURLConnection.HTTP_BAD_REQUEST, + String.format( + "Invalid HTTP Request-Line header: %s", + requestline + ) + ); } - return line; + return matcher; } } } diff --git a/src/test/java/org/takes/http/BkReuseTest.java b/src/test/java/org/takes/http/BkReuseTest.java deleted file mode 100644 index 3c0e03a58..000000000 --- a/src/test/java/org/takes/http/BkReuseTest.java +++ /dev/null @@ -1,143 +0,0 @@ -/** - * The MIT License (MIT) - * - * Copyright (c) 2015 Yegor Bugayenko - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included - * in all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - */ -package org.takes.http; - -import com.google.common.base.Joiner; -import com.jcabi.http.mock.MkAnswer; -import com.jcabi.http.mock.MkContainer; -import com.jcabi.http.mock.MkGrizzlyContainer; -import com.jcabi.matchers.RegexMatchers; -import java.net.Socket; -import java.net.URI; -import org.hamcrest.MatcherAssert; -import org.hamcrest.Matchers; -import org.junit.Ignore; -import org.junit.Test; -import org.takes.tk.TkText; - -/** - * Test case for {@link BkReuse}. - * - * @author Piotr Pradzynski (prondzyn@gmail.com) - * @version $Id$ - * @checkstyle MultipleStringLiteralsCheck (500 lines) - * @todo #306:30min Implement missing BkReuse.accept method to support - * HTTP persistent connections and to pass below three tests. BkReuse.accept - * should handles more than one HTTP requests in one connection and return - * correct HTTP status when Content-Length is not specified. - */ -@SuppressWarnings("PMD.AvoidDuplicateLiterals") -public final class BkReuseTest { - - /** - * BkReuse can handle two requests in one connection. - * @throws Exception If some problem inside - */ - @Ignore - @Test - public void handlesTwoRequestInOneConnection() throws Exception { - final String text = "Hello world!"; - final MkContainer container = new MkGrizzlyContainer().next( - new MkAnswer.Simple( - Joiner.on("\r\n").join( - "POST / HTTP/1.1", - "Host: localhost", - "Content-Length: 4", - "", - "hi", - "POST / HTTP/1.1", - "Host: localhost", - "Content-Length: 4", - "", - "hi" - ) - ) - ).start(); - final URI uri = container.home(); - final Socket socket = new Socket(uri.getHost(), uri.getPort()); - new BkReuse(new BkBasic(new TkText(text))).accept(socket); - container.stop(); - MatcherAssert.assertThat( - socket.getOutputStream().toString(), - RegexMatchers.containsPattern(text + ".*?" + text) - ); - } - - /** - * BkReuse can return HTTP status 411 when a persistent connection request - * has no Content-Length. - * @throws Exception If some problem inside - */ - @Ignore - @Test - public void returnsProperResponseCodeOnNoContentLength() throws Exception { - final MkContainer container = new MkGrizzlyContainer().next( - new MkAnswer.Simple( - Joiner.on("\r\n").join( - "POST / HTTP/1.1", - "Host: localhost", - "", - "hi" - ) - ) - ).start(); - final URI uri = container.home(); - final Socket socket = new Socket(uri.getHost(), uri.getPort()); - new BkReuse(new BkBasic(new TkText("411 Test"))).accept(socket); - container.stop(); - MatcherAssert.assertThat( - socket.getOutputStream().toString(), - Matchers.containsString("HTTP/1.1 411 Length Required") - ); - } - - /** - * BkReuse can accept no content-length on closed connection. - * @throws Exception If some problem inside - */ - @Ignore - @Test - public void acceptsNoContentLengthOnClosedConnection() throws Exception { - final String text = "Close Test"; - final MkContainer container = new MkGrizzlyContainer().next( - new MkAnswer.Simple( - Joiner.on("\r\n").join( - "POST / HTTP/1.1", - "Host: localhost", - "Connection: Close", - "", - "hi" - ) - ) - ).start(); - final URI uri = container.home(); - final Socket socket = new Socket(uri.getHost(), uri.getPort()); - new BkReuse(new BkBasic(new TkText(text))).accept(socket); - container.stop(); - MatcherAssert.assertThat( - socket.getOutputStream().toString(), - Matchers.containsString(text) - ); - } -} diff --git a/src/test/java/org/takes/rq/RqRequestLineTest.java b/src/test/java/org/takes/rq/RqRequestLineTest.java index c44129e92..4ea3ce2e4 100644 --- a/src/test/java/org/takes/rq/RqRequestLineTest.java +++ b/src/test/java/org/takes/rq/RqRequestLineTest.java @@ -30,12 +30,13 @@ import org.hamcrest.Matchers; import org.junit.Test; import org.takes.HttpException; +import org.takes.rq.RqRequestLine.Token; /** * Test case for {@link RqRequestLine.Base}. * @author Vladimir Maksimenko (xupypr@xupypr.com) * @version $Id$ - * @since 1.0 + * @since 0.29.1 */ @SuppressWarnings("PMD.TooManyMethods") public final class RqRequestLineTest { @@ -49,8 +50,8 @@ public final class RqRequestLineTest { @Test(expected = HttpException.class) public void failsOnAbsentRequestLine() throws IOException { new RqRequestLine.Base( - new RqSimple(Collections.emptyList(), null) - ).requestLineHeader(); + new RqSimple(Collections.emptyList(), null) + ).requestLineHeader(); } /** @@ -62,14 +63,14 @@ public void failsOnAbsentRequestLine() throws IOException { @Test(expected = HttpException.class) public void failsOnIllegalRequestLine() throws IOException { new RqRequestLine.Base( - new RqFake( - Arrays.asList( - "GIVE/contacts2", - "Host: 1.example.com" - ), - "" - ) - ).requestLineHeader(); + new RqFake( + Arrays.asList( + "GIVE/contacts2", + "Host: 1.example.com" + ), + "" + ) + ).requestLineHeader(); } /** @@ -81,17 +82,17 @@ public void failsOnIllegalRequestLine() throws IOException { public void extractsParams() throws IOException { final String requestline = "GET /hello?a=6&b=7&c&d=9%28x%29&ff"; MatcherAssert.assertThat( - new RqRequestLine.Base( - new RqFake( - Arrays.asList( - requestline, - "Host: a.example.com", - "Content-type: text/xml" - ), - "" - ) - ).requestLineHeader(), - Matchers.equalToIgnoringCase(requestline) + new RqRequestLine.Base( + new RqFake( + Arrays.asList( + requestline, + "Host: a.example.com", + "Content-type: text/xml" + ), + "" + ) + ).requestLineHeader(), + Matchers.equalToIgnoringCase(requestline) ); } @@ -104,8 +105,8 @@ public void extractsParams() throws IOException { @Test(expected = HttpException.class) public void failsOnAbsentRequestLineToken() throws IOException { new RqRequestLine.Base( - new RqSimple(Collections.emptyList(), null) - ).requestLineHeaderToken(1); + new RqSimple(Collections.emptyList(), null) + ).requestLineHeaderToken(Token.METHOD); } /** @@ -117,53 +118,14 @@ public void failsOnAbsentRequestLineToken() throws IOException { @Test(expected = HttpException.class) public void failsOnIllegalRequestLineToken() throws IOException { new RqRequestLine.Base( - new RqFake( - Arrays.asList( - "GIVE/contacts", - "Host: 3.example.com" - ), - "" - ) - ).requestLineHeaderToken(1); - } - - /** - * RqRequestLine.Base should throw {@link IllegalArgumentException} when - * we call requestLineHeaderToken with index number less then 1. - * @throws IOException If some problem inside - */ - @Test(expected = IllegalArgumentException.class) - public void failsOnIllegalRequestLineTokenIndexLessOne() - throws IOException { - new RqRequestLine.Base( - new RqFake( - Arrays.asList( - "GET /hello?a=4&b=7&c&d=9%28x%29&ff", - "Host: 4.example.com" - ), - "" - ) - ).requestLineHeaderToken(0); - } - - /** - * RqRequestLine.Base should throw {@link IllegalArgumentException} when - * we call requestLineHeaderToken with index number greater then 3. - * @throws IOException If some problem inside - */ - @Test(expected = IllegalArgumentException.class) - public void failsOnIllegalRequestLineTokenIndexGreaterThree() - throws IOException { - new RqRequestLine.Base( - new RqFake( - Arrays.asList( - "GET /hello?a=5&b=7&c&d=9%28x%29&ff", - "Host: 5.example.com" - ), - "" - ) - // @checkstyle MagicNumberCheck (1 lines) - ).requestLineHeaderToken(4); + new RqFake( + Arrays.asList( + "GIVE/contacts", + "Host: 3.example.com" + ), + "" + ) + ).requestLineHeaderToken(Token.METHOD); } /** @@ -175,16 +137,16 @@ public void failsOnIllegalRequestLineTokenIndexGreaterThree() @Test public void extractsFirstParam() throws IOException { MatcherAssert.assertThat( - new RqRequestLine.Base( - new RqFake( - Arrays.asList( - "GET /hello?since=3431", - "Host: f1.example.com" - ), - "" - ) - ).requestLineHeaderToken(RqRequestLine.METHOD), - Matchers.equalToIgnoringCase("GET") + new RqRequestLine.Base( + new RqFake( + Arrays.asList( + "GET /hello?since=3431", + "Host: f1.example.com" + ), + "" + ) + ).requestLineHeaderToken(Token.METHOD), + Matchers.equalToIgnoringCase("GET") ); } @@ -197,16 +159,16 @@ public void extractsFirstParam() throws IOException { @Test public void extractsSecondParam() throws IOException { MatcherAssert.assertThat( - new RqRequestLine.Base( - new RqFake( - Arrays.asList( - "GET /hello?since=3432", - "Host: f2.example.com" - ), - "" - ) - ).requestLineHeaderToken(RqRequestLine.URI), - Matchers.equalToIgnoringCase("/hello?since=3432") + new RqRequestLine.Base( + new RqFake( + Arrays.asList( + "GET /hello?since=3432", + "Host: f2.example.com" + ), + "" + ) + ).requestLineHeaderToken(Token.URI), + Matchers.equalToIgnoringCase("/hello?since=3432") ); } @@ -219,39 +181,38 @@ public void extractsSecondParam() throws IOException { @Test public void extractsThirdParam() throws IOException { MatcherAssert.assertThat( - new RqRequestLine.Base( - new RqFake( - Arrays.asList( - "GET /hello?since=343 HTTP/1.1", - "Host: f3.example.com" - ), - "" - ) - ).requestLineHeaderToken(RqRequestLine.HTTPVERSION), - Matchers.equalToIgnoringCase("HTTP/1.1") + new RqRequestLine.Base( + new RqFake( + Arrays.asList( + "GET /hello?since=343 HTTP/1.1", + "Host: f3.example.com" + ), + "" + ) + ).requestLineHeaderToken(Token.HTTPVERSION), + Matchers.equalToIgnoringCase("HTTP/1.1") ); } /** - * RqRequestLine.Base can extract third token (HTTP VERSION) - * when we call requestLineHeaderToken - * even for Request-Line without HTTP VERSION - * with valid Request-Line. + * RqRequestLine.Base should throw {@link IllegalArgumentException} + * when we call requestLineHeaderToken(Token.HTTPVERSION) + * even for valid Request-Line without HTTP VERSION. * @throws IOException If some problem inside */ - @Test + @Test(expected = IllegalArgumentException.class) public void extractsEmptyThirdParam() throws IOException { MatcherAssert.assertThat( - new RqRequestLine.Base( - new RqFake( - Arrays.asList( - "GET /hello?since=3433", - "Host: f4.example.com" - ), - "" - ) - ).requestLineHeaderToken(RqRequestLine.HTTPVERSION), - Matchers.equalTo(null) + new RqRequestLine.Base( + new RqFake( + Arrays.asList( + "GET /hello?since=3433", + "Host: f4.example.com" + ), + "" + ) + ).requestLineHeaderToken(Token.HTTPVERSION), + Matchers.equalTo(null) ); } } From ffb721e1b5c689e0dcb40c48352c37e2b912f240 Mon Sep 17 00:00:00 2001 From: Maksimenko Vladimir Date: Mon, 28 Dec 2015 20:33:50 +0300 Subject: [PATCH 05/11] smallfix --- src/main/java/org/takes/rq/RqRequestLine.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/org/takes/rq/RqRequestLine.java b/src/main/java/org/takes/rq/RqRequestLine.java index 16eb3f40d..fea10c19b 100644 --- a/src/main/java/org/takes/rq/RqRequestLine.java +++ b/src/main/java/org/takes/rq/RqRequestLine.java @@ -153,8 +153,7 @@ private String getRequestLineHeader() throws IOException { "HTTP Request should have Request-Line" ); } - final String requestLine = this.head().iterator().next(); - return requestLine; + return this.head().iterator().next(); } /** From dd4907178e9e853f762cd6c0c79762d2f194f09e Mon Sep 17 00:00:00 2001 From: Maksimenko Vladimir Date: Mon, 28 Dec 2015 20:34:58 +0300 Subject: [PATCH 06/11] identation fix --- src/main/java/org/takes/rq/RqRequestLine.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/takes/rq/RqRequestLine.java b/src/main/java/org/takes/rq/RqRequestLine.java index fea10c19b..eea64cb7f 100644 --- a/src/main/java/org/takes/rq/RqRequestLine.java +++ b/src/main/java/org/takes/rq/RqRequestLine.java @@ -124,7 +124,7 @@ public String requestLineHeader() throws IOException { @Override public String requestLineHeaderToken(final Token token) - throws IOException { + throws IOException { final String requestLine = this.getRequestLineHeader(); final Matcher matcher = this.validateRequestLine(requestLine); final String result = matcher.group(token.value); From b66a2141975e76eee39c2e3b7a7d5d4b60223327 Mon Sep 17 00:00:00 2001 From: Maksimenko Vladimir Date: Mon, 28 Dec 2015 23:35:38 +0300 Subject: [PATCH 07/11] rollback --- src/main/java/org/takes/http/BkReuse.java | 51 +++++++ src/test/java/org/takes/http/BkReuseTest.java | 143 ++++++++++++++++++ 2 files changed, 194 insertions(+) create mode 100644 src/main/java/org/takes/http/BkReuse.java create mode 100644 src/test/java/org/takes/http/BkReuseTest.java diff --git a/src/main/java/org/takes/http/BkReuse.java b/src/main/java/org/takes/http/BkReuse.java new file mode 100644 index 000000000..80eb2d3dd --- /dev/null +++ b/src/main/java/org/takes/http/BkReuse.java @@ -0,0 +1,51 @@ +/** + * The MIT License (MIT) + * + * Copyright (c) 2015 Yegor Bugayenko + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package org.takes.http; + +import java.io.IOException; +import java.net.Socket; + +/** + * Reusable back-end. + * + * @author Piotr Pradzynski (prondzyn@gmail.com) + * @version $Id$ + */ +public final class BkReuse extends BkWrap { + + /** + * Constructor of BkReuse. + * @param back Origin back-end. + */ + @SuppressWarnings("PMD.UnusedFormalParameter") + public BkReuse(final Back back) { + super(new Back() { + @Override + public void accept(final Socket socket) throws IOException { + throw new UnsupportedOperationException(); + } + }); + } + +} diff --git a/src/test/java/org/takes/http/BkReuseTest.java b/src/test/java/org/takes/http/BkReuseTest.java new file mode 100644 index 000000000..3c0e03a58 --- /dev/null +++ b/src/test/java/org/takes/http/BkReuseTest.java @@ -0,0 +1,143 @@ +/** + * The MIT License (MIT) + * + * Copyright (c) 2015 Yegor Bugayenko + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package org.takes.http; + +import com.google.common.base.Joiner; +import com.jcabi.http.mock.MkAnswer; +import com.jcabi.http.mock.MkContainer; +import com.jcabi.http.mock.MkGrizzlyContainer; +import com.jcabi.matchers.RegexMatchers; +import java.net.Socket; +import java.net.URI; +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; +import org.junit.Ignore; +import org.junit.Test; +import org.takes.tk.TkText; + +/** + * Test case for {@link BkReuse}. + * + * @author Piotr Pradzynski (prondzyn@gmail.com) + * @version $Id$ + * @checkstyle MultipleStringLiteralsCheck (500 lines) + * @todo #306:30min Implement missing BkReuse.accept method to support + * HTTP persistent connections and to pass below three tests. BkReuse.accept + * should handles more than one HTTP requests in one connection and return + * correct HTTP status when Content-Length is not specified. + */ +@SuppressWarnings("PMD.AvoidDuplicateLiterals") +public final class BkReuseTest { + + /** + * BkReuse can handle two requests in one connection. + * @throws Exception If some problem inside + */ + @Ignore + @Test + public void handlesTwoRequestInOneConnection() throws Exception { + final String text = "Hello world!"; + final MkContainer container = new MkGrizzlyContainer().next( + new MkAnswer.Simple( + Joiner.on("\r\n").join( + "POST / HTTP/1.1", + "Host: localhost", + "Content-Length: 4", + "", + "hi", + "POST / HTTP/1.1", + "Host: localhost", + "Content-Length: 4", + "", + "hi" + ) + ) + ).start(); + final URI uri = container.home(); + final Socket socket = new Socket(uri.getHost(), uri.getPort()); + new BkReuse(new BkBasic(new TkText(text))).accept(socket); + container.stop(); + MatcherAssert.assertThat( + socket.getOutputStream().toString(), + RegexMatchers.containsPattern(text + ".*?" + text) + ); + } + + /** + * BkReuse can return HTTP status 411 when a persistent connection request + * has no Content-Length. + * @throws Exception If some problem inside + */ + @Ignore + @Test + public void returnsProperResponseCodeOnNoContentLength() throws Exception { + final MkContainer container = new MkGrizzlyContainer().next( + new MkAnswer.Simple( + Joiner.on("\r\n").join( + "POST / HTTP/1.1", + "Host: localhost", + "", + "hi" + ) + ) + ).start(); + final URI uri = container.home(); + final Socket socket = new Socket(uri.getHost(), uri.getPort()); + new BkReuse(new BkBasic(new TkText("411 Test"))).accept(socket); + container.stop(); + MatcherAssert.assertThat( + socket.getOutputStream().toString(), + Matchers.containsString("HTTP/1.1 411 Length Required") + ); + } + + /** + * BkReuse can accept no content-length on closed connection. + * @throws Exception If some problem inside + */ + @Ignore + @Test + public void acceptsNoContentLengthOnClosedConnection() throws Exception { + final String text = "Close Test"; + final MkContainer container = new MkGrizzlyContainer().next( + new MkAnswer.Simple( + Joiner.on("\r\n").join( + "POST / HTTP/1.1", + "Host: localhost", + "Connection: Close", + "", + "hi" + ) + ) + ).start(); + final URI uri = container.home(); + final Socket socket = new Socket(uri.getHost(), uri.getPort()); + new BkReuse(new BkBasic(new TkText(text))).accept(socket); + container.stop(); + MatcherAssert.assertThat( + socket.getOutputStream().toString(), + Matchers.containsString(text) + ); + } +} From a9d7c999222c6fd3eeeb902fa9ba18c8063862a0 Mon Sep 17 00:00:00 2001 From: Maksimenko Vladimir Date: Mon, 4 Jan 2016 10:29:59 +0300 Subject: [PATCH 08/11] refactoring for #473 according to @yegor256 CR --- src/main/java/org/takes/rq/RqHref.java | 3 +- src/main/java/org/takes/rq/RqMethod.java | 3 +- src/main/java/org/takes/rq/RqRequestLine.java | 103 ++++++++++++------ .../java/org/takes/rq/RqRequestLineTest.java | 13 +-- 4 files changed, 76 insertions(+), 46 deletions(-) diff --git a/src/main/java/org/takes/rq/RqHref.java b/src/main/java/org/takes/rq/RqHref.java index 1963ed829..39d4f82a0 100644 --- a/src/main/java/org/takes/rq/RqHref.java +++ b/src/main/java/org/takes/rq/RqHref.java @@ -31,7 +31,6 @@ import org.takes.HttpException; import org.takes.Request; import org.takes.misc.Href; -import org.takes.rq.RqRequestLine.Token; /** * HTTP URI query parsing. @@ -73,7 +72,7 @@ public Base(final Request req) { @Override public Href href() throws IOException { final String uri = new RqRequestLine.Base(this) - .requestLineHeaderToken(Token.URI); + .requestUri(); return new Href( String.format( "http://%s%s", diff --git a/src/main/java/org/takes/rq/RqMethod.java b/src/main/java/org/takes/rq/RqMethod.java index 45f7502c9..db447d5f6 100644 --- a/src/main/java/org/takes/rq/RqMethod.java +++ b/src/main/java/org/takes/rq/RqMethod.java @@ -28,7 +28,6 @@ import java.util.regex.Pattern; import lombok.EqualsAndHashCode; import org.takes.Request; -import org.takes.rq.RqRequestLine.Token; /** * HTTP method parsing. @@ -123,7 +122,7 @@ public Base(final Request req) { @Override public String method() throws IOException { final String method = new RqRequestLine.Base(this) - .requestLineHeaderToken(Token.METHOD); + .method(); if (SEPARATORS.matcher(method).find()) { throw new IOException( String.format("Invalid HTTP method: %s", method) diff --git a/src/main/java/org/takes/rq/RqRequestLine.java b/src/main/java/org/takes/rq/RqRequestLine.java index eea64cb7f..a315d7083 100644 --- a/src/main/java/org/takes/rq/RqRequestLine.java +++ b/src/main/java/org/takes/rq/RqRequestLine.java @@ -42,36 +42,6 @@ */ public interface RqRequestLine extends Request { - public static enum Token { - /** - * METHOD token. - */ - METHOD(1), - - /** - * URI token. - */ - URI(2), - - /** - * HTTPVERSION token. - */ - HTTPVERSION(3); - - /** - * Value. - */ - private final int value; - - /** - * Ctor. - * @param val Value - */ - private Token(final int val) { - this.value = val; - } - } - /** * Get Request-Line header. * @return HTTP Request-Line header @@ -80,12 +50,25 @@ private Token(final int val) { String requestLineHeader() throws IOException; /** - * Get Request-Line header token. - * @param token Token - * @return HTTP Request-Line header token + * Get Request-Line method token. + * @return HTTP Request-Line method token + * @throws IOException If fails + */ + String method() throws IOException; + + /** + * Get Request-Line Request-URI token. + * @return HTTP Request-Line method token * @throws IOException If fails */ - String requestLineHeaderToken(Token token) throws IOException; + String requestUri() throws IOException; + + /** + * Get Request-Line HTTP-Version token. + * @return HTTP Request-Line method token + * @throws IOException If fails + */ + String httpVersion() throws IOException; /** * Request decorator for Request-Line header validation @@ -107,6 +90,36 @@ final class Base extends RqWrap implements RqRequestLine { "([!-~]+) ([^ ]+)( [^ ]+)?" ); + private static enum Token { + /** + * METHOD token. + */ + METHOD(1), + + /** + * URI token. + */ + URI(2), + + /** + * HTTPVERSION token. + */ + HTTPVERSION(3); + + /** + * Value. + */ + private final int value; + + /** + * Ctor. + * @param val Value + */ + private Token(final int val) { + this.value = val; + } + } + /** * Ctor. * @param req Original request @@ -123,7 +136,27 @@ public String requestLineHeader() throws IOException { } @Override - public String requestLineHeaderToken(final Token token) + public String method() throws IOException { + return this.requestLineHeaderToken(Token.METHOD); + } + + @Override + public String requestUri() throws IOException { + return this.requestLineHeaderToken(Token.URI); + } + + @Override + public String httpVersion() throws IOException { + return this.requestLineHeaderToken(Token.HTTPVERSION); + } + + /** + * Get Request-Line header token. + * @param token Token + * @return HTTP Request-Line header token + * @throws IOException If fails + */ + private String requestLineHeaderToken(final Token token) throws IOException { final String requestLine = this.getRequestLineHeader(); final Matcher matcher = this.validateRequestLine(requestLine); diff --git a/src/test/java/org/takes/rq/RqRequestLineTest.java b/src/test/java/org/takes/rq/RqRequestLineTest.java index 4ea3ce2e4..f300d72e4 100644 --- a/src/test/java/org/takes/rq/RqRequestLineTest.java +++ b/src/test/java/org/takes/rq/RqRequestLineTest.java @@ -30,7 +30,6 @@ import org.hamcrest.Matchers; import org.junit.Test; import org.takes.HttpException; -import org.takes.rq.RqRequestLine.Token; /** * Test case for {@link RqRequestLine.Base}. @@ -106,7 +105,7 @@ public void extractsParams() throws IOException { public void failsOnAbsentRequestLineToken() throws IOException { new RqRequestLine.Base( new RqSimple(Collections.emptyList(), null) - ).requestLineHeaderToken(Token.METHOD); + ).method(); } /** @@ -125,7 +124,7 @@ public void failsOnIllegalRequestLineToken() throws IOException { ), "" ) - ).requestLineHeaderToken(Token.METHOD); + ).method(); } /** @@ -145,7 +144,7 @@ public void extractsFirstParam() throws IOException { ), "" ) - ).requestLineHeaderToken(Token.METHOD), + ).method(), Matchers.equalToIgnoringCase("GET") ); } @@ -167,7 +166,7 @@ public void extractsSecondParam() throws IOException { ), "" ) - ).requestLineHeaderToken(Token.URI), + ).requestUri(), Matchers.equalToIgnoringCase("/hello?since=3432") ); } @@ -189,7 +188,7 @@ public void extractsThirdParam() throws IOException { ), "" ) - ).requestLineHeaderToken(Token.HTTPVERSION), + ).httpVersion(), Matchers.equalToIgnoringCase("HTTP/1.1") ); } @@ -211,7 +210,7 @@ public void extractsEmptyThirdParam() throws IOException { ), "" ) - ).requestLineHeaderToken(Token.HTTPVERSION), + ).httpVersion(), Matchers.equalTo(null) ); } From a28422de09be786c8bd2e92f6a6db0b4cc3098b3 Mon Sep 17 00:00:00 2001 From: Maksimenko Vladimir Date: Mon, 4 Jan 2016 19:44:52 +0300 Subject: [PATCH 09/11] #473 refacroting according to @yegor256 CR --- src/main/java/org/takes/rq/RqHref.java | 2 +- src/main/java/org/takes/rq/RqRequestLine.java | 49 +++++++++---------- .../java/org/takes/rq/RqRequestLineTest.java | 12 ++--- 3 files changed, 30 insertions(+), 33 deletions(-) diff --git a/src/main/java/org/takes/rq/RqHref.java b/src/main/java/org/takes/rq/RqHref.java index 39d4f82a0..709e10507 100644 --- a/src/main/java/org/takes/rq/RqHref.java +++ b/src/main/java/org/takes/rq/RqHref.java @@ -72,7 +72,7 @@ public Base(final Request req) { @Override public Href href() throws IOException { final String uri = new RqRequestLine.Base(this) - .requestUri(); + .uri(); return new Href( String.format( "http://%s%s", diff --git a/src/main/java/org/takes/rq/RqRequestLine.java b/src/main/java/org/takes/rq/RqRequestLine.java index a315d7083..df39117dc 100644 --- a/src/main/java/org/takes/rq/RqRequestLine.java +++ b/src/main/java/org/takes/rq/RqRequestLine.java @@ -47,7 +47,7 @@ public interface RqRequestLine extends Request { * @return HTTP Request-Line header * @throws IOException If fails */ - String requestLineHeader() throws IOException; + String header() throws IOException; /** * Get Request-Line method token. @@ -61,14 +61,14 @@ public interface RqRequestLine extends Request { * @return HTTP Request-Line method token * @throws IOException If fails */ - String requestUri() throws IOException; + String uri() throws IOException; /** * Get Request-Line HTTP-Version token. * @return HTTP Request-Line method token * @throws IOException If fails */ - String httpVersion() throws IOException; + String version() throws IOException; /** * Request decorator for Request-Line header validation @@ -83,8 +83,7 @@ final class Base extends RqWrap implements RqRequestLine { /** * HTTP Request-line pattern. * [!-~] is for method or extension-method token (octets 33 - 126). - * @see RFC 2616 + * @see RFC 2616 */ private static final Pattern PATTERN = Pattern.compile( "([!-~]+) ([^ ]+)( [^ ]+)?" @@ -129,25 +128,25 @@ public Base(final Request req) { } @Override - public String requestLineHeader() throws IOException { - final String requestLine = this.getRequestLineHeader(); - this.validateRequestLine(requestLine); - return requestLine; + public String header() throws IOException { + final String line = this.line(); + this.matcher(line); + return line; } @Override public String method() throws IOException { - return this.requestLineHeaderToken(Token.METHOD); + return this.token(Token.METHOD); } @Override - public String requestUri() throws IOException { - return this.requestLineHeaderToken(Token.URI); + public String uri() throws IOException { + return this.token(Token.URI); } @Override - public String httpVersion() throws IOException { - return this.requestLineHeaderToken(Token.HTTPVERSION); + public String version() throws IOException { + return this.token(Token.HTTPVERSION); } /** @@ -156,17 +155,15 @@ public String httpVersion() throws IOException { * @return HTTP Request-Line header token * @throws IOException If fails */ - private String requestLineHeaderToken(final Token token) + private String token(final Token token) throws IOException { - final String requestLine = this.getRequestLineHeader(); - final Matcher matcher = this.validateRequestLine(requestLine); - final String result = matcher.group(token.value); + final String result = this.matcher(this.line()) + .group(token.value); if (result == null) { throw new IllegalArgumentException( String.format( - "There is no token %s in Request-Line header: %s", - token.toString(), - requestLine + "There is no token %s in Request-Line header", + token.toString() ) ); } @@ -179,7 +176,7 @@ private String requestLineHeaderToken(final Token token) * @return Valid Request-Line header * @throws IOException If fails */ - private String getRequestLineHeader() throws IOException { + private String line() throws IOException { if (!this.head().iterator().hasNext()) { throw new HttpException( HttpURLConnection.HTTP_BAD_REQUEST, @@ -192,19 +189,19 @@ private String getRequestLineHeader() throws IOException { /** * Validate Request-Line according to PATTERN. * - * @param requestline Request-Line header + * @param line Request-Line header * @return Matcher that can be used to extract tokens * @throws HttpException If fails */ - private Matcher validateRequestLine(final String requestline) + private Matcher matcher(final String line) throws HttpException { - final Matcher matcher = PATTERN.matcher(requestline); + final Matcher matcher = PATTERN.matcher(line); if (!matcher.matches()) { throw new HttpException( HttpURLConnection.HTTP_BAD_REQUEST, String.format( "Invalid HTTP Request-Line header: %s", - requestline + line ) ); } diff --git a/src/test/java/org/takes/rq/RqRequestLineTest.java b/src/test/java/org/takes/rq/RqRequestLineTest.java index f300d72e4..a01ac53eb 100644 --- a/src/test/java/org/takes/rq/RqRequestLineTest.java +++ b/src/test/java/org/takes/rq/RqRequestLineTest.java @@ -50,7 +50,7 @@ public final class RqRequestLineTest { public void failsOnAbsentRequestLine() throws IOException { new RqRequestLine.Base( new RqSimple(Collections.emptyList(), null) - ).requestLineHeader(); + ).header(); } /** @@ -69,7 +69,7 @@ public void failsOnIllegalRequestLine() throws IOException { ), "" ) - ).requestLineHeader(); + ).header(); } /** @@ -90,7 +90,7 @@ public void extractsParams() throws IOException { ), "" ) - ).requestLineHeader(), + ).header(), Matchers.equalToIgnoringCase(requestline) ); } @@ -166,7 +166,7 @@ public void extractsSecondParam() throws IOException { ), "" ) - ).requestUri(), + ).uri(), Matchers.equalToIgnoringCase("/hello?since=3432") ); } @@ -188,7 +188,7 @@ public void extractsThirdParam() throws IOException { ), "" ) - ).httpVersion(), + ).version(), Matchers.equalToIgnoringCase("HTTP/1.1") ); } @@ -210,7 +210,7 @@ public void extractsEmptyThirdParam() throws IOException { ), "" ) - ).httpVersion(), + ).version(), Matchers.equalTo(null) ); } From a9619811acf1f2a5667532bcbb2e3a238d81fc4f Mon Sep 17 00:00:00 2001 From: Maksimenko Vladimir Date: Mon, 4 Jan 2016 19:53:09 +0300 Subject: [PATCH 10/11] check style modifications --- src/main/java/org/takes/rq/RqRequestLine.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/takes/rq/RqRequestLine.java b/src/main/java/org/takes/rq/RqRequestLine.java index df39117dc..4266d894c 100644 --- a/src/main/java/org/takes/rq/RqRequestLine.java +++ b/src/main/java/org/takes/rq/RqRequestLine.java @@ -40,6 +40,7 @@ * @version $Id$ * @since 0.29.1 */ +@SuppressWarnings("PMD.TooManyMethods") public interface RqRequestLine extends Request { /** From c343a536c80fc5ae098ed49bcaed434e91f2e294 Mon Sep 17 00:00:00 2001 From: Maksimenko Vladimir Date: Tue, 5 Jan 2016 00:26:37 +0300 Subject: [PATCH 11/11] #473 more FP --- src/main/java/org/takes/rq/RqRequestLine.java | 64 ++++++++++++++----- 1 file changed, 49 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/takes/rq/RqRequestLine.java b/src/main/java/org/takes/rq/RqRequestLine.java index 4266d894c..48781a60f 100644 --- a/src/main/java/org/takes/rq/RqRequestLine.java +++ b/src/main/java/org/takes/rq/RqRequestLine.java @@ -130,9 +130,7 @@ public Base(final Request req) { @Override public String header() throws IOException { - final String line = this.line(); - this.matcher(line); - return line; + return this.validated(this.line()); } @Override @@ -158,17 +156,10 @@ public String version() throws IOException { */ private String token(final Token token) throws IOException { - final String result = this.matcher(this.line()) - .group(token.value); - if (result == null) { - throw new IllegalArgumentException( - String.format( - "There is no token %s in Request-Line header", - token.toString() - ) - ); - } - return result.trim(); + return this.trimmed( + this.matcher(this.line()).group(token.value), + token + ); } /** @@ -188,7 +179,8 @@ private String line() throws IOException { } /** - * Validate Request-Line according to PATTERN. + * Validate Request-Line according to PATTERN + * and return matcher. * * @param line Request-Line header * @return Matcher that can be used to extract tokens @@ -201,6 +193,7 @@ private Matcher matcher(final String line) throw new HttpException( HttpURLConnection.HTTP_BAD_REQUEST, String.format( + // @checkstyle MultipleStringLiteralsCheck (1 line) "Invalid HTTP Request-Line header: %s", line ) @@ -208,5 +201,46 @@ private Matcher matcher(final String line) } return matcher; } + + /** + * Validate Request-Line according to PATTERN. + * + * @param line Request-Line header + * @return Validated Request-Line header + * @throws HttpException If fails + */ + private String validated(final String line) throws HttpException { + if (!PATTERN.matcher(line).matches()) { + throw new HttpException( + HttpURLConnection.HTTP_BAD_REQUEST, + String.format( + // @checkstyle MultipleStringLiteralsCheck (1 line) + "Invalid HTTP Request-Line header: %s", + line + ) + ); + } + return line; + } + + /** + * Check that token value is not null and + * return trimmed value. + * + * @param value Token value + * @param token Token + * @return Trimmed token value + */ + private String trimmed(final String value, final Token token) { + if (value == null) { + throw new IllegalArgumentException( + String.format( + "There is no token %s in Request-Line header", + token.toString() + ) + ); + } + return value.trim(); + } } }