From 867aec4cd10f2a3ef75193dd28a42d22645ab845 Mon Sep 17 00:00:00 2001 From: Raf Ponsaerts Date: Thu, 15 Oct 2020 18:31:04 +0200 Subject: [PATCH 1/4] Refactor/remove custom MultipartFileSender to use Spring Boot Range requests --- .../app/rest/BitstreamRestController.java | 48 +- .../app/rest/SitemapRestController.java | 26 +- .../app/rest/utils/BitstreamResource.java | 49 ++ .../rest/utils/HttpHeadersInitializer.java | 263 +++++++++ .../app/rest/utils/MultipartFileSender.java | 488 ---------------- .../app/rest/BitstreamRestControllerIT.java | 10 +- .../rest/utils/MultipartFileSenderTest.java | 536 ------------------ 7 files changed, 364 insertions(+), 1056 deletions(-) create mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java create mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/MultipartFileSender.java delete mode 100644 dspace-server-webapp/src/test/java/org/dspace/app/rest/utils/MultipartFileSenderTest.java diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java index 7149996d4d42..2ce1c4ffc7d8 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java @@ -21,6 +21,7 @@ import javax.ws.rs.core.Response; import org.apache.catalina.connector.ClientAbortException; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.tuple.Pair; import org.apache.logging.log4j.Logger; import org.dspace.app.rest.converter.ConverterService; @@ -28,7 +29,7 @@ import org.dspace.app.rest.model.BitstreamRest; import org.dspace.app.rest.model.hateoas.BitstreamResource; import org.dspace.app.rest.utils.ContextUtil; -import org.dspace.app.rest.utils.MultipartFileSender; +import org.dspace.app.rest.utils.HttpHeadersInitializer; import org.dspace.app.rest.utils.Utils; import org.dspace.authorize.AuthorizeException; import org.dspace.content.Bitstream; @@ -42,6 +43,8 @@ import org.dspace.usage.UsageEvent; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.rest.webmvc.ResourceNotFoundException; +import org.springframework.http.HttpHeaders; +import org.springframework.http.ResponseEntity; import org.springframework.security.access.prepost.PostAuthorize; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.web.bind.annotation.PathVariable; @@ -98,7 +101,7 @@ public class BitstreamRestController { @PreAuthorize("hasPermission(#uuid, 'BITSTREAM', 'READ')") @RequestMapping( method = {RequestMethod.GET, RequestMethod.HEAD}, value = "content") - public void retrieve(@PathVariable UUID uuid, HttpServletResponse response, + public ResponseEntity retrieve(@PathVariable UUID uuid, HttpServletResponse response, HttpServletRequest request) throws IOException, SQLException, AuthorizeException { @@ -107,7 +110,7 @@ public void retrieve(@PathVariable UUID uuid, HttpServletResponse response, Bitstream bit = bitstreamService.find(context, uuid); if (bit == null) { response.sendError(HttpServletResponse.SC_NOT_FOUND); - return; + return null; } Long lastModified = bitstreamService.getLastModified(bit); @@ -117,9 +120,21 @@ public void retrieve(@PathVariable UUID uuid, HttpServletResponse response, Pair bitstreamTuple = getBitstreamInputStreamAndSize(context, bit); + if (StringUtils.isBlank(request.getHeader("Range"))) { + //We only log a download request when serving a request without Range header. This is because + //a browser always sends a regular request first to check for Range support. + eventService.fireEvent( + new UsageEvent( + UsageEvent.Action.VIEW, + request, + context, + bit)); + } + // Pipe the bits - try (InputStream is = bitstreamTuple.getLeft()) { - MultipartFileSender sender = MultipartFileSender + InputStream is = bitstreamTuple.getLeft(); + try { + HttpHeadersInitializer httpHeadersInitializer = HttpHeadersInitializer .fromInputStream(is) .withBufferSize(BUFFER_SIZE) .withFileName(name) @@ -130,39 +145,34 @@ public void retrieve(@PathVariable UUID uuid, HttpServletResponse response, .with(response); if (lastModified != null) { - sender.withLastModified(lastModified); + httpHeadersInitializer.withLastModified(lastModified); } //Determine if we need to send the file as a download or if the browser can open it inline long dispositionThreshold = configurationService.getLongProperty("webui.content_disposition_threshold"); if (dispositionThreshold >= 0 && bitstreamTuple.getRight() > dispositionThreshold) { - sender.withDisposition(MultipartFileSender.CONTENT_DISPOSITION_ATTACHMENT); + httpHeadersInitializer.withDisposition(HttpHeadersInitializer.CONTENT_DISPOSITION_ATTACHMENT); } - if (sender.isNoRangeRequest() && isNotAnErrorResponse(response)) { - //We only log a download request when serving a request without Range header. This is because - //a browser always sends a regular request first to check for Range support. - eventService.fireEvent( - new UsageEvent( - UsageEvent.Action.VIEW, - request, - context, - bit)); - } + + org.dspace.app.rest.utils.BitstreamResource bitstreamResource = + new org.dspace.app.rest.utils.BitstreamResource(is, name, uuid, bit.getSizeBytes()); //We have all the data we need, close the connection to the database so that it doesn't stay open during //download/streaming context.complete(); //Send the data - if (sender.isValid()) { - sender.serveResource(); + if (httpHeadersInitializer.isValid()) { + HttpHeaders httpHeaders = httpHeadersInitializer.initialiseHeaders(); + return ResponseEntity.ok().headers(httpHeaders).body(bitstreamResource); } } catch (ClientAbortException ex) { log.debug("Client aborted the request before the download was completed. " + "Client is probably switching to a Range request.", ex); } + return null; } private Pair getBitstreamInputStreamAndSize(Context context, Bitstream bit) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/SitemapRestController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/SitemapRestController.java index 4eef1ba34b9b..d047f00b7dd8 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/SitemapRestController.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/SitemapRestController.java @@ -19,11 +19,14 @@ import org.apache.catalina.connector.ClientAbortException; import org.apache.logging.log4j.Logger; import org.dspace.app.rest.utils.ContextUtil; -import org.dspace.app.rest.utils.MultipartFileSender; +import org.dspace.app.rest.utils.HttpHeadersInitializer; import org.dspace.core.Context; import org.dspace.services.ConfigurationService; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.core.io.FileSystemResource; import org.springframework.data.rest.webmvc.ResourceNotFoundException; +import org.springframework.http.HttpHeaders; +import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; @@ -62,10 +65,11 @@ public class SitemapRestController { * @param request the HTTP request * @throws SQLException if db error while completing DSpace context * @throws IOException if IO error surrounding sitemap file + * @return */ @GetMapping("/{name}") - public void retrieve(@PathVariable String name, HttpServletResponse response, - HttpServletRequest request) throws IOException, SQLException { + public ResponseEntity retrieve(@PathVariable String name, HttpServletResponse response, + HttpServletRequest request) throws IOException, SQLException { // Find sitemap with given name in dspace/sitemaps File foundSitemapFile = null; File sitemapOutputDir = new File(configurationService.getProperty("sitemap.dir")); @@ -94,7 +98,7 @@ public void retrieve(@PathVariable String name, HttpServletResponse response, "Could not find sitemap file with name " + name + " in " + sitemapOutputDir.getAbsolutePath()); } else { // return found sitemap file - this.returnSitemapFile(foundSitemapFile, response, request); + return this.returnSitemapFile(foundSitemapFile, response, request); } } @@ -107,12 +111,13 @@ public void retrieve(@PathVariable String name, HttpServletResponse response, * @param request the HTTP request * @throws SQLException if db error while completing DSpace context * @throws IOException if IO error surrounding sitemap file + * @return */ - private void returnSitemapFile(File foundSitemapFile, HttpServletResponse response, HttpServletRequest request) - throws SQLException, IOException { + private ResponseEntity returnSitemapFile(File foundSitemapFile, HttpServletResponse response, + HttpServletRequest request) throws SQLException, IOException { // Pipe the bits try (InputStream is = new FileInputStream(foundSitemapFile)) { - MultipartFileSender sender = MultipartFileSender + HttpHeadersInitializer sender = HttpHeadersInitializer .fromInputStream(is) .withBufferSize(BUFFER_SIZE) .withFileName(foundSitemapFile.getName()) @@ -126,7 +131,7 @@ private void returnSitemapFile(File foundSitemapFile, HttpServletResponse respon // Determine if we need to send the file as a download or if the browser can open it inline long dispositionThreshold = configurationService.getLongProperty("webui.content_disposition_threshold"); if (dispositionThreshold >= 0 && foundSitemapFile.length() > dispositionThreshold) { - sender.withDisposition(MultipartFileSender.CONTENT_DISPOSITION_ATTACHMENT); + sender.withDisposition(HttpHeadersInitializer.CONTENT_DISPOSITION_ATTACHMENT); } Context context = ContextUtil.obtainContext(request); @@ -137,12 +142,15 @@ private void returnSitemapFile(File foundSitemapFile, HttpServletResponse respon // Send the data if (sender.isValid()) { - sender.serveResource(); + HttpHeaders httpHeaders = sender.initialiseHeaders(); + return ResponseEntity.ok().headers(httpHeaders).body(new FileSystemResource(foundSitemapFile)); + } } catch (ClientAbortException e) { log.debug("Client aborted the request before the download was completed. " + "Client is probably switching to a Range request.", e); } + return null; } } diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java new file mode 100644 index 000000000000..b1b6e463c2d9 --- /dev/null +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java @@ -0,0 +1,49 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.rest.utils; + +import java.io.IOException; +import java.io.InputStream; +import java.util.UUID; + +import org.springframework.core.io.AbstractResource; + +public class BitstreamResource extends AbstractResource { + + private InputStream inputStream; + private String name; + private UUID uuid; + private long sizeBytes; + + public BitstreamResource(InputStream inputStream, String name, UUID uuid, long sizeBytes) { + this.inputStream = inputStream; + this.name = name; + this.uuid = uuid; + this.sizeBytes = sizeBytes; + } + + @Override + public String getDescription() { + return "bitstream [" + uuid + "]"; + } + + @Override + public InputStream getInputStream() throws IOException { + return inputStream; + } + + @Override + public String getFilename() { + return name; + } + + @Override + public long contentLength() throws IOException { + return sizeBytes; + } +} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java new file mode 100644 index 000000000000..63498c4b20b7 --- /dev/null +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java @@ -0,0 +1,263 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.rest.utils; + +import static java.util.Objects.isNull; +import static java.util.Objects.nonNull; + +import java.io.BufferedInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.Arrays; +import java.util.Collections; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.apache.commons.lang3.StringUtils; +import org.apache.tomcat.util.http.FastHttpDateFormat; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.http.HttpHeaders; + +/** + * Utility class to send an input stream with Range header and ETag support. + * Based on https://github.com/davinkevin/Podcast-Server/blob/v1.0.0/src/main/java/lan/dk/podcastserver/service + * /MultiPartFileSenderService.java + * + * @author Tom Desair (tom dot desair at atmire dot com) + * @author Frederic Van Reet (frederic dot vanreet at atmire dot com) + */ +public class HttpHeadersInitializer { + + protected final Logger log = LoggerFactory.getLogger(this.getClass()); + + private static final String METHOD_HEAD = "HEAD"; + private static final String MULTIPART_BOUNDARY = "MULTIPART_BYTERANGES"; + private static final String CONTENT_TYPE_MULTITYPE_WITH_BOUNDARY = "multipart/byteranges; boundary=" + + MULTIPART_BOUNDARY; + public static final String CONTENT_DISPOSITION_INLINE = "inline"; + public static final String CONTENT_DISPOSITION_ATTACHMENT = "attachment"; + private static final String IF_NONE_MATCH = "If-None-Match"; + private static final String IF_MODIFIED_SINCE = "If-Modified-Since"; + private static final String ETAG = "ETag"; + private static final String IF_MATCH = "If-Match"; + private static final String IF_UNMODIFIED_SINCE = "If-Unmodified-Since"; + private static final String CONTENT_TYPE = "Content-Type"; + private static final String ACCEPT_RANGES = "Accept-Ranges"; + private static final String BYTES = "bytes"; + private static final String LAST_MODIFIED = "Last-Modified"; + private static final String EXPIRES = "Expires"; + private static final String APPLICATION_OCTET_STREAM = "application/octet-stream"; + private static final String IMAGE = "image"; + private static final String ACCEPT = "Accept"; + private static final String CONTENT_DISPOSITION = "Content-Disposition"; + private static final String CONTENT_DISPOSITION_FORMAT = "%s;filename=\"%s\""; + private static final String CACHE_CONTROL = "Cache-Control"; + + private int bufferSize = 1000000; + + private static final long DEFAULT_EXPIRE_TIME = 60L * 60L * 1000L; + + //no-cache so request is always performed for logging + private static final String CACHE_CONTROL_SETTING = "private,no-cache"; + + private BufferedInputStream inputStream; + private HttpServletRequest request; + private HttpServletResponse response; + private String contentType; + private String disposition; + private long lastModified; + private long length; + private String fileName; + private String checksum; + + public HttpHeadersInitializer(final InputStream inputStream) { + //Convert to BufferedInputStream so we can re-read the stream + this.inputStream = new BufferedInputStream(inputStream); + } + + + public static HttpHeadersInitializer fromInputStream(InputStream inputStream) { + return new HttpHeadersInitializer(inputStream); + } + + public HttpHeadersInitializer with(HttpServletRequest httpRequest) { + request = httpRequest; + return this; + } + + public HttpHeadersInitializer with(HttpServletResponse httpResponse) { + response = httpResponse; + return this; + } + + public HttpHeadersInitializer withLength(long length) { + this.length = length; + return this; + } + + public HttpHeadersInitializer withFileName(String fileName) { + this.fileName = fileName; + return this; + } + + public HttpHeadersInitializer withChecksum(String checksum) { + this.checksum = checksum; + return this; + } + + public HttpHeadersInitializer withMimetype(String mimetype) { + this.contentType = mimetype; + return this; + } + + public HttpHeadersInitializer withLastModified(long lastModified) { + this.lastModified = lastModified; + return this; + } + + public HttpHeadersInitializer withBufferSize(int bufferSize) { + if (bufferSize > 0) { + this.bufferSize = bufferSize; + } + return this; + } + public HttpHeadersInitializer withDisposition(String contentDisposition) { + this.disposition = contentDisposition; + return this; + } + + //TODO rename to initialiseHeaders + public HttpHeaders initialiseHeaders() throws IOException { + + HttpHeaders httpHeaders = new HttpHeaders(); + // Validate and process range ------------------------------------------------------------- + + log.debug("Content-Type : {}", contentType); + // Initialize response. + //TODO response.reset => Can be re-instated if we bump to 5.2.9 + response.setBufferSize(bufferSize); + if (contentType != null) { + httpHeaders.put(CONTENT_TYPE, Collections.singletonList(contentType)); + } + httpHeaders.put(ACCEPT_RANGES, Collections.singletonList(BYTES)); + if (checksum != null) { + httpHeaders.put(ETAG, Collections.singletonList(checksum)); + } + httpHeaders.put(LAST_MODIFIED, Collections.singletonList(FastHttpDateFormat.formatDate(lastModified))); + httpHeaders.put(EXPIRES, Collections.singletonList(FastHttpDateFormat.formatDate( + System.currentTimeMillis() + DEFAULT_EXPIRE_TIME))); + + //No-cache so that we can log every download + httpHeaders.put(CACHE_CONTROL, Collections.singletonList(CACHE_CONTROL_SETTING)); + + if (isNullOrEmpty(disposition)) { + if (contentType == null) { + contentType = APPLICATION_OCTET_STREAM; + } else if (!contentType.startsWith(IMAGE)) { + String accept = request.getHeader(ACCEPT); + disposition = accept != null && accepts(accept, + contentType) ? CONTENT_DISPOSITION_INLINE : + CONTENT_DISPOSITION_ATTACHMENT; + } + + } + + httpHeaders.put(CONTENT_DISPOSITION, Collections.singletonList(String.format(CONTENT_DISPOSITION_FORMAT, + disposition, fileName))); + log.debug("Content-Disposition : {}", disposition); + + // Content phase + if (METHOD_HEAD.equals(request.getMethod())) { + log.debug("HEAD request - skipping content"); + return null; + } + + return httpHeaders; + + } + + public boolean isValid() throws IOException { + if (response == null || request == null) { + return false; + } + + if (inputStream == null) { + log.error("Input stream has no content"); + response.sendError(HttpServletResponse.SC_NOT_FOUND); + return false; + } + + if (StringUtils.isEmpty(fileName)) { + response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + return false; + } + + // Validate request headers for caching --------------------------------------------------- + // If-None-Match header should contain "*" or ETag. If so, then return 304. + String ifNoneMatch = request.getHeader(IF_NONE_MATCH); + if (nonNull(ifNoneMatch) && matches(ifNoneMatch, checksum)) { + log.debug("If-None-Match header should contain \"*\" or ETag. If so, then return 304."); + response.setHeader(ETAG, checksum); // Required in 304. + response.sendError(HttpServletResponse.SC_NOT_MODIFIED); + return false; + } + + // If-Modified-Since header should be greater than LastModified. If so, then return 304. + // This header is ignored if any If-None-Match header is specified. + long ifModifiedSince = request.getDateHeader(IF_MODIFIED_SINCE); + if (isNull(ifNoneMatch) && ifModifiedSince != -1 && ifModifiedSince + 1000 > lastModified) { + log.debug("If-Modified-Since header should be greater than LastModified. If so, then return 304."); + response.setHeader(ETAG, checksum); // Required in 304. + response.sendError(HttpServletResponse.SC_NOT_MODIFIED); + return false; + } + + // Validate request headers for resume ---------------------------------------------------- + + // If-Match header should contain "*" or ETag. If not, then return 412. + String ifMatch = request.getHeader(IF_MATCH); + if (nonNull(ifMatch) && !matches(ifMatch, checksum)) { + log.error("If-Match header should contain \"*\" or ETag. If not, then return 412."); + response.sendError(HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE); + return false; + } + + // If-Unmodified-Since header should be greater than LastModified. If not, then return 412. + long ifUnmodifiedSince = request.getDateHeader(IF_UNMODIFIED_SINCE); + if (ifUnmodifiedSince != -1 && ifUnmodifiedSince + 1000 <= lastModified) { + log.error("If-Unmodified-Since header should be greater than LastModified. If not, then return 412."); + response.sendError(HttpServletResponse.SC_PRECONDITION_FAILED); + return false; + } + + return true; + } + + + private static boolean isNullOrEmpty(String disposition) { + return StringUtils.isBlank(disposition); + } + + + private static boolean accepts(String acceptHeader, String toAccept) { + String[] acceptValues = acceptHeader.split("\\s*(,|;)\\s*"); + Arrays.sort(acceptValues); + + return Arrays.binarySearch(acceptValues, toAccept) > -1 + || Arrays.binarySearch(acceptValues, toAccept.replaceAll("/.*$", "/*")) > -1 + || Arrays.binarySearch(acceptValues, "*/*") > -1; + } + + private static boolean matches(String matchHeader, String toMatch) { + String[] matchValues = matchHeader.split("\\s*,\\s*"); + Arrays.sort(matchValues); + return Arrays.binarySearch(matchValues, toMatch) > -1 || Arrays.binarySearch(matchValues, "*") > -1; + } + +} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/MultipartFileSender.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/MultipartFileSender.java deleted file mode 100644 index 284d0b87ab48..000000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/MultipartFileSender.java +++ /dev/null @@ -1,488 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.utils; - -import static java.util.Objects.isNull; -import static java.util.Objects.nonNull; - -import java.io.BufferedInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import javax.servlet.ServletOutputStream; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -import org.apache.commons.lang3.StringUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Utility class to send an input stream with Range header and ETag support. - * Based on https://github.com/davinkevin/Podcast-Server/blob/v1.0.0/src/main/java/lan/dk/podcastserver/service - * /MultiPartFileSenderService.java - * - * @author Tom Desair (tom dot desair at atmire dot com) - * @author Frederic Van Reet (frederic dot vanreet at atmire dot com) - */ -public class MultipartFileSender { - - protected final Logger log = LoggerFactory.getLogger(this.getClass()); - - private static final String METHOD_HEAD = "HEAD"; - private static final String MULTIPART_BOUNDARY = "MULTIPART_BYTERANGES"; - private static final String CONTENT_TYPE_MULTITYPE_WITH_BOUNDARY = "multipart/byteranges; boundary=" + - MULTIPART_BOUNDARY; - public static final String CONTENT_DISPOSITION_INLINE = "inline"; - public static final String CONTENT_DISPOSITION_ATTACHMENT = "attachment"; - private static final String IF_NONE_MATCH = "If-None-Match"; - private static final String IF_MODIFIED_SINCE = "If-Modified-Since"; - private static final String ETAG = "ETag"; - private static final String IF_MATCH = "If-Match"; - private static final String IF_UNMODIFIED_SINCE = "If-Unmodified-Since"; - private static final String RANGE = "Range"; - private static final String CONTENT_RANGE = "Content-Range"; - private static final String IF_RANGE = "If-Range"; - private static final String CONTENT_TYPE = "Content-Type"; - private static final String ACCEPT_RANGES = "Accept-Ranges"; - private static final String BYTES = "bytes"; - private static final String LAST_MODIFIED = "Last-Modified"; - private static final String EXPIRES = "Expires"; - private static final String APPLICATION_OCTET_STREAM = "application/octet-stream"; - private static final String IMAGE = "image"; - private static final String ACCEPT = "Accept"; - private static final String CONTENT_DISPOSITION = "Content-Disposition"; - private static final String CONTENT_LENGTH = "Content-Length"; - private static final String BYTES_RANGE_FORMAT = "bytes %d-%d/%d"; - private static final String CONTENT_DISPOSITION_FORMAT = "%s;filename=\"%s\""; - private static final String BYTES_DINVALID_BYTE_RANGE_FORMAT = "bytes */%d"; - private static final String CACHE_CONTROL = "Cache-Control"; - - private int bufferSize = 1000000; - - private static final long DEFAULT_EXPIRE_TIME = 60L * 60L * 1000L; - - //no-cache so request is always performed for logging - private static final String CACHE_CONTROL_SETTING = "private,no-cache"; - - private BufferedInputStream inputStream; - private HttpServletRequest request; - private HttpServletResponse response; - private String contentType; - private String disposition; - private long lastModified; - private long length; - private String fileName; - private String checksum; - - public MultipartFileSender(final InputStream inputStream) { - //Convert to BufferedInputStream so we can re-read the stream - this.inputStream = new BufferedInputStream(inputStream); - } - - - public static MultipartFileSender fromInputStream(InputStream inputStream) { - return new MultipartFileSender(inputStream); - } - - public MultipartFileSender with(HttpServletRequest httpRequest) { - request = httpRequest; - return this; - } - - public MultipartFileSender with(HttpServletResponse httpResponse) { - response = httpResponse; - return this; - } - - public MultipartFileSender withLength(long length) { - this.length = length; - return this; - } - - public MultipartFileSender withFileName(String fileName) { - this.fileName = fileName; - return this; - } - - public MultipartFileSender withChecksum(String checksum) { - this.checksum = checksum; - return this; - } - - public MultipartFileSender withMimetype(String mimetype) { - this.contentType = mimetype; - return this; - } - - public MultipartFileSender withLastModified(long lastModified) { - this.lastModified = lastModified; - return this; - } - - public MultipartFileSender withBufferSize(int bufferSize) { - if (bufferSize > 0) { - this.bufferSize = bufferSize; - } - return this; - } - public MultipartFileSender withDisposition(String contentDisposition) { - this.disposition = contentDisposition; - return this; - } - - public void serveResource() throws IOException { - - // Validate and process range ------------------------------------------------------------- - - // Prepare some variables. The full Range represents the complete file. - Range full = getFullRange(); - List ranges = getRanges(full); - - if (ranges == null) { - //The supplied range values were invalid - return; - } - - log.debug("Content-Type : {}", contentType); - // Initialize response. - response.reset(); - response.setBufferSize(bufferSize); - if (contentType != null) { - response.setHeader(CONTENT_TYPE, contentType); - } - response.setHeader(ACCEPT_RANGES, BYTES); - if (checksum != null) { - response.setHeader(ETAG, checksum); - } - response.setDateHeader(LAST_MODIFIED, lastModified); - response.setDateHeader(EXPIRES, System.currentTimeMillis() + DEFAULT_EXPIRE_TIME); - - //No-cache so that we can log every download - response.setHeader(CACHE_CONTROL, CACHE_CONTROL_SETTING); - - - if (isNullOrEmpty(disposition)) { - if (contentType == null) { - contentType = APPLICATION_OCTET_STREAM; - } else if (!contentType.startsWith(IMAGE)) { - String accept = request.getHeader(ACCEPT); - disposition = accept != null && accepts(accept, - contentType) ? CONTENT_DISPOSITION_INLINE : - CONTENT_DISPOSITION_ATTACHMENT; - } - - } - response.setHeader(CONTENT_DISPOSITION, String.format(CONTENT_DISPOSITION_FORMAT, disposition, fileName)); - log.debug("Content-Disposition : {}", disposition); - - // Content phase - if (METHOD_HEAD.equals(request.getMethod())) { - log.debug("HEAD request - skipping content"); - return; - } - // Send requested file (part(s)) to client ------------------------------------------------ - - // Prepare streams. - try (OutputStream output = response.getOutputStream()) { - - - if (hasNoRanges(full, ranges)) { - - // Return full file. - log.debug("Return full file"); - response.setContentType(contentType); - response.setHeader(CONTENT_LENGTH, String.valueOf(length)); - Range.copy(inputStream, output, length, 0, length, bufferSize); - - } else if (ranges.size() == 1) { - - // Return single part of file. - Range r = ranges.get(0); - log.debug("Return 1 part of file : from ({}) to ({})", r.start, r.end); - response.setContentType(contentType); - response.setHeader(CONTENT_RANGE, String.format(BYTES_RANGE_FORMAT, r.start, r.end, r.total)); - response.setHeader(CONTENT_LENGTH, String.valueOf(r.length)); - response.setStatus(HttpServletResponse.SC_PARTIAL_CONTENT); // 206. - - // Copy single part range. - Range.copy(inputStream, output, length, r.start, r.length, bufferSize); - - } else { - - // Return multiple parts of file. - response.setContentType(CONTENT_TYPE_MULTITYPE_WITH_BOUNDARY); - response.setStatus(HttpServletResponse.SC_PARTIAL_CONTENT); // 206. - - // Cast back to ServletOutputStream to get the easy println methods. - ServletOutputStream sos = (ServletOutputStream) output; - - // Copy multi part range. - for (Range r : ranges) { - log.debug("Return multi part of file : from ({}) to ({})", r.start, r.end); - // Add multipart boundary and header fields for every range. - sos.println("--" + MULTIPART_BOUNDARY); - sos.println(CONTENT_TYPE + ": " + contentType); - sos.println(CONTENT_RANGE + ": " + String.format(BYTES_RANGE_FORMAT, r.start, r.end, r.total)); - - //Mark position of inputstream so we can return to it later - inputStream.mark(0); - // Copy single part range of multi part range. - Range.copy(inputStream, output, length, r.start, r.length, bufferSize); - inputStream.reset(); - - sos.println(); - } - - // End with multipart boundary. - sos.println("--" + MULTIPART_BOUNDARY + "--"); - } - } - - - } - - public boolean isValid() throws IOException { - if (response == null || request == null) { - return false; - } - - if (inputStream == null) { - log.error("Input stream has no content"); - response.sendError(HttpServletResponse.SC_NOT_FOUND); - return false; - } - - if (StringUtils.isEmpty(fileName)) { - response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - return false; - } - - // Validate request headers for caching --------------------------------------------------- - // If-None-Match header should contain "*" or ETag. If so, then return 304. - String ifNoneMatch = request.getHeader(IF_NONE_MATCH); - if (nonNull(ifNoneMatch) && matches(ifNoneMatch, checksum)) { - log.debug("If-None-Match header should contain \"*\" or ETag. If so, then return 304."); - response.setHeader(ETAG, checksum); // Required in 304. - response.sendError(HttpServletResponse.SC_NOT_MODIFIED); - return false; - } - - // If-Modified-Since header should be greater than LastModified. If so, then return 304. - // This header is ignored if any If-None-Match header is specified. - long ifModifiedSince = request.getDateHeader(IF_MODIFIED_SINCE); - if (isNull(ifNoneMatch) && ifModifiedSince != -1 && ifModifiedSince + 1000 > lastModified) { - log.debug("If-Modified-Since header should be greater than LastModified. If so, then return 304."); - response.setHeader(ETAG, checksum); // Required in 304. - response.sendError(HttpServletResponse.SC_NOT_MODIFIED); - return false; - } - - // Validate request headers for resume ---------------------------------------------------- - - // If-Match header should contain "*" or ETag. If not, then return 412. - String ifMatch = request.getHeader(IF_MATCH); - if (nonNull(ifMatch) && !matches(ifMatch, checksum)) { - log.error("If-Match header should contain \"*\" or ETag. If not, then return 412."); - response.sendError(HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE); - return false; - } - - // If-Unmodified-Since header should be greater than LastModified. If not, then return 412. - long ifUnmodifiedSince = request.getDateHeader(IF_UNMODIFIED_SINCE); - if (ifUnmodifiedSince != -1 && ifUnmodifiedSince + 1000 <= lastModified) { - log.error("If-Unmodified-Since header should be greater than LastModified. If not, then return 412."); - response.sendError(HttpServletResponse.SC_PRECONDITION_FAILED); - return false; - } - - return true; - } - - public boolean isNoRangeRequest() throws IOException { - Range full = getFullRange(); - List ranges = getRanges(full); - - if (hasNoRanges(full, ranges)) { - return true; - } else { - return false; - } - } - - private boolean hasNoRanges(final Range full, final List ranges) { - return ranges != null && (ranges.isEmpty() || ranges.get(0) == full); - } - - private Range getFullRange() { - return new Range(0, length - 1, length); - } - - - private List getRanges(final Range fullRange) throws IOException { - List ranges = new ArrayList<>(); - - // Validate and process Range and If-Range headers. - String range = request.getHeader(RANGE); - if (nonNull(range)) { - - // Range header should match format "bytes=n-n,n-n,n-n...". If not, then return 416. - if (!range.matches("^bytes=\\d*-\\d*(,\\d*-\\d*)*$")) { - log.error("Range header should match format \"bytes=n-n,n-n,n-n...\". If not, then return 416."); - response.setHeader(CONTENT_RANGE, - String.format(BYTES_DINVALID_BYTE_RANGE_FORMAT, length)); // Required in 416. - response.sendError(HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE); - return null; - } - - String ifRange = request.getHeader(IF_RANGE); - if (nonNull(ifRange) && !ifRange.equals(fileName)) { - try { - //Assume that the If-Range contains a date - long ifRangeTime = request.getDateHeader(IF_RANGE); // Throws IAE if invalid. - - if (ifRangeTime == -1 || ifRangeTime + 1000 <= lastModified) { - //Our file has been updated, send the full range - ranges.add(fullRange); - } - - } catch (IllegalArgumentException ignore) { - //Assume that the If-Range contains an ETag - if (!matches(ifRange, checksum)) { - //Our file has been updated, send the full range - ranges.add(fullRange); - } - } - } - - // If any valid If-Range header, then process each part of byte range. - if (ranges.isEmpty()) { - log.debug("If any valid If-Range header, then process each part of byte range."); - for (String part : range.substring(6).split(",")) { - // Assuming a file with length of 100, the following examples returns bytes at: - // 50-80 (50 to 80), 40- (40 to length=100), -20 (length-20=80 to length=100). - long start = Range.sublong(part, 0, part.indexOf("-")); - long end = Range.sublong(part, part.indexOf("-") + 1, part.length()); - - if (start == -1) { - start = length - end; - end = length - 1; - } else if (end == -1 || end > length - 1) { - end = length - 1; - } - - // Check if Range is syntactically valid. If not, then return 416. - if (start > end) { - log.warn("Check if Range is syntactically valid. If not, then return 416."); - response.setHeader(CONTENT_RANGE, - String.format(BYTES_DINVALID_BYTE_RANGE_FORMAT, length)); // Required in 416. - response.sendError(HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE); - return null; - } - - // Add range. - ranges.add(new Range(start, end, length)); - } - } - } - return ranges; - } - - private static boolean isNullOrEmpty(String disposition) { - return StringUtils.isBlank(disposition); - } - - - private static class Range { - long start; - long end; - long length; - long total; - - /** - * Construct a byte range. - * - * @param start Start of the byte range. - * @param end End of the byte range. - * @param total Total length of the byte source. - */ - public Range(long start, long end, long total) { - this.start = start; - this.end = end; - this.length = this.end - this.start + 1; - this.total = total; - } - - private static List relativize(List ranges) { - - List builder = new ArrayList<>(ranges.size()); - - Range prevRange = null; - for (Range r : ranges) { - Range newRange = isNull(prevRange) ? r : new Range(r.start - prevRange.end - 1, - r.end - prevRange.end - 1, r.total); - builder.add(newRange); - prevRange = r; - } - - return builder; - } - - public static long sublong(String value, int beginIndex, int endIndex) { - String substring = value.substring(beginIndex, endIndex); - return (substring.length() > 0) ? Long.parseLong(substring) : -1; - } - - private static void copy(InputStream input, OutputStream output, long inputSize, long start, long length, - int bufferSize) throws IOException { - byte[] buffer = new byte[bufferSize]; - int read; - - if (inputSize == length) { - // Write full range. - while ((read = input.read(buffer)) > 0) { - output.write(buffer, 0, read); - output.flush(); - } - } else { - input.skip(start); - long toRead = length; - - while ((read = input.read(buffer)) > 0) { - if ((toRead -= read) > 0) { - output.write(buffer, 0, read); - output.flush(); - } else { - output.write(buffer, 0, (int) toRead + read); - output.flush(); - break; - } - } - } - } - } - - private static boolean accepts(String acceptHeader, String toAccept) { - String[] acceptValues = acceptHeader.split("\\s*(,|;)\\s*"); - Arrays.sort(acceptValues); - - return Arrays.binarySearch(acceptValues, toAccept) > -1 - || Arrays.binarySearch(acceptValues, toAccept.replaceAll("/.*$", "/*")) > -1 - || Arrays.binarySearch(acceptValues, "*/*") > -1; - } - - private static boolean matches(String matchHeader, String toMatch) { - String[] matchValues = matchHeader.split("\\s*,\\s*"); - Arrays.sort(matchValues); - return Arrays.binarySearch(matchValues, toMatch) > -1 || Arrays.binarySearch(matchValues, "*") > -1; - } - -} diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamRestControllerIT.java index 2a68c2e8876d..18131444d8fd 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamRestControllerIT.java @@ -198,7 +198,9 @@ public void retrieveFullBitstream() throws Exception { //The server should indicate we support Range requests .andExpect(header().string("Accept-Ranges", "bytes")) //The ETag has to be based on the checksum - .andExpect(header().string("ETag", bitstream.getChecksum())) + // We're checking this with quotes because it is required: + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ETag + .andExpect(header().string("ETag", "\"" + bitstream.getChecksum() + "\"")) //We expect the content type to match the bitstream mime type .andExpect(content().contentType("text/plain")) //THe bytes of the content must match the original content @@ -258,7 +260,7 @@ public void retrieveRangeBitstream() throws Exception { //The server should indicate we support Range requests .andExpect(header().string("Accept-Ranges", "bytes")) //The ETag has to be based on the checksum - .andExpect(header().string("ETag", bitstream.getChecksum())) + .andExpect(header().string("ETag", "\"" + bitstream.getChecksum() + "\"")) //The response should give us details about the range .andExpect(header().string("Content-Range", "bytes 1-3/10")) //We expect the content type to match the bitstream mime type @@ -279,7 +281,7 @@ public void retrieveRangeBitstream() throws Exception { //The server should indicate we support Range requests .andExpect(header().string("Accept-Ranges", "bytes")) //The ETag has to be based on the checksum - .andExpect(header().string("ETag", bitstream.getChecksum())) + .andExpect(header().string("ETag", "\"" + bitstream.getChecksum() + "\"")) //The response should give us details about the range .andExpect(header().string("Content-Range", "bytes 4-9/10")) //We expect the content type to match the bitstream mime type @@ -775,7 +777,7 @@ public void retrieveCitationCoverpageOfBitstream() throws Exception { //The server should indicate we support Range requests .andExpect(header().string("Accept-Ranges", "bytes")) //The ETag has to be based on the checksum - .andExpect(header().string("ETag", bitstream.getChecksum())) + .andExpect(header().string("ETag", "\"" + bitstream.getChecksum() + "\"")) //We expect the content type to match the bitstream mime type .andExpect(content().contentType("application/pdf")) //THe bytes of the content must match the original content diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/utils/MultipartFileSenderTest.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/utils/MultipartFileSenderTest.java deleted file mode 100644 index e800b021e994..000000000000 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/utils/MultipartFileSenderTest.java +++ /dev/null @@ -1,536 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.utils; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import java.io.IOException; -import java.io.InputStream; -import java.util.Date; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -import org.apache.commons.io.IOUtils; -import org.apache.commons.lang3.CharEncoding; -import org.apache.logging.log4j.Logger; -import org.dspace.authorize.AuthorizeException; -import org.junit.After; -import org.junit.Before; -import org.junit.Test; -import org.springframework.mock.web.MockHttpServletResponse; -import org.springframework.web.util.ContentCachingRequestWrapper; -import org.springframework.web.util.ContentCachingResponseWrapper; - -/** - * Test class for MultipartFileSender - * - * @author Tom Desair (tom dot desair at atmire dot com) - * @author Frederic Van Reet (frederic dot vanreet at atmire dot com) - */ -public class MultipartFileSenderTest { - - /** - * log4j category - */ - private static final Logger log = org.apache.logging.log4j.LogManager.getLogger(MultipartFileSenderTest.class); - - private InputStream is; - private String mimeType; - private long lastModified; - private long length; - private String fileName; - private String checksum; - - - private HttpServletRequest request; - - private HttpServletResponse response; - - private ContentCachingRequestWrapper requestWrapper; - private ContentCachingResponseWrapper responseWrapper; - - - /** - * This method will be run before every test as per @Before. It will - * initialize resources required for the tests. - *

- * Other methods can be annotated with @Before here or in subclasses - * but no execution order is guaranteed - */ - @Before - public void init() throws AuthorizeException { - try { - String content = "0123456789"; - - this.is = IOUtils.toInputStream(content, CharEncoding.UTF_8); - this.fileName = "Test-Item.txt"; - this.mimeType = "text/plain"; - this.lastModified = new Date().getTime(); - this.length = content.getBytes().length; - this.checksum = "testsum"; - - this.request = mock(HttpServletRequest.class); - this.response = new MockHttpServletResponse(); - - //Using wrappers so we can save the content of the bodies and use them for tests - this.requestWrapper = new ContentCachingRequestWrapper(request); - this.responseWrapper = new ContentCachingResponseWrapper(response); - } catch (IOException ex) { - log.error("IO Error in init", ex); - fail("SQL Error in init: " + ex.getMessage()); - } - } - - /** - * This method will be run after every test as per @After. It will - * clean resources initialized by the @Before methods. - *

- * Other methods can be annotated with @After here or in subclasses - * but no execution order is guaranteed - */ - @After - public void destroy() { - try { - is.close(); - } catch (IOException e) { - e.printStackTrace(); - } - } - - - /** - * Test if Range header is supported and gives back the right range - * - * @throws Exception - */ - @Test - public void testRangeHeader() throws Exception { - MultipartFileSender multipartFileSender = MultipartFileSender - .fromInputStream(is) - .with(requestWrapper) - .with(responseWrapper) - .withFileName(fileName) - .withChecksum(checksum) - .withMimetype(mimeType) - .withLength(length); - - when(request.getHeader(eq("If-Range"))).thenReturn("not_file_to_serve.txt"); - when(request.getHeader(eq("Range"))).thenReturn("bytes=1-3"); - - multipartFileSender.serveResource(); - - String content = new String(responseWrapper.getContentAsByteArray(), CharEncoding.UTF_8); - - assertEquals("123", content); - } - - /** - * Test if we can just request the full file without ranges - * - * @throws Exception - */ - @Test - public void testFullFileReturn() throws Exception { - MultipartFileSender multipartFileSender = MultipartFileSender - .fromInputStream(is) - .with(requestWrapper) - .with(responseWrapper) - .withFileName(fileName) - .withChecksum(checksum) - .withMimetype(mimeType) - .withLength(length); - - multipartFileSender.serveResource(); - - String content = new String(responseWrapper.getContentAsByteArray(), CharEncoding.UTF_8); - - assertEquals("0123456789", content); - assertEquals(checksum, responseWrapper.getHeader("ETag")); - } - - /** - * Test for support of Open ranges - * - * @throws Exception - */ - @Test - public void testOpenRange() throws Exception { - MultipartFileSender multipartFileSender = MultipartFileSender - .fromInputStream(is) - .with(requestWrapper) - .with(responseWrapper) - .withFileName(fileName) - .withChecksum(checksum) - .withMimetype(mimeType) - .withLength(length); - - when(request.getHeader(eq("Range"))).thenReturn("bytes=5-"); - - multipartFileSender.serveResource(); - - String content = new String(responseWrapper.getContentAsByteArray(), CharEncoding.UTF_8); - - assertEquals("56789", content); - } - - /** - * Test support for multiple ranges - * - * @throws Exception - */ - @Test - public void testMultipleRanges() throws Exception { - MultipartFileSender multipartFileSender = MultipartFileSender - .fromInputStream(is) - .with(requestWrapper) - .with(responseWrapper) - .withFileName(fileName) - .withChecksum(checksum) - .withMimetype(mimeType) - .withLength(length); - - when(request.getHeader(eq("Range"))).thenReturn("bytes=1-2,3-4,5-9"); - - multipartFileSender.serveResource(); - - String content = new String(responseWrapper.getContentAsByteArray(), CharEncoding.UTF_8); - - assertEquals("--MULTIPART_BYTERANGES" + - "Content-Type: text/plain" + - "Content-Range: bytes 1-2/10" + - "12" + - "--MULTIPART_BYTERANGES" + - "Content-Type: text/plain" + - "Content-Range: bytes 3-4/10" + - "34" + - "--MULTIPART_BYTERANGES" + - "Content-Type: text/plain" + - "Content-Range: bytes 5-9/10" + - "56789" + - "--MULTIPART_BYTERANGES--".replace("\n", "").replace("\r", "") - , content.replace("\n", "").replace("\r", "") - ); - - } - - /** - * Test with a unvalid Range header, should return status 416 - * - * @throws Exception - */ - @Test - public void testInvalidRange() throws Exception { - MultipartFileSender multipartFileSender = MultipartFileSender - .fromInputStream(is) - .with(requestWrapper) - .with(responseWrapper) - .withFileName(fileName) - .withChecksum(checksum) - .withMimetype(mimeType) - .withLength(length); - - when(request.getHeader(eq("Range"))).thenReturn("bytes=invalid"); - - multipartFileSender.serveResource(); - - assertEquals(HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE, responseWrapper.getStatusCode()); - } - - /** - * Test if the ETAG is in the response header - * - * @throws Exception - */ - @Test - public void testEtagInResponse() throws Exception { - MultipartFileSender multipartFileSender = MultipartFileSender - .fromInputStream(is) - .with(requestWrapper) - .with(responseWrapper) - .withFileName(fileName) - .withChecksum(checksum) - .withMimetype(mimeType) - .withLength(length); - - - when(request.getHeader(eq("Range"))).thenReturn("bytes=1-3"); - - multipartFileSender.serveResource(); - - String etag = responseWrapper.getHeader("Etag"); - - assertEquals(checksum, etag); - } - - //Check that a head request doesn't return any body, but returns the headers - @Test - public void testHeadRequest() throws Exception { - MultipartFileSender multipartFileSender = MultipartFileSender - .fromInputStream(is) - .with(requestWrapper) - .with(responseWrapper) - .withFileName(fileName) - .withChecksum(checksum) - .withMimetype(mimeType) - .withLength(length); - - - when(request.getMethod()).thenReturn("HEAD"); - - multipartFileSender.serveResource(); - - String content = new String(responseWrapper.getContentAsByteArray(), CharEncoding.UTF_8); - - assertEquals("bytes", responseWrapper.getHeader("Accept-Ranges")); - assertEquals(checksum, responseWrapper.getHeader("ETag")); - assertEquals("", content); - assertEquals(200, responseWrapper.getStatusCode()); - - } - - /** - * If ETAG is equal to that of the requested Resource then this should return 304 - * - * @throws Exception - */ - @Test - public void testIfNoneMatchFail() throws Exception { - MultipartFileSender multipartFileSender = MultipartFileSender - .fromInputStream(is) - .with(requestWrapper) - .with(responseWrapper) - .withFileName(fileName) - .withChecksum(checksum) - .withMimetype(mimeType) - .withLength(length); - - when(request.getHeader(eq("If-None-Match"))).thenReturn(checksum); - - multipartFileSender.isValid(); - - assertEquals(HttpServletResponse.SC_NOT_MODIFIED, responseWrapper.getStatusCode()); - } - - /** - * Happy path of If-None-Match header - * - * @throws Exception - */ - @Test - public void testIfNoneMatchPass() throws Exception { - MultipartFileSender multipartFileSender = MultipartFileSender - .fromInputStream(is) - .with(requestWrapper) - .with(responseWrapper) - .withFileName(fileName) - .withChecksum(checksum) - .withMimetype(mimeType) - .withLength(length); - - - when(request.getHeader(eq("If-None-Match"))) - .thenReturn("pretendthisisarandomchecksumnotequaltotherequestedbitstream"); - - multipartFileSender.isValid(); - multipartFileSender.serveResource(); - - assertEquals(HttpServletResponse.SC_OK, responseWrapper.getStatusCode()); - } - - /** - * If the bitstream has no filename this should throw an internal server error - * - * @throws Exception - */ - @Test - public void testNoFileName() throws Exception { - MultipartFileSender multipartFileSender = MultipartFileSender - .fromInputStream(is) - .with(requestWrapper) - .with(responseWrapper) - .withChecksum(checksum) - .withMimetype(mimeType) - .withLength(length); - - - multipartFileSender.isValid(); - - - assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, responseWrapper.getStatusCode()); - } - - /** - * Test if the Modified Since precondition works, should return 304 if it hasn't been modified - * - * @throws Exception - */ - @Test - public void testIfModifiedSinceNotModifiedSince() throws Exception { - Long time = new Date().getTime(); - MultipartFileSender multipartFileSender = MultipartFileSender - .fromInputStream(is) - .with(requestWrapper) - .withFileName(fileName) - .with(responseWrapper) - .withChecksum(checksum) - .withMimetype(mimeType) - .withLength(length) - .withLastModified(time); - - when(request.getDateHeader(eq("If-Modified-Since"))).thenReturn(time + 100000); - when(request.getDateHeader(eq("If-Unmodified-Since"))).thenReturn(-1L); - - multipartFileSender.isValid(); - - assertEquals(HttpServletResponse.SC_NOT_MODIFIED, responseWrapper.getStatusCode()); - - - } - - /** - * Happy path for modified since - * - * @throws Exception - */ - @Test - public void testIfModifiedSinceModifiedSince() throws Exception { - Long time = new Date().getTime(); - MultipartFileSender multipartFileSender = MultipartFileSender - .fromInputStream(is) - .with(requestWrapper) - .withFileName(fileName) - .with(responseWrapper) - .withChecksum(checksum) - .withMimetype(mimeType) - .withLength(length) - .withLastModified(time); - - when(request.getDateHeader(eq("If-Modified-Since"))).thenReturn(time - 100000); - when(request.getDateHeader(eq("If-Unmodified-Since"))).thenReturn(-1L); - - multipartFileSender.isValid(); - multipartFileSender.serveResource(); - - assertEquals(HttpServletResponse.SC_OK, responseWrapper.getStatusCode()); - - } - - /** - * If the If-Match doesn't match the ETAG then return 416 Status code - * - * @throws Exception - */ - @Test - public void testIfMatchNoMatch() throws Exception { - Long time = new Date().getTime(); - MultipartFileSender multipartFileSender = MultipartFileSender - .fromInputStream(is) - .with(requestWrapper) - .withFileName(fileName) - .with(responseWrapper) - .withChecksum(checksum) - .withMimetype(mimeType) - .withLength(length) - .withLastModified(time); - - when(request.getDateHeader(eq("If-Modified-Since"))).thenReturn(-1L); - when(request.getDateHeader(eq("If-Unmodified-Since"))).thenReturn(-1L); - when(request.getHeader(eq("If-Match"))).thenReturn("None-Matching-ETAG"); - - multipartFileSender.isValid(); - - assertEquals(HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE, responseWrapper.getStatusCode()); - } - - /** - * If matches then just return resource - * - * @throws Exception - */ - @Test - public void testIfMatchMatch() throws Exception { - Long time = new Date().getTime(); - MultipartFileSender multipartFileSender = MultipartFileSender - .fromInputStream(is) - .with(requestWrapper) - .withFileName(fileName) - .with(responseWrapper) - .withChecksum(checksum) - .withMimetype(mimeType) - .withLength(length) - .withLastModified(time); - - when(request.getDateHeader(eq("If-Modified-Since"))).thenReturn(-1L); - when(request.getDateHeader(eq("If-Unmodified-Since"))).thenReturn(-1L); - when(request.getHeader(eq("If-Match"))).thenReturn(checksum); - - multipartFileSender.isValid(); - - assertEquals(HttpServletResponse.SC_OK, responseWrapper.getStatusCode()); - } - - /** - * If not modified since given date then return resource - * - * @throws Exception - */ - @Test - public void testIfUnmodifiedSinceNotModifiedSince() throws Exception { - Long time = new Date().getTime(); - MultipartFileSender multipartFileSender = MultipartFileSender - .fromInputStream(is) - .with(requestWrapper) - .withFileName(fileName) - .with(responseWrapper) - .withChecksum(checksum) - .withMimetype(mimeType) - .withLength(length) - .withLastModified(time); - - when(request.getDateHeader(eq("If-Unmodified-Since"))).thenReturn(time + 100000); - when(request.getDateHeader(eq("If-Modified-Since"))).thenReturn(-1L); - - multipartFileSender.isValid(); - - assertEquals(HttpServletResponse.SC_OK, responseWrapper.getStatusCode()); - - } - - /** - * If modified since given date then return 412 - * - * @throws Exception - */ - @Test - public void testIfUnmodifiedSinceModifiedSince() throws Exception { - Long time = new Date().getTime(); - MultipartFileSender multipartFileSender = MultipartFileSender - .fromInputStream(is) - .with(requestWrapper) - .withFileName(fileName) - .with(responseWrapper) - .withChecksum(checksum) - .withMimetype(mimeType) - .withLength(length) - .withLastModified(time); - - when(request.getDateHeader(eq("If-Unmodified-Since"))).thenReturn(time - 100000); - when(request.getDateHeader(eq("If-Modified-Since"))).thenReturn(-1L); - - multipartFileSender.isValid(); - - assertEquals(HttpServletResponse.SC_PRECONDITION_FAILED, responseWrapper.getStatusCode()); - - } - - -} From c4fe9e667453a1edf7acff713b50da7a6aeb59eb Mon Sep 17 00:00:00 2001 From: Raf Ponsaerts Date: Fri, 6 Nov 2020 16:39:31 +0100 Subject: [PATCH 2/4] [Task 73179] added javadocs and cleaned up where necessary --- .../app/rest/utils/BitstreamResource.java | 6 +++++ .../rest/utils/HttpHeadersInitializer.java | 26 +++++++++++++------ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java index b1b6e463c2d9..d338ddfaf777 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java @@ -13,6 +13,12 @@ import org.springframework.core.io.AbstractResource; +/** + * This class acts as a {@link AbstractResource} used by Spring's framework to send the data in a proper and + * streamlined way inside the {@link org.springframework.http.ResponseEntity} body. + * This class' attributes are being used by Spring's framework in the overridden methods so that the proper + * attributes are given and used in the response. + */ public class BitstreamResource extends AbstractResource { private InputStream inputStream; diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java index 63498c4b20b7..108194acc8ed 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java @@ -25,12 +25,10 @@ import org.springframework.http.HttpHeaders; /** - * Utility class to send an input stream with Range header and ETag support. - * Based on https://github.com/davinkevin/Podcast-Server/blob/v1.0.0/src/main/java/lan/dk/podcastserver/service - * /MultiPartFileSenderService.java - * - * @author Tom Desair (tom dot desair at atmire dot com) - * @author Frederic Van Reet (frederic dot vanreet at atmire dot com) + * This class takes data from the Bitstream/File that has to be send. It'll then digest this input and save it in + * its local variables. + * When calling {{@link #initialiseHeaders()}}, the input and information will be used to set the proper headers + * with this info and return an Object of {@link HttpHeaders} to be used in the response that'll be generated */ public class HttpHeadersInitializer { @@ -132,7 +130,12 @@ public HttpHeadersInitializer withDisposition(String contentDisposition) { return this; } - //TODO rename to initialiseHeaders + /** + * This method will be called to create a {@link HttpHeaders} object which will contain the headers needed + * to form a proper response when returning the Bitstream/File + * @return A {@link HttpHeaders} object containing the information for the Bitstream/File to be sent + * @throws IOException If something goes wrong + */ public HttpHeaders initialiseHeaders() throws IOException { HttpHeaders httpHeaders = new HttpHeaders(); @@ -140,7 +143,6 @@ public HttpHeaders initialiseHeaders() throws IOException { log.debug("Content-Type : {}", contentType); // Initialize response. - //TODO response.reset => Can be re-instated if we bump to 5.2.9 response.setBufferSize(bufferSize); if (contentType != null) { httpHeaders.put(CONTENT_TYPE, Collections.singletonList(contentType)); @@ -182,6 +184,14 @@ public HttpHeaders initialiseHeaders() throws IOException { } + /** + * This method will validate whether or not the given Response/Request/Information/Variables are valid. + * If they're invalid, the Response shouldn't be given. + * This will do null checks on the response, request, inputstream and filename. + * Other than this, it'll check Request headers to see if their information is correct. + * @return + * @throws IOException + */ public boolean isValid() throws IOException { if (response == null || request == null) { return false; From 6dd048c68eb9029b4de3d8335e39a4837eda139e Mon Sep 17 00:00:00 2001 From: jonas-atmire Date: Wed, 18 Nov 2020 10:08:12 +0100 Subject: [PATCH 3/4] Re-enabling of response.reset() method -> https://github.com/DSpace/DSpace/pull/3009#discussion_r525254104 --- .../java/org/dspace/app/rest/utils/HttpHeadersInitializer.java | 1 + 1 file changed, 1 insertion(+) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java index 108194acc8ed..993ed26bcf11 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java @@ -142,6 +142,7 @@ public HttpHeaders initialiseHeaders() throws IOException { // Validate and process range ------------------------------------------------------------- log.debug("Content-Type : {}", contentType); + response.reset(); // Initialize response. response.setBufferSize(bufferSize); if (contentType != null) { From 50703a0771751c7818f6367234f41fccf34a3aac Mon Sep 17 00:00:00 2001 From: jonas-atmire Date: Thu, 19 Nov 2020 08:01:42 +0100 Subject: [PATCH 4/4] Reverting previous commit + clarification comment --- .../java/org/dspace/app/rest/utils/HttpHeadersInitializer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java index 993ed26bcf11..e5f3a9365cb2 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java @@ -142,7 +142,7 @@ public HttpHeaders initialiseHeaders() throws IOException { // Validate and process range ------------------------------------------------------------- log.debug("Content-Type : {}", contentType); - response.reset(); + //TODO response.reset() => Can be re-instated/investigated once we upgrade to Spring 5.2.9, see issue #3056 // Initialize response. response.setBufferSize(bufferSize); if (contentType != null) {