From 619658cb04f555ef926c873ad2269ccc3633dd24 Mon Sep 17 00:00:00 2001 From: Steve Ikeoka Date: Fri, 6 Oct 2023 13:11:46 -0700 Subject: [PATCH] [GEOS-11148] Update response headers for the Resources REST API --- .../rest/resources/ResourceController.java | 46 +++++++++---------- .../resources/ResourceControllerTest.java | 25 +++++++--- 2 files changed, 41 insertions(+), 30 deletions(-) diff --git a/src/restconfig/src/main/java/org/geoserver/rest/resources/ResourceController.java b/src/restconfig/src/main/java/org/geoserver/rest/resources/ResourceController.java index b8976974344..7c91a12eb61 100644 --- a/src/restconfig/src/main/java/org/geoserver/rest/resources/ResourceController.java +++ b/src/restconfig/src/main/java/org/geoserver/rest/resources/ResourceController.java @@ -9,11 +9,8 @@ import com.thoughtworks.xstream.XStream; import com.thoughtworks.xstream.annotations.XStreamAlias; import freemarker.template.ObjectWrapper; -import java.io.BufferedInputStream; import java.io.IOException; -import java.io.InputStream; import java.io.UnsupportedEncodingException; -import java.net.URLConnection; import java.net.URLDecoder; import java.text.DateFormat; import java.text.SimpleDateFormat; @@ -23,6 +20,7 @@ import java.util.Date; import java.util.List; import java.util.Locale; +import java.util.Optional; import java.util.TimeZone; import java.util.logging.Level; import java.util.logging.Logger; @@ -53,6 +51,7 @@ import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.http.ContentDisposition; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; @@ -129,24 +128,18 @@ protected String getTemplateName(Object object) { * @return Content type requested */ protected static MediaType getMediaType(Resource resource, HttpServletRequest request) { - if (resource.getType() == Resource.Type.DIRECTORY) { - return getFormat(request); - } else if (resource.getType() == Resource.Type.RESOURCE) { - String mimeType = URLConnection.guessContentTypeFromName(resource.name()); - if (mimeType == null - || MediaType.APPLICATION_OCTET_STREAM.toString().equals(mimeType)) { - // try guessing from data - try (InputStream is = new BufferedInputStream(resource.in())) { - mimeType = URLConnection.guessContentTypeFromStream(is); - } catch (IOException e) { - // do nothing, we'll just use application/octet-stream - } - } - return mimeType == null - ? MediaType.APPLICATION_OCTET_STREAM - : MediaType.valueOf(mimeType); - } else { - return null; + switch (resource.getType()) { + case DIRECTORY: + return getFormat(request); + case RESOURCE: + // set the mime if known by the servlet container, otherwise default to + // application/octet-stream to mitigate potential cross-site scripting + return Optional.ofNullable(request.getServletContext()) + .map(sc -> sc.getMimeType(resource.name())) + .map(MediaType::valueOf) + .orElse(MediaType.APPLICATION_OCTET_STREAM); + default: + throw new ResourceNotFoundException("Undefined resource path."); } } @@ -265,13 +258,13 @@ public Object resourceGet( Resource resource = resource(request); Operation operation = operation(operationName); Object result; - response.setContentType(getFormat(format).toString()); if (operation == Operation.METADATA) { result = wrapObject( new ResourceMetadataInfo(resource, request), ResourceMetadataInfo.class); + response.setContentType(getFormat(format).toString()); } else { if (resource.getType() == Resource.Type.UNDEFINED) { throw new ResourceNotFoundException("Undefined resource path."); @@ -279,7 +272,13 @@ public Object resourceGet( HttpHeaders responseHeaders = new HttpHeaders(); MediaType mediaType = getMediaType(resource, request); responseHeaders.setContentType(mediaType); - response.setContentType(mediaType.toString()); + if (resource.getType() == Resource.Type.RESOURCE) { + // Use Content-Disposition: attachment to mitigate potential XSS issues + responseHeaders.setContentDisposition( + ContentDisposition.builder("attachment") + .filename(resource.name()) + .build()); + } if (request.getMethod().equals("HEAD")) { result = new ResponseEntity<>("", responseHeaders, HttpStatus.OK); @@ -288,6 +287,7 @@ public Object resourceGet( wrapObject( new ResourceDirectoryInfo(resource, request), ResourceDirectoryInfo.class); + response.setContentType(mediaType.toString()); } else { result = new ResponseEntity<>(resource.in(), responseHeaders, HttpStatus.OK); } diff --git a/src/restconfig/src/test/java/org/geoserver/rest/resources/ResourceControllerTest.java b/src/restconfig/src/test/java/org/geoserver/rest/resources/ResourceControllerTest.java index b95d3f82abb..088e5453fe1 100644 --- a/src/restconfig/src/test/java/org/geoserver/rest/resources/ResourceControllerTest.java +++ b/src/restconfig/src/test/java/org/geoserver/rest/resources/ResourceControllerTest.java @@ -5,13 +5,13 @@ package org.geoserver.rest.resources; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import java.io.IOException; import java.io.InputStream; import java.io.OutputStreamWriter; -import java.net.URLConnection; import java.nio.charset.Charset; import java.nio.charset.CharsetEncoder; import java.text.DateFormat; @@ -239,6 +239,11 @@ public void testResourceMetadataHTML() throws Exception { public void testResourceHeaders() throws Exception { MockHttpServletResponse response = getAsServletResponse(RestBaseController.ROOT_PATH + "/resource/mydir2/fake.png"); + assertEquals( + "http://localhost:8080/geoserver" + + RestBaseController.ROOT_PATH + + "/resource/mydir2/fake.png", + response.getHeader("Location")); Assert.assertEquals( FORMAT_HEADER.format(getDataDirectory().get("mydir2/fake.png").lastmodified()), response.getHeader("Last-Modified")); @@ -249,12 +254,19 @@ public void testResourceHeaders() throws Exception { response.getHeader("Resource-Parent")); Assert.assertEquals("resource", response.getHeader("Resource-Type")); assertContentType("image/png", response); + assertEquals( + "attachment; filename=\"fake.png\"", response.getHeader("Content-Disposition")); } @Test public void testResourceHead() throws Exception { MockHttpServletResponse response = headAsServletResponse(RestBaseController.ROOT_PATH + "/resource/mydir2/fake.png"); + assertEquals( + "http://localhost:8080/geoserver" + + RestBaseController.ROOT_PATH + + "/resource/mydir2/fake.png", + response.getHeader("Location")); Assert.assertEquals( FORMAT_HEADER.format(getDataDirectory().get("mydir2/fake.png").lastmodified()), response.getHeader("Last-Modified")); @@ -265,6 +277,8 @@ public void testResourceHead() throws Exception { response.getHeader("Resource-Parent")); Assert.assertEquals("resource", response.getHeader("Resource-Type")); assertContentType("image/png", response); + assertEquals( + "attachment; filename=\"fake.png\"", response.getHeader("Content-Disposition")); } @Test @@ -409,7 +423,7 @@ public void testDirectoryJSONMultipleChildren() throws Exception { + " 'link': {\n" + " 'href': 'http://localhost:8080/geoserver/rest/resource/mydir2/imagewithoutextension',\n" + " 'rel': 'alternate',\n" - + " 'type': 'image/png'\n" + + " 'type': 'application/octet-stream'\n" + " }\n" + " },\n" + " {\n" @@ -417,7 +431,7 @@ public void testDirectoryJSONMultipleChildren() throws Exception { + " 'link': {\n" + " 'href': 'http://localhost:8080/geoserver/rest/resource/mydir2/myres.json',\n" + " 'rel': 'alternate',\n" - + " 'type': 'application/octet-stream'\n" + + " 'type': 'application/json'\n" + " }\n" + " },\n" + " {\n" @@ -430,9 +444,6 @@ public void testDirectoryJSONMultipleChildren() throws Exception { + " }\n" + " ]}\n" + "}}"; - // starting with JDK 17 (v3?) json is correctly recognized, the test output - String jsonType = URLConnection.guessContentTypeFromName("test.json"); - if (jsonType != null) expected = expected.replace("application/octet-stream", jsonType); JSONAssert.assertEquals(expected, (JSONObject) json); } @@ -508,7 +519,7 @@ public void testDirectoryMimeTypes() throws Exception { Document doc = getAsDOM(RestBaseController.ROOT_PATH + "/resource/mydir2?format=xml"); // print(doc); XMLAssert.assertXpathEvaluatesTo( - "image/png", + "application/octet-stream", "/ResourceDirectory/children/child[name='imagewithoutextension']/atom:link/@type", doc); XMLAssert.assertXpathEvaluatesTo(