diff --git a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/ResponsePatternRule.java b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/ResponsePatternRule.java index f0ab22576a6..62d4b4b221e 100644 --- a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/ResponsePatternRule.java +++ b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/ResponsePatternRule.java @@ -30,23 +30,23 @@ import org.eclipse.jetty.util.annotation.Name; */ public class ResponsePatternRule extends PatternRule { - private String _code; - private String _reason; + private int _code; + private String _message; /* ------------------------------------------------------------ */ public ResponsePatternRule() { - this(null,null,""); + this(null,null,null); } /* ------------------------------------------------------------ */ - public ResponsePatternRule(@Name("pattern") String pattern, @Name("code") String code, @Name("reason") String reason) + public ResponsePatternRule(@Name("pattern") String pattern, @Name("code") String code, @Name("message") String message) { super(pattern); _handling = true; _terminating = true; setCode(code); - setReason(reason); + setMessage(message); } /* ------------------------------------------------------------ */ @@ -56,19 +56,34 @@ public class ResponsePatternRule extends PatternRule */ public void setCode(String code) { - _code = code; + _code = code==null ? 0 : Integer.parseInt(code); } /* ------------------------------------------------------------ */ /** * Sets the reason for the response status code. Reasons will only reflect * if the code value is greater or equal to 400. - * + * @deprecated Reason has been replaced by message * @param reason the reason + * @see #setMessage(String) */ + @Deprecated public void setReason(String reason) { - _reason = reason; + setMessage(reason); + } + + /* ------------------------------------------------------------ */ + /** + * Sets the message for the {@link org.eclipse.jetty.server.Response#sendError(int, String)} method. + * Reasons will only reflect + * if the code value is greater or equal to 400. + * + * @param message the reason + */ + public void setMessage(String message) + { + _message = message; } /* ------------------------------------------------------------ */ @@ -79,16 +94,13 @@ public class ResponsePatternRule extends PatternRule @Override public String apply(String target, HttpServletRequest request, HttpServletResponse response) throws IOException { - int code = Integer.parseInt(_code); - // status code 400 and up are error codes - if (code >= 400) + if (_code>0) { - response.sendError(code, _reason); - } - else - { - response.setStatus(code); + if (_message != null && !_message.isEmpty()) + response.sendError(_code, _message); + else + response.setStatus(_code); } return target; } @@ -100,6 +112,6 @@ public class ResponsePatternRule extends PatternRule @Override public String toString() { - return super.toString()+"["+_code+","+_reason+"]"; + return super.toString()+"["+_code+","+ _message +"]"; } } diff --git a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/ValidUrlRule.java b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/ValidUrlRule.java index 650fe11012c..cff2080f697 100644 --- a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/ValidUrlRule.java +++ b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/ValidUrlRule.java @@ -42,7 +42,7 @@ public class ValidUrlRule extends Rule private static final Logger LOG = Log.getLogger(ValidUrlRule.class); String _code = "400"; - String _reason = "Illegal Url"; + String _message = "Illegal Url"; public ValidUrlRule() { @@ -64,13 +64,26 @@ public class ValidUrlRule extends Rule /* ------------------------------------------------------------ */ /** - * Sets the reason for the response status code. Reasons will only reflect if the code value is greater or equal to 400. + * Sets the reason for the response status code. * * @param reason the reason + * @deprecated use {@link #setMessage(String)} */ + @Deprecated public void setReason(String reason) { - _reason = reason; + _message = reason; + } + + /* ------------------------------------------------------------ */ + /** + * Sets the message for the {@link org.eclipse.jetty.server.Response#sendError(int, String)} method. + * + * @param message the message + */ + public void setMessage(String message) + { + _message = message; } @Override @@ -90,14 +103,10 @@ public class ValidUrlRule extends Rule int code = Integer.parseInt(_code); // status code 400 and up are error codes so include a reason - if (code >= 400) - { - response.sendError(code,_reason); - } + if (_message!=null && !_message.isEmpty()) + response.sendError(code, _message); else - { response.setStatus(code); - } // we have matched, return target and consider it is handled return target; @@ -121,6 +130,6 @@ public class ValidUrlRule extends Rule @Override public String toString() { - return super.toString() + "[" + _code + ":" + _reason + "]"; + return super.toString() + "[" + _code + ":" + _message + "]"; } } diff --git a/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/ResponsePatternRuleTest.java b/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/ResponsePatternRuleTest.java index 542a04d0434..29ad8c3d2cb 100644 --- a/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/ResponsePatternRuleTest.java +++ b/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/ResponsePatternRuleTest.java @@ -18,13 +18,15 @@ package org.eclipse.jetty.rewrite.handler; -import static org.junit.jupiter.api.Assertions.assertEquals; - import java.io.IOException; +import org.eclipse.jetty.server.Dispatcher; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + public class ResponsePatternRuleTest extends AbstractRuleTestCase { private ResponsePatternRule _rule; @@ -38,57 +40,27 @@ public class ResponsePatternRuleTest extends AbstractRuleTestCase } @Test - public void testStatusCodeNoReason() throws IOException + public void testStatusCodeNoMessage() throws IOException { - for (int i = 1; i < 400; i++) + for (int i = 1; i < 600; i++) { _rule.setCode("" + i); + _rule.setMessage(null); _rule.apply(null, _request, _response); assertEquals(i, _response.getStatus()); + assertNull(_request.getAttribute(Dispatcher.ERROR_MESSAGE)); } } @Test - public void testStatusCodeWithReason() throws IOException + public void testStatusCodeMessage() throws IOException { - for (int i = 1; i < 400; i++) - { - _rule.setCode("" + i); - _rule.setReason("reason" + i); - _rule.apply(null, _request, _response); + _rule.setCode("499"); + _rule.setMessage("Message 499"); + _rule.apply(null, _request, _response); - assertEquals(i, _response.getStatus()); - assertEquals(null, _response.getReason()); - } - } - - @Test - public void testErrorStatusNoReason() throws IOException - { - for (int i = 400; i < 600; i++) - { - _rule.setCode("" + i); - _rule.apply(null, _request, _response); - - assertEquals(i, _response.getStatus()); - assertEquals("", _response.getReason()); - super.reset(); - } - } - - @Test - public void testErrorStatusWithReason() throws IOException - { - for (int i = 400; i < 600; i++) - { - _rule.setCode("" + i); - _rule.setReason("reason-" + i); - _rule.apply(null, _request, _response); - - assertEquals(i, _response.getStatus()); - assertEquals("reason-" + i, _response.getReason()); - super.reset(); - } + assertEquals(499, _response.getStatus()); + assertEquals( "Message 499", _request.getAttribute(Dispatcher.ERROR_MESSAGE)); } } diff --git a/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/ValidUrlRuleTest.java b/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/ValidUrlRuleTest.java index 9b1855faf13..846194b15ba 100644 --- a/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/ValidUrlRuleTest.java +++ b/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/ValidUrlRuleTest.java @@ -18,14 +18,15 @@ package org.eclipse.jetty.rewrite.handler; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; - +import org.eclipse.jetty.server.Dispatcher; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + @SuppressWarnings("unused") public class ValidUrlRuleTest extends AbstractRuleTestCase { @@ -61,16 +62,16 @@ public class ValidUrlRuleTest extends AbstractRuleTestCase } @Test - public void testInvalidUrlWithReason() throws Exception + public void testInvalidUrlWithMessage() throws Exception { _rule.setCode("405"); - _rule.setReason("foo"); + _rule.setMessage("foo"); _request.setURIPathQuery("/%00/"); String result = _rule.matchAndApply(_request.getRequestURI(), _request, _response); assertEquals(405,_response.getStatus()); - assertEquals("foo",_response.getReason()); + assertEquals( "foo", _request.getAttribute(Dispatcher.ERROR_MESSAGE)); } @Test @@ -83,7 +84,7 @@ public class ValidUrlRuleTest extends AbstractRuleTestCase String result = _rule.matchAndApply(_request.getRequestURI(), _request, _response); assertEquals(405,_response.getStatus()); - assertEquals("foo",_response.getReason()); + assertEquals( "foo", _request.getAttribute(Dispatcher.ERROR_MESSAGE)); } @Disabled("Not working in jetty-9") @@ -97,7 +98,7 @@ public class ValidUrlRuleTest extends AbstractRuleTestCase String result = _rule.matchAndApply(_request.getRequestURI(), _request, _response); assertEquals(405,_response.getStatus()); - assertEquals("foo",_response.getReason()); + assertEquals( "foo", _request.getAttribute(Dispatcher.ERROR_MESSAGE)); } @Disabled("Not working in jetty-9") diff --git a/jetty-security/src/test/java/org/eclipse/jetty/security/SpecExampleConstraintTest.java b/jetty-security/src/test/java/org/eclipse/jetty/security/SpecExampleConstraintTest.java index 74e241334d9..e057058a83a 100644 --- a/jetty-security/src/test/java/org/eclipse/jetty/security/SpecExampleConstraintTest.java +++ b/jetty-security/src/test/java/org/eclipse/jetty/security/SpecExampleConstraintTest.java @@ -18,11 +18,6 @@ package org.eclipse.jetty.security; -import static org.hamcrest.Matchers.startsWith; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.io.IOException; import java.util.Arrays; import java.util.HashSet; @@ -48,6 +43,11 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + /** * @version $Revision: 1441 $ $Date: 2010-04-02 12:28:17 +0200 (Fri, 02 Apr 2010) $ */ @@ -323,7 +323,7 @@ public class SpecExampleConstraintTest response = _connector.getResponse("POST /ctx/acme/wholesale/index.html HTTP/1.0\r\n" + "Authorization: Basic " + B64Code.encode("chris:password") + "\r\n" + "\r\n"); - assertThat(response,startsWith("HTTP/1.1 403 !")); + assertThat(response,startsWith("HTTP/1.1 403 ")); //a user in role HOMEOWNER can do a GET response = _connector.getResponse("GET /ctx/acme/retail/index.html HTTP/1.0\r\n" + diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index 7cc8c604154..d4200db3ea4 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -633,13 +633,9 @@ public class Response implements HttpServletResponse Request request = _channel.getRequest(); Throwable cause = (Throwable)request.getAttribute(Dispatcher.ERROR_EXCEPTION); + _reason=HttpStatus.getMessage(code); if (message==null) - { - _reason=HttpStatus.getMessage(code); message=cause==null?_reason:cause.toString(); - } - else - _reason=message; // If we are allowed to have a body, then produce the error page. if (code != SC_NO_CONTENT && code != SC_NOT_MODIFIED && @@ -873,19 +869,19 @@ public class Response implements HttpServletResponse @Override @Deprecated - public void setStatus(int sc, String sm) + public void setStatus(int sc, String message) { - setStatusWithReason(sc,sm); + setStatusWithReason(sc, null); } - public void setStatusWithReason(int sc, String sm) + public void setStatusWithReason(int sc, String message) { if (sc <= 0) throw new IllegalArgumentException(); if (!isIncluding()) { _status = sc; - _reason = sm; + _reason = message; } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java index 84088146cd3..48d38feee41 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java @@ -139,7 +139,14 @@ public class ErrorHandler extends AbstractHandler if (_cacheControl != null) response.setHeader(HttpHeader.CACHE_CONTROL.asString(), _cacheControl); - generateAcceptableResponse(baseRequest,request,response,response.getStatus(),baseRequest.getResponse().getReason()); + + String message = (String)request.getAttribute(RequestDispatcher.ERROR_MESSAGE); + if (message==null) + message = baseRequest.getResponse().getReason(); + if (message==null) + message = HttpStatus.getMessage(response.getStatus()); + + generateAcceptableResponse(baseRequest,request,response,response.getStatus(),message); } /** diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java index 812519b8210..38e7ac1025d 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java @@ -18,21 +18,6 @@ package org.eclipse.jetty.server; -import static java.nio.charset.StandardCharsets.UTF_8; -import static org.hamcrest.CoreMatchers.allOf; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.nullValue; -import static org.hamcrest.Matchers.startsWith; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.io.IOException; import java.io.InputStreamReader; import java.io.LineNumberReader; @@ -62,6 +47,7 @@ import org.eclipse.jetty.http.CookieCompliance; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; @@ -86,6 +72,21 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.hamcrest.CoreMatchers.allOf; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + public class ResponseTest { @@ -676,7 +677,9 @@ public class ResponseTest response.sendError(500, "Database Error"); assertEquals(500, response.getStatus()); - assertEquals("Database Error", response.getReason()); + assertEquals("Server Error", response.getReason()); + assertThat(BufferUtil.toString(_content), containsString("Database Error")); + assertEquals("must-revalidate,no-cache,no-store", response.getHeader(HttpHeader.CACHE_CONTROL.asString())); response = getResponse(); @@ -689,7 +692,8 @@ public class ResponseTest response.sendError(406, "Super Nanny"); assertEquals(406, response.getStatus()); - assertEquals("Super Nanny", response.getReason()); + assertEquals(HttpStatus.Code.NOT_ACCEPTABLE.getMessage(), response.getReason()); + assertThat(BufferUtil.toString(_content), containsString("Super Nanny")); assertEquals("must-revalidate,no-cache,no-store", response.getHeader(HttpHeader.CACHE_CONTROL.asString())); } @@ -707,7 +711,8 @@ public class ResponseTest response.sendError(500, "Database Error"); assertEquals(500, response.getStatus()); - assertEquals("Database Error", response.getReason()); + assertEquals("Server Error", response.getReason()); + assertThat(BufferUtil.toString(_content), is("")); assertThat(response.getHeader(HttpHeader.CACHE_CONTROL.asString()),Matchers.nullValue()); response = getResponse(); @@ -720,7 +725,8 @@ public class ResponseTest response.sendError(406, "Super Nanny"); assertEquals(406, response.getStatus()); - assertEquals("Super Nanny", response.getReason()); + assertEquals(HttpStatus.Code.NOT_ACCEPTABLE.getMessage(), response.getReason()); + assertThat(BufferUtil.toString(_content), is("")); assertThat(response.getHeader(HttpHeader.CACHE_CONTROL.asString()),Matchers.nullValue()); } @@ -1384,6 +1390,7 @@ public class ResponseTest { _channel.recycle(); _channel.getRequest().setMetaData(new MetaData.Request("GET",new HttpURI("/path/info"),HttpVersion.HTTP_1_0,new HttpFields())); + BufferUtil.clear(_content); return _channel.getResponse(); } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SniSslConnectionFactoryTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SniSslConnectionFactoryTest.java index 7b39695b5a4..ec912540f29 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SniSslConnectionFactoryTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SniSslConnectionFactoryTest.java @@ -18,12 +18,6 @@ package org.eclipse.jetty.server.ssl; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.startsWith; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; @@ -70,6 +64,12 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + public class SniSslConnectionFactoryTest { private Server _server; @@ -268,8 +268,9 @@ public class SniSslConnectionFactoryTest output.flush(); response = response(input); + String body = IO.toString(input); assertThat(response,startsWith("HTTP/1.1 400 ")); - assertThat(response, containsString("Host does not match SNI")); + assertThat(body, containsString("Host does not match SNI")); } finally { @@ -326,7 +327,8 @@ public class SniSslConnectionFactoryTest response = response(input); assertTrue(response.startsWith("HTTP/1.1 400 ")); - assertThat(response, Matchers.containsString("Host does not match SNI")); + String body = IO.toString(input); + assertThat(body, Matchers.containsString("Host does not match SNI")); } finally { diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ErrorPageTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ErrorPageTest.java index e6414d7b561..8a881c339a2 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ErrorPageTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ErrorPageTest.java @@ -18,10 +18,6 @@ package org.eclipse.jetty.servlet; -import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.CoreMatchers.not; -import static org.hamcrest.MatcherAssert.assertThat; - import java.io.IOException; import java.io.PrintWriter; @@ -42,6 +38,10 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.MatcherAssert.assertThat; + public class ErrorPageTest { private Server _server; @@ -168,7 +168,7 @@ public class ErrorPageTest try (StacklessLogging ignore = new StacklessLogging(Dispatcher.class)) { String response = _connector.getResponse("GET /app?baa=%88%A4 HTTP/1.0\r\n\r\n"); - assertThat(response, Matchers.containsString("HTTP/1.1 400 Bad query encoding")); + assertThat(response, Matchers.containsString("HTTP/1.1 400 ")); assertThat(response, Matchers.containsString("ERROR_PAGE: /BadMessageException")); assertThat(response, Matchers.containsString("ERROR_MESSAGE: Bad query encoding")); assertThat(response, Matchers.containsString("ERROR_CODE: 400"));