Skip to content

Commit

Permalink
fix: don't set Transfer-Encoding twice #600
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fengelniederhammer authored and JonasKellerer committed Feb 15, 2024
1 parent 107396d commit 7e49aa5
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -85,7 +96,10 @@ class CompressionFilter(val objectMapper: ObjectMapper) : OncePerRequestFilter()
acceptEncodingHeaders: Enumeration<String>?,
) = when (val compression = Compression.fromHeaders(acceptEncodingHeaders)) {
null -> response
else -> CompressingResponse(response, compression)
else -> {
requestCompression.compression = compression
CompressingResponse(response, compression)
}
}
}

Expand All @@ -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)
Expand Down Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 7e49aa5

Please sign in to comment.