Fix default servlet character encoding directories (#9970)

#9966 adapt character encoding when including a path that is a directory listing

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
This commit is contained in:
Ludovic Orban 2023-06-26 22:09:17 +02:00 committed by GitHub
parent 939689b669
commit cbd83c91c2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 155 additions and 16 deletions

View File

@ -51,7 +51,7 @@ public class ResourceListing
* @param base The base URL
* @param parent True if the parent directory should be included
* @param query query params
* @return the HTML as String
* @return the XHTML as String
*/
public static String getAsXHTML(Resource resource, String base, boolean parent, String query)
{
@ -108,7 +108,8 @@ public class ResourceListing
StringBuilder buf = new StringBuilder(4096);
// Doctype Declaration + XHTML
// Doctype Declaration + XHTML. The spec says the encoding MUST be "utf-8" in all cases at it is ignored;
// see: https://html.spec.whatwg.org/multipage/semantics.html#attr-meta-charset
buf.append("""
<?xml version="1.0" encoding="utf-8"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">

View File

@ -16,6 +16,7 @@ package org.eclipse.jetty.server;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.ReadableByteChannel;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collection;
@ -626,8 +627,10 @@ public class ResourceService
return;
}
byte[] data = listing.getBytes(StandardCharsets.UTF_8);
response.getHeaders().put(HttpHeader.CONTENT_TYPE, "text/html;charset=utf-8");
String characterEncoding = httpContent.getCharacterEncoding();
Charset charset = characterEncoding == null ? StandardCharsets.UTF_8 : Charset.forName(characterEncoding);
byte[] data = listing.getBytes(charset);
response.getHeaders().put(HttpHeader.CONTENT_TYPE, "text/html;charset=" + charset.name());
response.getHeaders().put(HttpHeader.CONTENT_LENGTH, data.length);
response.write(true, ByteBuffer.wrap(data), callback);
}

View File

@ -490,7 +490,7 @@ public class DefaultServlet extends HttpServlet
else
{
ServletCoreRequest coreRequest = new ServletCoreRequest(req);
ServletCoreResponse coreResponse = new ServletCoreResponse(coreRequest, resp);
ServletCoreResponse coreResponse = new ServletCoreResponse(coreRequest, resp, included);
if (coreResponse.isCommitted())
{
@ -861,13 +861,34 @@ public class DefaultServlet extends HttpServlet
private final ServletCoreRequest _coreRequest;
private final Response _coreResponse;
private final HttpFields.Mutable _httpFields;
private final boolean _included;
public ServletCoreResponse(ServletCoreRequest coreRequest, HttpServletResponse response)
public ServletCoreResponse(ServletCoreRequest coreRequest, HttpServletResponse response, boolean included)
{
_coreRequest = coreRequest;
_response = response;
_coreResponse = ServletContextResponse.getServletContextResponse(response);
_httpFields = new HttpServletResponseHttpFields(response);
HttpFields.Mutable fields = new HttpServletResponseHttpFields(response);
if (included)
{
// If included, accept but ignore mutations.
fields = new HttpFields.Mutable.Wrapper(fields)
{
@Override
public HttpField onAddField(HttpField field)
{
return null;
}
@Override
public boolean onRemoveField(HttpField field)
{
return false;
}
};
}
_httpFields = fields;
_included = included;
}
@Override
@ -918,6 +939,8 @@ public class DefaultServlet extends HttpServlet
@Override
public void write(boolean last, ByteBuffer byteBuffer, Callback callback)
{
if (_included)
last = false;
try
{
if (BufferUtil.hasContent(byteBuffer))
@ -967,6 +990,8 @@ public class DefaultServlet extends HttpServlet
{
if (LOG.isDebugEnabled())
LOG.debug("{}.setStatus({})", this.getClass().getSimpleName(), code);
if (_included)
return;
_response.setStatus(code);
}
@ -1151,7 +1176,8 @@ public class DefaultServlet extends HttpServlet
HttpServletResponse response = getServletResponse(coreResponse);
try
{
// TODO: not sure if this is allowed here.
if (isIncluded(request))
return;
if (cause != null)
request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, cause);
response.sendError(statusCode, reason);
@ -1231,13 +1257,20 @@ public class DefaultServlet extends HttpServlet
public ForcedCharacterEncodingHttpContent(HttpContent content, String characterEncoding)
{
super(content);
super(Objects.requireNonNull(content));
this.characterEncoding = characterEncoding;
String mimeType = content.getContentTypeValue();
int idx = mimeType.indexOf(";charset");
if (idx >= 0)
mimeType = mimeType.substring(0, idx);
this.contentType = mimeType + ";charset=" + this.characterEncoding;
if (content.getContentTypeValue() == null || content.getResource().isDirectory())
{
this.contentType = null;
}
else
{
String mimeType = content.getContentTypeValue();
int idx = mimeType.indexOf(";charset");
if (idx >= 0)
mimeType = mimeType.substring(0, idx);
this.contentType = mimeType + ";charset=" + characterEncoding;
}
}
@Override

View File

@ -20,6 +20,7 @@ import java.io.InputStream;
import java.io.OutputStream;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
@ -62,6 +63,7 @@ import org.eclipse.jetty.toolchain.test.MavenPaths;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.StringUtil;
import org.junit.jupiter.api.AfterEach;
@ -377,6 +379,101 @@ public class DefaultServletTest
assertThat(body, containsString("f??r"));
}
@Test
public void testSimpleListing() throws Exception
{
ServletHolder defHolder = context.addServlet(DefaultServlet.class, "/*");
defHolder.setInitParameter("dirAllowed", "true");
String rawResponse = connector.getResponse("""
GET /context/ HTTP/1.1\r
Host: local\r
Connection: close\r
\r
""");
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat(response.getStatus(), is(200));
assertThat(response.getField("content-type").getValue(), is("text/html;charset=UTF-8"));
String body = response.getContent();
assertThat(body, containsString("<?xml version=\"1.0\" encoding=\"utf-8\"?>"));
}
@Test
public void testIncludeListingAllowed() throws Exception
{
ServletHolder defHolder = context.addServlet(DefaultServlet.class, "/*");
defHolder.setInitParameter("dirAllowed", "true");
/* create a file with a non-fully ASCII name in the docroot */
Files.writeString(docRoot.resolve("numéros-en-français.txt"), "un deux trois", StandardCharsets.ISO_8859_1);
ServletHolder incHolder = new ServletHolder();
incHolder.setInstance(new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
// Getting the writer implicitly sets the charset to iso-8859-1.
resp.getWriter().println(">>>");
resp.getWriter().println("éèàîû");
req.getRequestDispatcher("/").include(req, resp);
resp.getWriter().println("<<<");
}
});
context.addServlet(incHolder, "/inclusion");
String rawResponse = connector.getResponse("""
GET /context/inclusion HTTP/1.1\r
Host: local\r
Connection: close\r
\r
""");
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat(response.getStatus(), is(200));
String body = BufferUtil.toString(response.getContentByteBuffer(), StandardCharsets.ISO_8859_1);
assertThat(body, startsWith(">>>\néèàîû\n"));
assertThat(body, containsString("<?xml version=\"1.0\" encoding=\"utf-8\"?>"));
assertThat(body, containsString("numéros-en-français.txt"));
assertThat(body, endsWith("<<<\n"));
}
@Test
public void testIncludeListingForbidden() throws Exception
{
ServletHolder defHolder = context.addServlet(DefaultServlet.class, "/*");
defHolder.setInitParameter("dirAllowed", "false");
ServletHolder incHolder = new ServletHolder();
incHolder.setInstance(new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
{
// Getting the writer implicitly sets the charset to iso-8859-1.
resp.getWriter().println(">>>");
req.getRequestDispatcher("/").include(req, resp);
resp.getWriter().println("éèàîû");
resp.getWriter().println("<<<");
}
});
context.addServlet(incHolder, "/inclusion");
String rawResponse = connector.getResponse("""
GET /context/inclusion HTTP/1.1\r
Host: local\r
Connection: close\r
\r
""");
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat(response.getStatus(), is(200));
assertThat(response.get(HttpHeader.CONTENT_LENGTH), is("14"));
String body = BufferUtil.toString(response.getContentByteBuffer(), StandardCharsets.ISO_8859_1);
assertThat(body, is(">>>\néèàîû\n<<<\n"));
}
/**
* A regression on windows allowed the directory listing show
* the fully qualified paths within the directory listing.
@ -1178,7 +1275,11 @@ public class DefaultServletTest
*/
assertThat(response.toString(), response.getStatus(), is(HttpStatus.INTERNAL_SERVER_ERROR_500));
// This resource does exist but directory listings are not allowed and there are no welcome files.
/* This resource does exist but directory listings are not allowed and there are no welcome files;
* and since RequestDispatcher#include(ServletRequest, ServletResponse) says:
* "The included servlet cannot change the response status code or set headers; any attempt to make a change is ignored."
* the response status should be 200 and there should be no content.
*/
rawResponse = connector.getResponse("""
GET /context/gateway?includeTarget=/alt/ HTTP/1.1\r
Host: local\r
@ -1186,7 +1287,8 @@ public class DefaultServletTest
\r
""");
response = HttpTester.parseResponse(rawResponse);
assertThat(response.toString(), response.getStatus(), is(HttpStatus.FORBIDDEN_403));
assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200));
assertThat(response.toString(), containsString("Content-Length: 0"));
// Once index.html has been created we can include this same target and see it as a welcome file.
Files.writeString(altRoot.resolve("index.html"), "<h1>Alt Index</h1>", UTF_8);