From 7e49aa5e03bf296f75ebe82fd96f7775efe83978 Mon Sep 17 00:00:00 2001 From: Fabian Engelniederhammer Date: Thu, 15 Feb 2024 15:59:25 +0100 Subject: [PATCH] fix: don't set Transfer-Encoding twice #600 Some proxies such as Traefik will throw an error when the Transfer-Encoding header is present twice. We shouldn't set the header ourselves, but leave that to Tomcat (which also makes sure that the chunking is done correctly). Instead, prevent setting a content length when the response is compressed. --- .../lapis/controller/CompressionFilter.kt | 51 ++++++++++++++++--- .../LapisControllerCompressionTest.kt | 2 + 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/CompressionFilter.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/CompressionFilter.kt index 2d4e1ab5..6e2ce987 100644 --- a/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/CompressionFilter.kt +++ b/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/CompressionFilter.kt @@ -10,13 +10,19 @@ import jakarta.servlet.http.HttpServletResponse import mu.KotlinLogging import org.genspectrum.lapis.util.CachedBodyHttpServletRequest import org.genspectrum.lapis.util.HeaderModifyingRequestWrapper +import org.springframework.boot.context.properties.bind.Binder +import org.springframework.boot.web.servlet.server.Encoding import org.springframework.core.annotation.Order +import org.springframework.core.env.Environment import org.springframework.http.HttpHeaders.ACCEPT_ENCODING import org.springframework.http.HttpHeaders.CONTENT_ENCODING -import org.springframework.http.HttpHeaders.TRANSFER_ENCODING +import org.springframework.http.MediaType +import org.springframework.http.converter.StringHttpMessageConverter import org.springframework.stereotype.Component +import org.springframework.web.context.annotation.RequestScope import org.springframework.web.filter.OncePerRequestFilter import java.io.OutputStream +import java.nio.charset.Charset import java.util.Enumeration import java.util.zip.GZIPOutputStream @@ -50,9 +56,14 @@ fun ZstdOutputStream.commitUnderlyingResponseToPreventContentLengthFromBeingSet( write(nothing) } +@Component +@RequestScope +class RequestCompression(var compression: Compression? = null) + @Component @Order(DOWNLOAD_AS_FILE_FILTER_ORDER - 1) -class CompressionFilter(val objectMapper: ObjectMapper) : OncePerRequestFilter() { +class CompressionFilter(val objectMapper: ObjectMapper, val requestCompression: RequestCompression) : + OncePerRequestFilter() { override fun doFilterInternal( request: HttpServletRequest, response: HttpServletResponse, @@ -85,7 +96,10 @@ class CompressionFilter(val objectMapper: ObjectMapper) : OncePerRequestFilter() acceptEncodingHeaders: Enumeration?, ) = when (val compression = Compression.fromHeaders(acceptEncodingHeaders)) { null -> response - else -> CompressingResponse(response, compression) + else -> { + requestCompression.compression = compression + CompressingResponse(response, compression) + } } } @@ -103,11 +117,6 @@ class CompressingResponse( init { log.info { "Compressing using $compression" } response.setHeader(CONTENT_ENCODING, compression.value) - preventSpringFromSettingTheContentLengthWhichIsUnknownWhenCompressing(response) - } - - private fun preventSpringFromSettingTheContentLengthWhichIsUnknownWhenCompressing(response: HttpServletResponse) { - response.addHeader(TRANSFER_ENCODING, "chunked") } private val servletOutputStream = CompressingServletOutputStream(response.outputStream, compression) @@ -139,3 +148,29 @@ class CompressingServletOutputStream( compressingStream.flush() } } + +@Component +class StringHttpMessageConverterWithUnknownContentLengthInCaseOfCompression( + environment: Environment, + val requestCompression: RequestCompression, +) : StringHttpMessageConverter(getCharsetFromEnvironment(environment)) { + override fun getContentLength( + str: String, + contentType: MediaType?, + ): Long? { + return when (requestCompression.compression) { + null -> super.getContentLength(str, contentType) + else -> null + } + } + + companion object { + // taken from the initialization in + // org.springframework.boot.autoconfigure.http.HttpMessageConvertersAutoConfiguration + // since this class replaces that one + private fun getCharsetFromEnvironment(environment: Environment): Charset = + Binder.get(environment) + .bindOrCreate("server.servlet.encoding", Encoding::class.java) + .charset + } +} diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerCompressionTest.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerCompressionTest.kt index aa22809e..d7ba550e 100644 --- a/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerCompressionTest.kt +++ b/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerCompressionTest.kt @@ -13,6 +13,7 @@ import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMock import org.springframework.boot.test.context.SpringBootTest import org.springframework.http.HttpHeaders.ACCEPT_ENCODING import org.springframework.http.HttpHeaders.CONTENT_ENCODING +import org.springframework.http.HttpHeaders.CONTENT_LENGTH import org.springframework.http.MediaType.APPLICATION_JSON import org.springframework.http.MediaType.APPLICATION_JSON_VALUE import org.springframework.test.web.servlet.MockMvc @@ -46,6 +47,7 @@ class LapisControllerCompressionTest( val content = mockMvc.perform(requestsScenario.request) .andExpect(status().isOk) .andExpect(content().contentType(requestsScenario.expectedContentType)) + .andExpect(header().doesNotExist(CONTENT_LENGTH)) .andExpect(header().string(CONTENT_ENCODING, requestsScenario.compressionFormat)) .andReturn() .response