Skip to content

Commit

Permalink
Issue #4719 Keep previous charencoding if not set in content type (#4720
Browse files Browse the repository at this point in the history 
)

* Issue #4719 Keep previous charencoding if not set in content type

Signed-off-by: Jan Bartel <janb@webtide.com>

* Issue #4719 - Adding unit tests for charset reset/change

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
  • Loading branch information
@janbartel
janbartel authored Mar 31, 2020
1 parent 4837fe1 commit 3f7d04f
Show file tree
Hide file tree
Showing 4 changed files with 188 additions and 10 deletions.
16 changes: 7 additions & 9 deletions jetty-server/src/main/java/org/eclipse/jetty/server/Response.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1017,11 +1017,9 @@ public void setContentType(String contentType)
_contentType = contentType;
_mimeType = MimeTypes.CACHE.get(contentType);

String charset;
if (_mimeType != null && _mimeType.getCharset() != null && !_mimeType.isCharsetAssumed())
String charset = MimeTypes.getCharsetFromContentType(contentType);
if (charset == null && _mimeType != null && _mimeType.isCharsetAssumed())
charset = _mimeType.getCharsetString();
else
charset = MimeTypes.getCharsetFromContentType(contentType);

if (charset == null)
{
Expand All @@ -1030,23 +1028,23 @@ public void setContentType(String contentType)
case NOT_SET:
break;
case INFERRED:
case SET_CONTENT_TYPE:
if (isWriting())
{
_mimeType = null;
_contentType = _contentType + ";charset=" + _characterEncoding;
_mimeType = MimeTypes.CACHE.get(_contentType);
}
else
{
_encodingFrom = EncodingFrom.NOT_SET;
_characterEncoding = null;
}
break;
case SET_CONTENT_TYPE:
case SET_LOCALE:
case SET_CHARACTER_ENCODING:
{
_contentType = contentType + ";charset=" + _characterEncoding;
_mimeType = null;
_mimeType = MimeTypes.CACHE.get(_contentType);
break;
}
default:
Expand All @@ -1056,10 +1054,10 @@ public void setContentType(String contentType)
else if (isWriting() && !charset.equalsIgnoreCase(_characterEncoding))
{
// too late to change the character encoding;
_mimeType = null;
_contentType = MimeTypes.getContentTypeWithoutCharset(_contentType);
if (_characterEncoding != null)
if (_characterEncoding != null && (_mimeType == null || !_mimeType.isCharsetAssumed()))
_contentType = _contentType + ";charset=" + _characterEncoding;
_mimeType = MimeTypes.CACHE.get(_contentType);
}
else
{
Expand Down
13 changes: 12 additions & 1 deletion jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,17 @@ public void testContentTypeCharacterEncoding() throws Exception
response.setCharacterEncoding("ISO-8859-1");
assertEquals("text/xml;charset=utf-8", response.getContentType());
}

@Test
public void testContentEncodingViaContentTypeChange() throws Exception
{
Response response = getResponse();
response.setContentType("text/html;charset=Shift_Jis");
assertEquals("Shift_Jis", response.getCharacterEncoding());

response.setContentType("text/xml");
assertEquals("Shift_Jis", response.getCharacterEncoding());
}

@Test
public void testCharacterEncodingContentType() throws Exception
Expand Down Expand Up @@ -624,7 +635,7 @@ public void testResetContentTypeWithCharacterEncoding() throws Exception

response.setContentType("wrong/answer;charset=utf-8");
response.setContentType("foo/bar");
assertEquals("foo/bar", response.getContentType());
assertEquals("foo/bar;charset=utf-8", response.getContentType());
response.setContentType("wrong/answer;charset=utf-8");
response.getWriter();
response.setContentType("foo2/bar2;charset=utf-16");
Expand Down
46 changes: 46 additions & 0 deletions jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ErrorPageTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
import javax.servlet.http.HttpServletResponse;

import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.tools.HttpTester;
import org.eclipse.jetty.logging.StacklessLogging;
import org.eclipse.jetty.server.Dispatcher;
import org.eclipse.jetty.server.HttpChannel;
Expand All @@ -60,6 +62,7 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -101,6 +104,7 @@ public void init() throws Exception
_context.addServlet(UnavailableServlet.class, "/unavailable/*");
_context.addServlet(DeleteServlet.class, "/delete/*");
_context.addServlet(ErrorAndStatusServlet.class, "/error-and-status/*");
_context.addServlet(ErrorContentTypeCharsetWriterInitializedServlet.class, "/error-mime-charset-writer/*");

HandlerWrapper noopHandler = new HandlerWrapper()
{
Expand Down Expand Up @@ -140,6 +144,36 @@ public void destroy() throws Exception
_server.join();
}

@Test
public void testErrorOverridesMimeTypeAndCharset() throws Exception
{
StringBuilder rawRequest = new StringBuilder();
rawRequest.append("GET /error-mime-charset-writer/ HTTP/1.1\r\n");
rawRequest.append("Host: test\r\n");
rawRequest.append("Connection: close\r\n");
rawRequest.append("Accept: */*\r\n");
rawRequest.append("Accept-Charset: *\r\n");
rawRequest.append("\r\n");

String rawResponse = _connector.getResponse(rawRequest.toString());
System.out.println(rawResponse);
HttpTester.Response response = HttpTester.parseResponse(rawResponse);

assertThat(response.getStatus(), is(595));
String actualContentType = response.get(HttpHeader.CONTENT_TYPE);
// should not expect to see charset line from servlet
assertThat(actualContentType, not(containsString("charset=US-ASCII")));
String body = response.getContent();

assertThat(body, containsString("ERROR_PAGE: /595"));
assertThat(body, containsString("ERROR_MESSAGE: 595"));
assertThat(body, containsString("ERROR_CODE: 595"));
assertThat(body, containsString("ERROR_EXCEPTION: null"));
assertThat(body, containsString("ERROR_EXCEPTION_TYPE: null"));
assertThat(body, containsString("ERROR_SERVLET: org.eclipse.jetty.servlet.ErrorPageTest$ErrorContentTypeCharsetWriterInitializedServlet-"));
assertThat(body, containsString("ERROR_REQUEST_URI: /error-mime-charset-writer/"));
}

@Test
public void testErrorOverridesStatus() throws Exception
{
Expand Down Expand Up @@ -612,6 +646,18 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t
}
}

public static class ErrorContentTypeCharsetWriterInitializedServlet extends HttpServlet
{
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
response.setContentType("text/html; charset=US-ASCII");
PrintWriter writer = response.getWriter();
writer.println("Intentionally using sendError(595)");
response.sendError(595);
}
}

public static class ErrorAndStatusServlet extends HttpServlet implements Servlet
{
@Override
Expand Down
123 changes: 123 additions & 0 deletions jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ResponseHeadersTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.eclipse.jetty.servlet;

import java.io.IOException;
import java.io.PrintWriter;
import java.net.URLDecoder;
import java.nio.ByteBuffer;
import javax.servlet.ServletException;
Expand Down Expand Up @@ -73,6 +74,65 @@ protected void doGet(HttpServletRequest req, HttpServletResponse response) throw
}
}

public static class CharsetResetToJsonMimeTypeServlet extends HttpServlet
{
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
// We set an initial desired behavior.
response.setContentType("text/html; charset=US-ASCII");
PrintWriter writer = response.getWriter();

// We reset the response, as we don't want it to be HTML anymore.
response.reset();

// switch to json operation
// The use of application/json is always assumed to be UTF-8
// and should never have a `charset=` entry on the `Content-Type` response header
response.setContentType("application/json");
writer.println("{ \"what\": \"should this be?\" }");
}
}

public static class CharsetChangeToJsonMimeTypeServlet extends HttpServlet
{
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
// We set an initial desired behavior.
response.setContentType("text/html; charset=US-ASCII");

// switch to json behavior.
// The use of application/json is always assumed to be UTF-8
// and should never have a `charset=` entry on the `Content-Type` response header
response.setContentType("application/json");

PrintWriter writer = response.getWriter();
writer.println("{ \"what\": \"should this be?\" }");
}
}

public static class CharsetChangeToJsonMimeTypeSetCharsetToNullServlet extends HttpServlet
{
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
// We set an initial desired behavior.

response.setContentType("text/html; charset=us-ascii");
PrintWriter writer = response.getWriter();

// switch to json behavior.
// The use of application/json is always assumed to be UTF-8
// and should never have a `charset=` entry on the `Content-Type` response header
response.setContentType("application/json");
// attempt to indicate that there is truly no charset meant to be used in the response header
response.setCharacterEncoding(null);

writer.println("{ \"what\": \"should this be?\" }");
}
}

private static Server server;
private static LocalConnector connector;

Expand All @@ -89,6 +149,9 @@ public static void startServer() throws Exception

context.addServlet(new ServletHolder(new SimulateUpgradeServlet()), "/ws/*");
context.addServlet(new ServletHolder(new MultilineResponseValueServlet()), "/multiline/*");
context.addServlet(CharsetResetToJsonMimeTypeServlet.class, "/charset/json-reset/*");
context.addServlet(CharsetChangeToJsonMimeTypeServlet.class, "/charset/json-change/*");
context.addServlet(CharsetChangeToJsonMimeTypeSetCharsetToNullServlet.class, "/charset/json-change-null/*");

server.start();
}
Expand Down Expand Up @@ -149,4 +212,64 @@ public void testMultilineResponseHeaderValue() throws Exception
expected = expected.trim(); // trim whitespace at start/end
assertThat("Response Header X-example", response.get("X-Example"), is(expected));
}

@Test
public void testCharsetResetToJsonMimeType() throws Exception
{
HttpTester.Request request = new HttpTester.Request();
request.setMethod("GET");
request.setURI("/charset/json-reset/");
request.setVersion(HttpVersion.HTTP_1_1);
request.setHeader("Connection", "close");
request.setHeader("Host", "test");

ByteBuffer responseBuffer = connector.getResponse(request.generate());
// System.err.println(BufferUtil.toUTF8String(responseBuffer));
HttpTester.Response response = HttpTester.parseResponse(responseBuffer);

// Now test for properly formatted HTTP Response Headers.
assertThat("Response Code", response.getStatus(), is(200));
// The Content-Type should not have a charset= portion
assertThat("Response Header Content-Type", response.get("Content-Type"), is("application/json"));
}

@Test
public void testCharsetChangeToJsonMimeType() throws Exception
{
HttpTester.Request request = new HttpTester.Request();
request.setMethod("GET");
request.setURI("/charset/json-change/");
request.setVersion(HttpVersion.HTTP_1_1);
request.setHeader("Connection", "close");
request.setHeader("Host", "test");

ByteBuffer responseBuffer = connector.getResponse(request.generate());
// System.err.println(BufferUtil.toUTF8String(responseBuffer));
HttpTester.Response response = HttpTester.parseResponse(responseBuffer);

// Now test for properly formatted HTTP Response Headers.
assertThat("Response Code", response.getStatus(), is(200));
// The Content-Type should not have a charset= portion
assertThat("Response Header Content-Type", response.get("Content-Type"), is("application/json"));
}

@Test
public void testCharsetChangeToJsonMimeTypeSetCharsetToNull() throws Exception
{
HttpTester.Request request = new HttpTester.Request();
request.setMethod("GET");
request.setURI("/charset/json-change-null/");
request.setVersion(HttpVersion.HTTP_1_1);
request.setHeader("Connection", "close");
request.setHeader("Host", "test");

ByteBuffer responseBuffer = connector.getResponse(request.generate());
// System.err.println(BufferUtil.toUTF8String(responseBuffer));
HttpTester.Response response = HttpTester.parseResponse(responseBuffer);

// Now test for properly formatted HTTP Response Headers.
assertThat("Response Code", response.getStatus(), is(200));
// The Content-Type should not have a charset= portion
assertThat("Response Header Content-Type", response.get("Content-Type"), is("application/json"));
}
}

0 comments on commit 3f7d04f

Please sign in to comment.