fix: file ending when downloading compressed file #685

also fix that LAPIS now correctly respects multiple, comma-separated values in the accept-encoding header.

lapis2-docs/src/content/docs/references/additional-request-properties.mdx

@@ -92,7 +92,16 @@ GET /sample/aggregated?downloadAsFile=true
 ## Compression
 
 LAPIS supports gzip and Zstd compression.
-You can request compressed data via the `Accept-Encoding` header.
+You can request compressed data by setting the `compression` property in the request.
+Refer to the Swagger UI for allowed values.
+
+```http
+GET /sample/aggregated?compression=gzip
+```
+
+:::note
+
+Alternatively, you can set the `Accept-Encoding` header.
 
 ```http
 GET /sample/aggregated
@@ -106,12 +115,11 @@ Accept-Encoding: zstd
 
 LAPIS will set the `Content-Encoding` header in the response to indicate the compression used.
 
-:::note
-Alternatively, you can use the `compression` property in the request.
-Refer to the Swagger UI for allowed values.
-
-```http
-GET /sample/aggregated?compression=gzip
-```
+:::caution
+If you want to download compressed data via the `downloadAsFile` property,
+then you need to specify the compression type via the `compression` property.
 
+LAPIS will ignore the `Accept-Encoding` header, if `downloadAsFile == true`,
+since browsers always accept GZIP encoding.
+Then, you would not be able to download uncompressed data in a browser.
 :::

lapis2/src/main/kotlin/org/genspectrum/lapis/controller/CompressionFilter.kt

@@ -36,18 +36,21 @@ enum class Compression(
     val value: String,
     val contentType: MediaType,
     val compressionOutputStreamFactory: (OutputStream) -> OutputStream,
+    val fileEnding: String,
 ) {
     GZIP(
         value = "gzip",
         contentType = MediaType.parseMediaType("application/gzip"),
         compressionOutputStreamFactory = ::LazyGzipOutputStream,
+        fileEnding = ".gz",
     ),
     ZSTD(
         value = "zstd",
         contentType = MediaType.parseMediaType("application/zstd"),
         compressionOutputStreamFactory = {
             ZstdOutputStream(it).apply { commitUnderlyingResponseToPreventContentLengthFromBeingSet() }
         },
+        fileEnding = ".zstd",
     ),
     ;
 
@@ -58,6 +61,8 @@ enum class Compression(
             }
 
             val headersList = acceptEncodingHeaders.toList()
+                .flatMap { it.split(',') }
+                .map { it.trim() }
 
             return when {
                 headersList.contains(GZIP.value) -> GZIP
@@ -140,7 +145,7 @@ class CompressionFilter(val objectMapper: ObjectMapper, val requestCompression:
 
         val maybeCompressingResponse = createMaybeCompressingResponse(
             response,
-            reReadableRequest.getHeaders(ACCEPT_ENCODING),
+            reReadableRequest,
             compressionPropertyInRequest,
         )
 
@@ -162,9 +167,11 @@ class CompressionFilter(val objectMapper: ObjectMapper, val requestCompression:
 
     private fun createMaybeCompressingResponse(
         response: HttpServletResponse,
-        acceptEncodingHeaders: Enumeration<String>?,
+        reReadableRequest: CachedBodyHttpServletRequest,
         compressionPropertyInRequest: Compression?,
     ): HttpServletResponse {
+        val acceptEncodingHeaders = reReadableRequest.getHeaders(ACCEPT_ENCODING)
+
         if (compressionPropertyInRequest != null) {
             log.info { "Compressing using $compressionPropertyInRequest from request property" }
 
@@ -175,6 +182,11 @@ class CompressionFilter(val objectMapper: ObjectMapper, val requestCompression:
             )
         }
 
+        val downloadAsFile = reReadableRequest.getBooleanField(DOWNLOAD_AS_FILE_PROPERTY) ?: false
+        if (downloadAsFile) {
+            return response
+        }
+
         val compression = Compression.fromHeaders(acceptEncodingHeaders) ?: return response
 
         log.info { "Compressing using $compression from $ACCEPT_ENCODING header" }

lapis2/src/main/kotlin/org/genspectrum/lapis/controller/DownloadAsFileFilter.kt

@@ -9,14 +9,16 @@ import org.genspectrum.lapis.controller.LapisMediaType.TEXT_TSV
 import org.genspectrum.lapis.util.CachedBodyHttpServletRequest
 import org.springframework.core.annotation.Order
 import org.springframework.http.HttpHeaders.ACCEPT
-import org.springframework.http.HttpHeaders.ACCEPT_ENCODING
 import org.springframework.http.HttpHeaders.CONTENT_DISPOSITION
 import org.springframework.stereotype.Component
 import org.springframework.web.filter.OncePerRequestFilter
 
 @Component
 @Order(DOWNLOAD_AS_FILE_FILTER_ORDER)
-class DownloadAsFileFilter(private val objectMapper: ObjectMapper) : OncePerRequestFilter() {
+class DownloadAsFileFilter(
+    private val objectMapper: ObjectMapper,
+    private val requestCompression: RequestCompression,
+) : OncePerRequestFilter() {
     override fun doFilterInternal(
         request: HttpServletRequest,
         response: HttpServletResponse,
@@ -37,10 +39,9 @@ class DownloadAsFileFilter(private val objectMapper: ObjectMapper) : OncePerRequ
             SampleRoute.entries.find { request.getProxyAwarePath().startsWith("/sample${it.pathSegment}") }
         val dataName = matchingRoute?.pathSegment?.trim('/') ?: "data"
 
-        val compressionEnding = when (Compression.fromHeaders(request.getHeaders(ACCEPT_ENCODING))) {
-            Compression.GZIP -> ".gzip"
-            Compression.ZSTD -> ".zstd"
-            null -> ""
+        val compressionEnding = when (val compressionSource = requestCompression.compressionSource) {
+            is CompressionSource.RequestProperty -> compressionSource.compression.fileEnding
+            else -> ""
         }
 
         val fileEnding = when (request.getHeader(ACCEPT)) {

lapis2/src/main/kotlin/org/genspectrum/lapis/controller/ExceptionHandler.kt

@@ -1,49 +1,34 @@
 package org.genspectrum.lapis.controller
 
 import mu.KotlinLogging
+import org.genspectrum.lapis.config.DatabaseConfig
 import org.genspectrum.lapis.model.SiloNotImplementedError
 import org.genspectrum.lapis.silo.SiloException
 import org.genspectrum.lapis.silo.SiloUnavailableException
-import org.springframework.core.annotation.Order
+import org.springframework.http.HttpHeaders
+import org.springframework.http.HttpHeaders.ACCEPT
 import org.springframework.http.HttpHeaders.RETRY_AFTER
 import org.springframework.http.HttpStatus
 import org.springframework.http.HttpStatusCode
 import org.springframework.http.MediaType
 import org.springframework.http.ProblemDetail
 import org.springframework.http.ResponseEntity
 import org.springframework.http.ResponseEntity.BodyBuilder
+import org.springframework.stereotype.Component
 import org.springframework.web.bind.annotation.ControllerAdvice
 import org.springframework.web.bind.annotation.ExceptionHandler
 import org.springframework.web.bind.annotation.ResponseStatus
+import org.springframework.web.context.request.WebRequest
 import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler
 import org.springframework.web.servlet.resource.NoResourceFoundException
+import org.springframework.web.servlet.support.ServletUriComponentsBuilder
 
 private val log = KotlinLogging.logger {}
 
 private typealias ErrorResponse = ResponseEntity<LapisErrorResponse>
 
-/**
- * Taken from https://github.com/spring-projects/spring-framework/issues/31569#issuecomment-1825444419
- * Due to https://github.com/spring-projects/spring-framework/commit/c00508d6cf2408d06a0447ed193ad96466d0d7b4
- *
- * This forwards "404" errors to the ErrorController to allow it to return a view.
- * Thus, browsers get their own error page.
- *
- * Spring reworked handling of "404 not found" errors. This was introduced with the upgrade to Spring boot 3.2.0.
- * This can be removed/reworked once Spring decides on how to return a view from an ExceptionHandler
- * or allows them to respect the Accept header.
- */
 @ControllerAdvice
-@Order(-1)
-internal class ExceptionToErrorControllerBypass {
-    @ExceptionHandler(NoResourceFoundException::class)
-    fun handleResourceNotFound(e: Exception): Nothing {
-        throw e
-    }
-}
-
-@ControllerAdvice
-class ExceptionHandler : ResponseEntityExceptionHandler() {
+class ExceptionHandler(private val notFoundView: NotFoundView) : ResponseEntityExceptionHandler() {
     @ExceptionHandler(Throwable::class)
     @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
     fun handleUnexpectedException(e: Throwable): ErrorResponse {
@@ -89,6 +74,23 @@ class ExceptionHandler : ResponseEntityExceptionHandler() {
         return responseEntity(HttpStatus.SERVICE_UNAVAILABLE, e.message) { header(RETRY_AFTER, e.retryAfter) }
     }
 
+    override fun handleNoResourceFoundException(
+        ex: NoResourceFoundException,
+        headers: HttpHeaders,
+        status: HttpStatusCode,
+        request: WebRequest,
+    ): ResponseEntity<Any>? {
+        val acceptMediaTypes = MediaType.parseMediaTypes(request.getHeaderValues(ACCEPT)?.toList())
+        if (MediaType.TEXT_HTML.isPresentIn(acceptMediaTypes)) {
+            return ResponseEntity
+                .status(HttpStatus.NOT_FOUND)
+                .contentType(MediaType.TEXT_HTML)
+                .body(notFoundView.render())
+        }
+
+        return super.handleNoResourceFoundException(ex, headers, status, request)
+    }
+
     private fun responseEntity(
         httpStatus: HttpStatus,
         detail: String?,
@@ -123,6 +125,32 @@ class ExceptionHandler : ResponseEntityExceptionHandler() {
     }
 }
 
+@Component
+class NotFoundView(private val databaseConfig: DatabaseConfig) {
+    fun render(): String {
+        val uri = ServletUriComponentsBuilder.fromCurrentRequest()
+            .replacePath("/swagger-ui/index.html")
+            .replaceQuery(null)
+            .fragment(null)
+            .toUriString()
+
+        return """
+            <!DOCTYPE html>
+            <html lang="en">
+            <head>
+                <meta charset="UTF-8">
+                <title>Error 404</title>
+            </head>
+            <body>
+                <h1>LAPIS - ${databaseConfig.schema.instanceName}</h1>
+                <h3>Page not found!</h3>
+                <a href="$uri">Visit our swagger-ui</a>
+            </body>
+            </html>
+            """.trimIndent()
+    }
+}
+
 /** This is not yet actually thrown, but makes "403 Forbidden" appear in OpenAPI docs. */
 class AddForbiddenToOpenApiDocsHelper(message: String) : Exception(message)