Jetty 12.0.x 10716 charset printwriter (#10737)

* Issue #10716 tck compliance for setting response charset

---------

Co-authored-by: gregw <gregw@webtide.com>
This commit is contained in:
Jan Bartel 2023-10-19 05:09:42 +02:00 committed by GitHub
parent 524bde565e
commit 467f026d37
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 286 additions and 41 deletions

View File

@ -1602,6 +1602,11 @@ public interface HttpFields extends Iterable<HttpField>, Supplier<HttpFields>
return true; return true;
} }
public HttpField onReplaceField(HttpField oldField, HttpField newField)
{
return newField;
}
@Override @Override
public int size() public int size()
{ {
@ -1623,11 +1628,47 @@ public interface HttpFields extends Iterable<HttpField>, Supplier<HttpFields>
{ {
field = onAddField(field); field = onAddField(field);
if (field != null) if (field != null)
return _fields.add(field); _fields.add(field);
} }
return this; return this;
} }
@Override
public Mutable put(HttpField field)
{
// rewrite put to ensure that removes are called before replace
int put = -1;
ListIterator<HttpField> i = _fields.listIterator();
while (i.hasNext())
{
HttpField f = i.next();
if (f.isSameName(field))
{
if (put < 0)
put = i.previousIndex();
else if (onRemoveField(f))
i.remove();
}
}
if (put < 0)
{
field = onAddField(field);
if (field != null)
add(field);
}
else
{
i = _fields.listIterator(put);
HttpField old = i.next();
field = onReplaceField(old, field);
if (field != null)
i.set(field);
}
return this;
}
@Override @Override
public Mutable clear() public Mutable clear()
{ {
@ -1702,9 +1743,9 @@ public interface HttpFields extends Iterable<HttpField>, Supplier<HttpFields>
} }
else else
{ {
if (last != null && onRemoveField(last)) if (last != null)
{ {
field = onAddField(field); field = onReplaceField(last, field);
if (field != null) if (field != null)
{ {
last = null; last = null;

View File

@ -540,60 +540,67 @@ public class ServletContextResponse extends ContextResponse implements ServletCo
@Override @Override
public HttpField onAddField(HttpField field) public HttpField onAddField(HttpField field)
{ {
if (field.getHeader() != null) if (isCommitted())
{ return null;
switch (field.getHeader())
{
case CONTENT_LENGTH ->
{
if (!isCommitted())
{
return setContentLength(field);
}
}
case CONTENT_TYPE ->
{
if (!isCommitted())
{
return setContentType(field);
}
}
}
}
return super.onAddField(field); if (field.getHeader() == null)
return super.onAddField(field);
return switch (field.getHeader())
{
case CONTENT_LENGTH -> setContentLength(field);
case CONTENT_TYPE -> setContentType(field);
default -> super.onAddField(field);
};
} }
@Override @Override
public boolean onRemoveField(HttpField field) public boolean onRemoveField(HttpField field)
{ {
if (field.getHeader() != null) if (isCommitted())
return false;
if (field.getHeader() == null)
return true;
switch (field.getHeader())
{ {
switch (field.getHeader()) case CONTENT_LENGTH -> _contentLength = -1;
case CONTENT_TYPE ->
{ {
case CONTENT_LENGTH -> _contentType = null;
_mimeType = null;
if (!isWriting())
{ {
if (!isCommitted()) _characterEncoding = switch (_encodingFrom)
_contentLength = -1;
}
case CONTENT_TYPE ->
{
if (!isCommitted())
{ {
_contentType = null; case SET_CHARACTER_ENCODING, SET_LOCALE -> _characterEncoding;
_mimeType = null; default -> null;
_characterEncoding = switch (_encodingFrom) };
{
case SET_CHARACTER_ENCODING, SET_LOCALE -> _characterEncoding;
default -> null;
};
}
} }
} }
} }
return true; return true;
} }
@Override
public HttpField onReplaceField(HttpField oldField, HttpField newField)
{
assert oldField != null && newField != null;
if (isCommitted())
return null;
if (newField.getHeader() == null)
return newField;
return switch (newField.getHeader())
{
case CONTENT_LENGTH -> setContentLength(newField);
case CONTENT_TYPE -> setContentType(newField);
default -> newField;
};
}
private HttpField setContentLength(HttpField field) private HttpField setContentLength(HttpField field)
{ {
long len = field.getLongValue(); long len = field.getLongValue();

View File

@ -34,9 +34,11 @@ import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.nullValue;
public class ResponseHeadersTest public class ResponseHeadersTest
@ -444,6 +446,44 @@ public class ResponseHeadersTest
assertThat("Response Header X-Foo", response.getField("X-Foo").getIntValue(), is(foovalue)); assertThat("Response Header X-Foo", response.getField("X-Foo").getIntValue(), is(foovalue));
} }
@Test
public void testSetIntHeader() throws Exception
{
ServletContextHandler contextHandler = new ServletContextHandler();
contextHandler.setContextPath("/");
HttpServlet addHeaderServlet = new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
response.addIntHeader("X-Foo", 10);
response.setIntHeader("X-Foo", 20);
PrintWriter writer = response.getWriter();
writer.println("Done");
}
};
contextHandler.addServlet(addHeaderServlet, "/add-intheader/*");
startServer(contextHandler);
HttpTester.Request request = new HttpTester.Request();
request.setMethod("GET");
request.setURI("/add-intheader/");
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 X-Foo header should be present an unchanged
assertThat("Response Header X-Foo", response.getField("X-Foo").getIntValue(), is(20));
}
@Test @Test
public void testFlushPrintWriter() throws Exception public void testFlushPrintWriter() throws Exception
{ {
@ -480,4 +520,161 @@ public class ResponseHeadersTest
assertThat("Response Code", response.getStatus(), is(200)); assertThat("Response Code", response.getStatus(), is(200));
assertThat(response.getContent(), equalTo("Hello\nWorld\n")); assertThat(response.getContent(), equalTo("Hello\nWorld\n"));
} }
@Test
public void testContentTypeAfterWriterBeforeWrite() throws Exception
{
ServletContextHandler contextHandler = new ServletContextHandler();
contextHandler.setContextPath("/");
HttpServlet contentTypeServlet = new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
response.setContentType("text/xml;charset=ISO-8859-7");
PrintWriter pw = response.getWriter();
response.setContentType("text/html;charset=UTF-8");
PrintWriter writer = response.getWriter();
writer.println("Hello");
}
};
contextHandler.addServlet(contentTypeServlet, "/content/*");
startServer(contextHandler);
HttpTester.Request request = new HttpTester.Request();
request.setMethod("GET");
request.setURI("/content");
request.setVersion(HttpVersion.HTTP_1_1);
request.setHeader("Connection", "close");
request.setHeader("Host", "test");
ByteBuffer responseBuffer = connector.getResponse(request.generate());
HttpTester.Response response = HttpTester.parseResponse(responseBuffer);
assertThat("Response Code", response.getStatus(), is(200));
assertThat("Content Type", response.getField("Content-Type").getValue(), containsString("text/html;charset=ISO-8859-7"));
assertThat(response.getContent(), containsString("Hello"));
}
@Test
public void testContentTypeNoCharset() throws Exception
{
ServletContextHandler contextHandler = new ServletContextHandler();
contextHandler.setContextPath("/");
HttpServlet contentTypeServlet = new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
response.setContentType("text/html;charset=Shift_Jis");
response.setContentType("text/xml");
PrintWriter pw = response.getWriter();
pw.println("Hello");
}
};
contextHandler.addServlet(contentTypeServlet, "/content/*");
startServer(contextHandler);
HttpTester.Request request = new HttpTester.Request();
request.setMethod("GET");
request.setURI("/content");
request.setVersion(HttpVersion.HTTP_1_1);
request.setHeader("Connection", "close");
request.setHeader("Host", "test");
ByteBuffer responseBuffer = connector.getResponse(request.generate());
HttpTester.Response response = HttpTester.parseResponse(responseBuffer);
assertThat("Response Code", response.getStatus(), is(200));
assertThat("Content Type", response.getField("Content-Type").getValue(), containsString("text/xml;charset=Shift_Jis"));
assertThat(response.getContent(), containsString("Hello"));
}
@Test
public void testContentTypeNull() throws Exception
{
ServletContextHandler contextHandler = new ServletContextHandler();
contextHandler.setContextPath("/");
HttpServlet contentTypeServlet = new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
response.setContentType("text/html;charset=Shift_Jis");
response.setContentType(null);
PrintWriter pw = response.getWriter();
assertThat(response.getCharacterEncoding(), not(is("Shift_Jis")));
pw.println("Hello");
}
};
contextHandler.addServlet(contentTypeServlet, "/content/*");
startServer(contextHandler);
HttpTester.Request request = new HttpTester.Request();
request.setMethod("GET");
request.setURI("/content");
request.setVersion(HttpVersion.HTTP_1_1);
request.setHeader("Connection", "close");
request.setHeader("Host", "test");
ByteBuffer responseBuffer = connector.getResponse(request.generate());
HttpTester.Response response = HttpTester.parseResponse(responseBuffer);
assertThat("Response Code", response.getStatus(), is(200));
assertThat("Content Type", response.getField("Content-Type"), nullValue());
assertThat(response.getContent(), containsString("Hello"));
}
@Test
public void testCommittedNoop() throws Exception
{
ServletContextHandler contextHandler = new ServletContextHandler();
contextHandler.setContextPath("/");
HttpServlet addHeaderServlet = new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
response.setHeader("Test", "Before");
response.setHeader("Content-Length", "2");
response.setHeader("Content-Type", "text/html");
response.getOutputStream().print("OK");
response.flushBuffer();
// These should be silently ignored
response.setHeader("Test", "After");
response.setHeader("Content-Length", "10");
response.setHeader("Content-Type", "text/xml");
assertThat(response.getHeader("Test"), is("Before"));
assertThat(response.getContentType(), is("text/html"));
assertThat(response.getHeader("Content-Length"), is("2"));
}
};
contextHandler.addServlet(addHeaderServlet, "/test/*");
startServer(contextHandler);
HttpTester.Request request = new HttpTester.Request();
request.setMethod("GET");
request.setURI("/test/");
request.setVersion(HttpVersion.HTTP_1_1);
request.setHeader("Connection", "close");
request.setHeader("Host", "test");
ByteBuffer responseBuffer = connector.getResponse(request.generate());
HttpTester.Response response = HttpTester.parseResponse(responseBuffer);
assertThat(response.getStatus(), is(200));
assertThat(response.getField("Test").getValue(), is("Before"));
assertThat(response.getField("Content-Type").getValue(), is("text/html"));
assertThat(response.getContent(), is("OK"));
}
} }