diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java index d380d1775c0..f83d477dd10 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java @@ -400,8 +400,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor { if (!_response.isCommitted() && !_request.isHandled()) _response.sendError(HttpStatus.NOT_FOUND_404); - else - _response.closeOutput(); + _response.closeOutput(); _request.setHandled(true); _state.onComplete(); 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 018c5f201db..145026623d4 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 @@ -23,6 +23,9 @@ import java.io.PrintWriter; import java.io.StringWriter; import java.io.Writer; import java.nio.ByteBuffer; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.util.List; import javax.servlet.ServletContext; import javax.servlet.RequestDispatcher; @@ -30,7 +33,6 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpMethod; @@ -41,7 +43,6 @@ import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.BufferUtil; -import org.eclipse.jetty.util.ByteArrayISO8859Writer; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -130,29 +131,120 @@ public class ErrorHandler extends AbstractHandler } } } - - - baseRequest.setHandled(true); - // Issue #124 - Don't produce text/html if the request doesn't accept it - HttpField accept = baseRequest.getHttpFields().getField(HttpHeader.ACCEPT); - if (accept == null || accept.contains("text/html") || accept.contains("*/*")) - { - response.setContentType(MimeTypes.Type.TEXT_HTML_8859_1.asString()); - if (_cacheControl != null) - response.setHeader(HttpHeader.CACHE_CONTROL.asString(), _cacheControl); - ByteArrayISO8859Writer writer = new ByteArrayISO8859Writer(4096); - String reason = (response instanceof Response) ? ((Response) response).getReason() : null; - if (LOG.isDebugEnabled()) - LOG.debug("default error page {} {}",request,response); - handleErrorPage(request, writer, response.getStatus(), reason); - writer.flush(); - response.setContentLength(writer.size()); - writer.writeTo(response.getOutputStream()); - writer.destroy(); - } + if (_cacheControl != null) + response.setHeader(HttpHeader.CACHE_CONTROL.asString(), _cacheControl); + generateAcceptableResponse(baseRequest,request,response,response.getStatus(),baseRequest.getResponse().getReason()); } + /* ------------------------------------------------------------ */ + /** Generate an acceptable error response. + *

This method is called to generate an Error page of a mime type that is + * acceptable to the user-agent. The Accept header is evaluated in + * quality order and the method + * {@link #generateAcceptableResponse(Request, HttpServletRequest, HttpServletResponse, String)} + * is called for each mimetype until {@link Request#isHandled()} is true.

+ * @param baseRequest The base request + * @param request The servlet request (may be wrapped) + * @param response The response (may be wrapped) + * @throws IOException + */ + protected void generateAcceptableResponse(Request baseRequest, HttpServletRequest request, HttpServletResponse response, int code, String message) + throws IOException + { + List acceptable=baseRequest.getHttpFields().getQualityCSV(HttpHeader.ACCEPT); + + if (acceptable.isEmpty() && !baseRequest.getHttpFields().contains(HttpHeader.ACCEPT)) + generateAcceptableResponse(baseRequest,request,response,code,message,MimeTypes.Type.TEXT_HTML.asString()); + else + { + for (String mimeType:acceptable) + { + generateAcceptableResponse(baseRequest,request,response,code,message,mimeType); + if (baseRequest.isHandled()) + return; + } + } + baseRequest.setHandled(true); + } + + /* ------------------------------------------------------------ */ + /** get an acceptable writer for an error page. + *

Uses the user-agent's Accept-Charset to get response + * {@link Writer}. The acceptable charsets are tested in quality order + * if they are known to the JVM and the first known is set on + * {@link HttpServletResponse#setCharacterEncoding(String)} and the + * {@link HttpServletResponse#getWriter()} method used to return a writer. + * If there is no Accept-Charset header then + * ISO-8859-1 is used. If '*' is the highest quality known + * charset, then utf-8 is used. + *

+ * @param baseRequest The base request + * @param request The servlet request (may be wrapped) + * @param response The response (may be wrapped) + * @return A {@link Writer} if there is a known acceptable charset or null + * @throws IOException + */ + protected Writer getAcceptableWriter(Request baseRequest, HttpServletRequest request, HttpServletResponse response) + throws IOException + { + List acceptable=baseRequest.getHttpFields().getQualityCSV(HttpHeader.ACCEPT_CHARSET); + if (acceptable.isEmpty()) + { + response.setCharacterEncoding(StandardCharsets.ISO_8859_1.name()); + return response.getWriter(); + } + + for (String charset:acceptable) + { + try + { + if ("*".equals(charset)) + response.setCharacterEncoding(StandardCharsets.UTF_8.name()); + else + response.setCharacterEncoding(Charset.forName(charset).name()); + return response.getWriter(); + } + catch(Exception e) + { + LOG.ignore(e); + } + } + return null; + } + + /* ------------------------------------------------------------ */ + /** Generate an acceptable error response for a mime type. + *

This method is called for each mime type in the users agent's + * Accept header, until {@link Request#isHandled()} is true and a + * response of the appropriate type is generated. + * + * @param baseRequest The base request + * @param request The servlet request (may be wrapped) + * @param response The response (may be wrapped) + * @param mimeType The mimetype to generate (may be */*or other wildcard) + * @throws IOException + */ + protected void generateAcceptableResponse(Request baseRequest, HttpServletRequest request, HttpServletResponse response, int code, String message, String mimeType) + throws IOException + { + switch(mimeType) + { + case "text/html": + case "text/*": + case "*/*": + { + baseRequest.setHandled(true); + Writer writer = getAcceptableWriter(baseRequest,request,response); + if (writer!=null) + { + response.setContentType(MimeTypes.Type.TEXT_HTML.asString()); + handleErrorPage(request, writer, code, message); + } + } + } + } + /* ------------------------------------------------------------ */ protected void handleErrorPage(HttpServletRequest request, Writer writer, int code, String message) throws IOException diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java new file mode 100644 index 00000000000..5040abbdc5c --- /dev/null +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java @@ -0,0 +1,260 @@ +// +// ======================================================================== +// Copyright (c) 1995-2016 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.server; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.Assert.*; + +import java.io.IOException; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.server.handler.ErrorHandler; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +public class ErrorHandlerTest +{ + Server server; + LocalConnector connector; + + @Before + public void before() throws Exception + { + server = new Server(); + connector = new LocalConnector(server); + server.addConnector(connector); + server.addBean(new ErrorHandler() + { + @Override + protected void generateAcceptableResponse( + Request baseRequest, + HttpServletRequest request, + HttpServletResponse response, + int code, + String message, + String mimeType) throws IOException + { + switch(mimeType) + { + case "text/json": + case "application/json": + { + baseRequest.setHandled(true); + response.setContentType(mimeType); + response.getWriter() + .append("{") + .append("code: \"").append(Integer.toString(code)).append("\",") + .append("message: \"").append(message).append('"') + .append("}"); + break; + } + default: + super.generateAcceptableResponse(baseRequest,request,response,code,message,mimeType); + } + } + + }); + server.start(); + } + + @After + public void after() throws Exception + { + server.stop(); + } + + @Test + public void test404NoAccept() throws Exception + { + String response = connector.getResponse( + "GET / HTTP/1.1\r\n"+ + "Host: Localhost\r\n"+ + "\r\n"); + + assertThat(response,startsWith("HTTP/1.1 404 ")); + assertThat(response,not(containsString("Content-Length: 0"))); + assertThat(response,containsString("Content-Type: text/html;charset=iso-8859-1")); + } + + @Test + public void test404EmptyAccept() throws Exception + { + String response = connector.getResponse( + "GET / HTTP/1.1\r\n"+ + "Accept: \r\n"+ + "Host: Localhost\r\n"+ + "\r\n"); + assertThat(response,startsWith("HTTP/1.1 404 ")); + assertThat(response,containsString("Content-Length: 0")); + assertThat(response,not(containsString("Content-Type"))); + } + + @Test + public void test404UnAccept() throws Exception + { + String response = connector.getResponse( + "GET / HTTP/1.1\r\n"+ + "Accept: text/*;q=0\r\n"+ + "Host: Localhost\r\n"+ + "\r\n"); + + assertThat(response,startsWith("HTTP/1.1 404 ")); + assertThat(response,containsString("Content-Length: 0")); + assertThat(response,not(containsString("Content-Type"))); + } + + @Test + public void test404AllAccept() throws Exception + { + String response = connector.getResponse( + "GET / HTTP/1.1\r\n"+ + "Host: Localhost\r\n"+ + "Accept: */*\r\n"+ + "\r\n"); + + assertThat(response,startsWith("HTTP/1.1 404 ")); + assertThat(response,not(containsString("Content-Length: 0"))); + assertThat(response,containsString("Content-Type: text/html;charset=iso-8859-1")); + } + + @Test + public void test404HtmlAccept() throws Exception + { + String response = connector.getResponse( + "GET / HTTP/1.1\r\n"+ + "Host: Localhost\r\n"+ + "Accept: text/html\r\n"+ + "\r\n"); + + assertThat(response,startsWith("HTTP/1.1 404 ")); + assertThat(response,not(containsString("Content-Length: 0"))); + assertThat(response,containsString("Content-Type: text/html;charset=iso-8859-1")); + } + + @Test + public void test404HtmlAcceptAnyCharset() throws Exception + { + String response = connector.getResponse( + "GET / HTTP/1.1\r\n"+ + "Host: Localhost\r\n"+ + "Accept: text/html\r\n"+ + "Accept-Charset: *\r\n"+ + "\r\n"); + + assertThat(response,startsWith("HTTP/1.1 404 ")); + assertThat(response,not(containsString("Content-Length: 0"))); + assertThat(response,containsString("Content-Type: text/html;charset=utf-8")); + } + + @Test + public void test404HtmlAcceptUtf8Charset() throws Exception + { + String response = connector.getResponse( + "GET / HTTP/1.1\r\n"+ + "Host: Localhost\r\n"+ + "Accept: text/html\r\n"+ + "Accept-Charset: utf-8\r\n"+ + "\r\n"); + + assertThat(response,startsWith("HTTP/1.1 404 ")); + assertThat(response,not(containsString("Content-Length: 0"))); + assertThat(response,containsString("Content-Type: text/html;charset=utf-8")); + } + + @Test + public void test404HtmlAcceptNotUtf8Charset() throws Exception + { + String response = connector.getResponse( + "GET / HTTP/1.1\r\n"+ + "Host: Localhost\r\n"+ + "Accept: text/html\r\n"+ + "Accept-Charset: utf-8;q=0\r\n"+ + "\r\n"); + + assertThat(response,startsWith("HTTP/1.1 404 ")); + assertThat(response,not(containsString("Content-Length: 0"))); + assertThat(response,containsString("Content-Type: text/html;charset=iso-8859-1")); + } + + @Test + public void test404HtmlAcceptNotUtf8UnknownCharset() throws Exception + { + String response = connector.getResponse( + "GET / HTTP/1.1\r\n"+ + "Host: Localhost\r\n"+ + "Accept: text/html\r\n"+ + "Accept-Charset: utf-8;q=0,unknown\r\n"+ + "\r\n"); + + assertThat(response,startsWith("HTTP/1.1 404 ")); + assertThat(response,containsString("Content-Length: 0")); + assertThat(response,not(containsString("Content-Type"))); + } + + @Test + public void test404HtmlAcceptUnknownUtf8Charset() throws Exception + { + String response = connector.getResponse( + "GET / HTTP/1.1\r\n"+ + "Host: Localhost\r\n"+ + "Accept: text/html\r\n"+ + "Accept-Charset: utf-8;q=0.1,unknown\r\n"+ + "\r\n"); + + assertThat(response,startsWith("HTTP/1.1 404 ")); + assertThat(response,not(containsString("Content-Length: 0"))); + assertThat(response,containsString("Content-Type: text/html;charset=utf-8")); + } + + @Test + public void test404PreferHtml() throws Exception + { + String response = connector.getResponse( + "GET / HTTP/1.1\r\n"+ + "Host: Localhost\r\n"+ + "Accept: text/html;q=1.0,text/json;q=0.5,*/*\r\n"+ + "Accept-Charset: *\r\n"+ + "\r\n"); + + assertThat(response,startsWith("HTTP/1.1 404 ")); + assertThat(response,not(containsString("Content-Length: 0"))); + assertThat(response,containsString("Content-Type: text/html;charset=utf-8")); + } + + @Test + public void test404PreferJson() throws Exception + { + String response = connector.getResponse( + "GET / HTTP/1.1\r\n"+ + "Host: Localhost\r\n"+ + "Accept: text/html;q=0.5,text/json;q=1.0,*/*\r\n"+ + "Accept-Charset: *\r\n"+ + "\r\n"); + + assertThat(response,startsWith("HTTP/1.1 404 ")); + assertThat(response,not(containsString("Content-Length: 0"))); + assertThat(response,containsString("Content-Type: text/json")); + } + +}