gzip server responses break JS client

[dobe]

Summary

gzip compressed responses currently break the apollo client on the js target, jvm works

it looks like the client uses the content-length of the response and truncates the decompressed bytes at this offset instead of consuming all the decompressed bytes.

background might be as described here https://slack-chats.kotlinlang.org/t/545061/i-think-i-ve-discovered-a-bug-with-gzip-client-handling-and-

Version

3.7.1

Steps to reproduce the behavior

do a graphql request to a server which has gzip encoding on, this results in

JsonEncodingException: Unterminated string at ...

There is also a bug in ktor https://youtrack.jetbrains.com/issue/KTOR-4653

however when we force the ktor version to 2.1.3, there is still another error from apollo client, which might somehow relate to this.

ApolloNetworkException: Expected 2161, actual 5899

[martinbonnin]

Thanks for sending this 🙏 .

I bumped the ktor version and added a test in #4554 and everything seems to be good 🤞 . Any chance you can try with your project? (See CONTRIBUTING.md for how to use a local version of Apollo Kotlin)

The ApolloNetworkException: Expected 2161, actual 5899 is a wrapped exception from a ktor one so there a chance that bumping everything made the trick. If not, if you could share your payload, either here or martin at apollographql.com, that'd be super helpful!

Thanks again!

[martinbonnin]

Hi @dobe, this is now in version 3.7.2, let us know how it works.

[dobe]

hi @martinbonnin, thx for the quick response

however in the browser test it still fails on my side, i've upgraded to 3.7.2 i've also ensured that i have all ktor deps on v 2.1.3, here is the output of my test, however i guess it will not help a lot:

testClient[js, browser, ChromeHeadless108.0.5359.94, MacOS10.15.7]
ApolloNetworkException: Expected 3901, actual 12500
    at 16.doResume_5yljmg(/var/folders/j3/r2svh6b51t582417f69xmbf80000gn/T/_karma_webpack_392871/commons.js:66153)
    at CoroutineImpl.resumeWith_7onugl(/var/folders/j3/r2svh6b51t582417f69xmbf80000gn/T/_karma_webpack_392871/commons.js:99356)
    at CoroutineImpl.resumeWith_s3a3yh(/var/folders/j3/r2svh6b51t582417f69xmbf80000gn/T/_karma_webpack_392871/commons.js:99402)
    at <global>.resumeRootWith(/var/folders/j3/r2svh6b51t582417f69xmbf80000gn/T/_karma_webpack_392871/commons.js:155853)
    at 1.resumeWith_k14j9i(/var/folders/j3/r2svh6b51t582417f69xmbf80000gn/T/_karma_webpack_392871/commons.js:155905)
    at 1.resumeWith_s3a3yh(/var/folders/j3/r2svh6b51t582417f69xmbf80000gn/T/_karma_webpack_392871/commons.js:155911)
    at CoroutineImpl.resumeWith_7onugl(/var/folders/j3/r2svh6b51t582417f69xmbf80000gn/T/_karma_webpack_392871/commons.js:99383)
    at CoroutineImpl.resumeWith_s3a3yh(/var/folders/j3/r2svh6b51t582417f69xmbf80000gn/T/_karma_webpack_392871/commons.js:99402)
    at <global>.resumeRootWith(/var/folders/j3/r2svh6b51t582417f69xmbf80000gn/T/_karma_webpack_392871/commons.js:155853)
    at 1.resumeWith_k14j9i(/var/folders/j3/r2svh6b51t582417f69xmbf80000gn/T/_karma_webpack_392871/commons.js:155905)

here is the dependecy tree of the apollo runtime in the js target

+--- com.apollographql.apollo3:apollo-runtime:3.7.2
|    \--- com.apollographql.apollo3:apollo-runtime-js:3.7.2
|         +--- io.ktor:ktor-client-js:2.1.3
|         |    \--- io.ktor:ktor-client-js-js:2.1.3
|         |         +--- io.ktor:ktor-client-core:2.1.3
|         |         |    \--- io.ktor:ktor-client-core-js:2.1.3
|         |         |         +--- org.jetbrains.kotlin:kotlin-stdlib-js:1.6.21 -> 1.7.20
|         |         |         +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.4 (*)
|         |         |         +--- io.ktor:ktor-http:2.1.3
|         |         |         |    \--- io.ktor:ktor-http-js:2.1.3
|         |         |         |         +--- org.jetbrains.kotlin:kotlin-stdlib-js:1.6.21 -> 1.7.20
|         |         |         |         +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.4 (*)
|         |         |         |         +--- io.ktor:ktor-utils:2.1.3
|         |         |         |         |    \--- io.ktor:ktor-utils-js:2.1.3
|         |         |         |         |         +--- org.jetbrains.kotlin:kotlin-stdlib-js:1.6.21 -> 1.7.20
|         |         |         |         |         +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.4 (*)
|         |         |         |         |         +--- io.ktor:ktor-io:2.1.3
|         |         |         |         |         |    \--- io.ktor:ktor-io-js:2.1.3
|         |         |         |         |         |         +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.4 (*)
|         |         |         |         |         |         +--- org.jetbrains.kotlin:kotlin-stdlib-js:1.6.21 -> 1.7.20
|         |         |         |         |         |         \--- org.jetbrains.kotlin:atomicfu:1.6.21
|         |         |         |         |         \--- org.jetbrains.kotlin:atomicfu:1.6.21
|         |         |         |         \--- org.jetbrains.kotlin:atomicfu:1.6.21
|         |         |         +--- io.ktor:ktor-events:2.1.3
|         |         |         |    \--- io.ktor:ktor-events-js:2.1.3
|         |         |         |         +--- org.jetbrains.kotlin:kotlin-stdlib-js:1.6.21 -> 1.7.20
|         |         |         |         +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.4 (*)
|         |         |         |         +--- io.ktor:ktor-http:2.1.3 (*)
|         |         |         |         +--- io.ktor:ktor-utils:2.1.3 (*)
|         |         |         |         \--- org.jetbrains.kotlin:atomicfu:1.6.21
|         |         |         +--- io.ktor:ktor-websocket-serialization:2.1.3
|         |         |         |    \--- io.ktor:ktor-websocket-serialization-js:2.1.3
|         |         |         |         +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.4 (*)
|         |         |         |         +--- io.ktor:ktor-http:2.1.3 (*)
|         |         |         |         +--- io.ktor:ktor-serialization:2.1.3
|         |         |         |         |    \--- io.ktor:ktor-serialization-js:2.1.3
|         |         |         |         |         +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.4 (*)
|         |         |         |         |         +--- io.ktor:ktor-http:2.1.3 (*)
|         |         |         |         |         +--- io.ktor:ktor-websockets:2.1.3
|         |         |         |         |         |    \--- io.ktor:ktor-websockets-js:2.1.3
|         |         |         |         |         |         +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.4 (*)
|         |         |         |         |         |         +--- io.ktor:ktor-http:2.1.3 (*)
|         |         |         |         |         |         +--- org.jetbrains.kotlin:kotlin-stdlib-js:1.6.21 -> 1.7.20
|         |         |         |         |         |         \--- org.jetbrains.kotlin:atomicfu:1.6.21
|         |         |         |         |         +--- org.jetbrains.kotlin:kotlin-stdlib-js:1.6.21 -> 1.7.20
|         |         |         |         |         \--- org.jetbrains.kotlin:atomicfu:1.6.21
|         |         |         |         +--- org.jetbrains.kotlin:kotlin-stdlib-js:1.6.21 -> 1.7.20
|         |         |         |         \--- org.jetbrains.kotlin:atomicfu:1.6.21
|         |         |         \--- org.jetbrains.kotlin:atomicfu:1.6.21
|         |         +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.4 (*)
|         |         +--- org.jetbrains.kotlin:kotlin-stdlib-js:1.6.21 -> 1.7.20
|         |         \--- org.jetbrains.kotlin:atomicfu:1.6.21
|         +--- com.apollographql.apollo3:apollo-api:3.7.2
|         |    \--- com.apollographql.apollo3:apollo-api-js:3.7.2
|         |         +--- com.squareup.okio:okio:3.2.0
|         |         |    \--- com.squareup.okio:okio-js:3.2.0
|         |         |         \--- org.jetbrains.kotlin:kotlin-stdlib-js:1.6.20 -> 1.7.20
|         |         +--- com.benasher44:uuid:0.3.1
|         |         |    \--- com.benasher44:uuid-js:0.3.1
|         |         |         \--- org.jetbrains.kotlin:kotlin-stdlib-js:1.5.30 -> 1.7.20
|         |         \--- com.apollographql.apollo3:apollo-annotations:3.7.2
|         |              \--- com.apollographql.apollo3:apollo-annotations-js:3.7.2
|         |                   +--- org.jetbrains.kotlin:kotlin-stdlib-js:1.6.21 -> 1.7.20
|         |                   +--- org.jetbrains.kotlin:kotlin-stdlib:1.6.21 (*)
|         |                   \--- org.jetbrains:annotations:23.0.0
|         +--- com.apollographql.apollo3:apollo-mpp-utils:3.7.2
|         |    \--- com.apollographql.apollo3:apollo-mpp-utils-js:3.7.2
|         |         \--- com.apollographql.apollo3:apollo-annotations:3.7.2 (*)
|         +--- com.squareup.okio:okio:3.2.0 (*)
|         +--- com.benasher44:uuid:0.3.1 (*)
|         \--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.4 (*)

[martinbonnin]

Can you share your gzip'd data ?

[dobe]

this is not straight forward. what would be the insights? i think the decompression in the browser is transparent to the js runtime, isn't it?

[martinbonnin]

Not 100% sure TBH but it'd be nice to be able to reproduce this. A number of things could go wrong.

Is your API public by any chance? That'd help a ton as well.

[martinbonnin]

Looking at the ktor source code, it looks like the Expected 3901, actual 12500 error only happens if "content-encoding" is not set 🤔 :

if (contentEncoding == null && contentLength != null && contentLength > 0) {
    check(bytes.size == contentLength.toInt()) { "Expected $contentLength, actual ${bytes.size}" }
}

Can you double check that your backend is setting content-encoding: gzip?

[adamkobor]

Hey @martinbonnin, we checked it, and yes, our backend sets the content-encoding correctly:

[martinbonnin]

I'll try one last thing which is trying to run the test in the browser (and not node.js). Maybe there's something different there but appart from that, I'm not sure...

[adamkobor]

I'll try one last thing which is trying to run the test in the browser (and not node.js). Maybe there's something different there but appart from that, I'm not sure...

I also checked in the browser with the built-in debugger, and that contentEncoding variable has a null value runtime :/

[martinbonnin]

Sounds like a bug in Ktor. If you can execute the same query without Apollo but using plain ktor client APIs then we'd know for sure.

[adamkobor]

After I've checked the source code and documentation of ktor, I was wondering if it's necessary to install the ContentEncoding plugin of Ktor when you instantiate the KtorHttpEngine? I've also tried to fiddle around with a tiny example:

import io.ktor.client.*
import io.ktor.client.engine.cio.*
import io.ktor.client.plugins.compression.*
import io.ktor.client.request.*
import io.ktor.client.statement.*
import io.ktor.http.*
import kotlinx.coroutines.runBlocking

val client = HttpClient(CIO) {
    install(ContentEncoding) {
        gzip()
    }
}
runBlocking {
    val response = client.get("http://localhost:8082/health") {
        contentType(ContentType.Application.Json)
        // header("Accept-Encoding", "gzip")
    }
    println(response.headers)
    println(response.bodyAsText())
}

If I disable the plugin, then the request won't contain an Accept-Encoding header, unless I explicitly set it a few lines below, but when I do so, the request fails (correctly), because the client doesn't really expect a gzipped response. The suspicious part is that there is an Accept-Encoding header in the outgoing requests in our case in the browser, which should not be possible according to their docs, and also to the example above.

[martinbonnin]

Thanks for the details and sorry for the delay, this stuff is hairy. I was able to reproduce with this:

    val client = io.ktor.client.HttpClient(Js)

    val response = client.get("https://raw.githubusercontent.com/apollographql/apollo-kotlin/main/renovate.json")

    // throws IllegalStateException: Expected 112, actual 113
    println(response.body<ByteArray>())

    // works fine
    // println(response.body<String>())

It looks like a Ktor bug in the ByteArray conversion. I'll dig more and update this issue. Thanks!

[martinbonnin]

Ktor + Browser bug confirmed: https://youtrack.jetbrains.com/issue/KTOR-5300/Gzip-encoding-IllegalStateException-Expected-112-actual-113

The tldr; is that in browser context some headers are not exposed for CORS/security reasons. More on mdn: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Expose-Headers

Until this is fixed in Ktor, you can workaround by copy/pasting the DefaultHttpEngine and use String conversion instead of ByteArray:

class WorkaroundKTOR5300HttpEngine: HttpEngine {
  var disposed = false

  private val client = HttpClient(Js) {
    expectSuccess = false
  }

  override suspend fun execute(request: HttpRequest): HttpResponse {
    try {
      val response = client.request(request.url) {
        method = when (request.method) {
          HttpMethod.Get -> io.ktor.http.HttpMethod.Get
          HttpMethod.Post -> io.ktor.http.HttpMethod.Post
        }
        request.headers.forEach {
          header(it.name, it.value)
        }
        request.body?.let {
          header(HttpHeaders.ContentType, it.contentType)
          val buffer = Buffer()
          it.writeTo(buffer)
          setBody(buffer.readUtf8())
        }
      }

      // Use String here
      val responseString: String = response.body()
      val responseBufferedSource = Buffer().writeUtf8(responseString)
      return HttpResponse.Builder(statusCode = response.status.value)
          .body(responseBufferedSource)
          .addHeaders(response.headers.flattenEntries().map { HttpHeader(it.first, it.second) })
          .build()
    } catch (e: CancellationException) {
      // Cancellation Exception is passthrough
      throw e
    } catch (t: Throwable) {
      throw ApolloNetworkException(t.message, t)
    }
  }

  override fun dispose() {
    if (!disposed) {
      client.close()
      disposed = true
    }
  }
}

And then use that in your ApolloClient.Builder:

  val apolloClient = ApolloClient.Builder()
      .serverUrl("http://localhost:8081/")
      // If you're in multiplatform code, you'll have to create the expect/actual for JVM/Android/Native/...
      .httpEngine(WorkaroundKTOR5300HttpEngine())
      .build()

Let us know again how that works and thanks again for reporting this.

[adamkobor]

Ktor + Browser bug confirmed: https://youtrack.jetbrains.com/issue/KTOR-5300/Gzip-encoding-IllegalStateException-Expected-112-actual-113

The tldr; is that in browser context some headers are not exposed for CORS/security reasons. More on mdn: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Expose-Headers

Until this is fixed in Ktor, you can workaround by copy/pasting the DefaultHttpEngine and use String conversion instead of ByteArray:

class WorkaroundKTOR5300HttpEngine: HttpEngine {
  var disposed = false

  private val client = HttpClient(Js) {
    expectSuccess = false
  }

  override suspend fun execute(request: HttpRequest): HttpResponse {
    try {
      val response = client.request(request.url) {
        method = when (request.method) {
          HttpMethod.Get -> io.ktor.http.HttpMethod.Get
          HttpMethod.Post -> io.ktor.http.HttpMethod.Post
        }
        request.headers.forEach {
          header(it.name, it.value)
        }
        request.body?.let {
          header(HttpHeaders.ContentType, it.contentType)
          val buffer = Buffer()
          it.writeTo(buffer)
          setBody(buffer.readUtf8())
        }
      }

      // Use String here
      val responseString: String = response.body()
      val responseBufferedSource = Buffer().writeUtf8(responseString)
      return HttpResponse.Builder(statusCode = response.status.value)
          .body(responseBufferedSource)
          .addHeaders(response.headers.flattenEntries().map { HttpHeader(it.first, it.second) })
          .build()
    } catch (e: CancellationException) {
      // Cancellation Exception is passthrough
      throw e
    } catch (t: Throwable) {
      throw ApolloNetworkException(t.message, t)
    }
  }

  override fun dispose() {
    if (!disposed) {
      client.close()
      disposed = true
    }
  }
}

And then use that in your ApolloClient.Builder:

  val apolloClient = ApolloClient.Builder()
      .serverUrl("http://localhost:8081/")
      // If you're in multiplatform code, you'll have to create the expect/actual for JVM/Android/Native/...
      .httpEngine(WorkaroundKTOR5300HttpEngine())
      .build()

Let us know again how that works and thanks again for reporting this.

Thanks for the thorough investigation, we just set Content-Encoding as an exposed header on our server, and this was also able to fix the issue. (We're facing another one now, but it's probably irrelevant and it's a completely different case :) )

[martinbonnin]

we just set Content-Encoding as an exposed header on our server

Nice 👍 Thanks for the follow up !

[martinbonnin]

I'll keep this bug open until https://youtrack.jetbrains.com/issue/KTOR-5300 is fixed as a reminder we need to update ktor

[adamkobor]

we just set Content-Encoding as an exposed header on our server

Nice 👍 Thanks for the follow up !

@martinbonnin Regarding the other bug, you should probably be aware of this one too, because it seems that gzipped responses are still affected, even if we expose the Content-Encoding header. In this case the request fails with this:

JsonEncodingException: Unterminated string at path [data, assets, items, 2, description]

The suspicious thing is that the Content-Length header's value for the given response is 1972, and if we get the (0,1972) substring of the JSON response, the end of it will be at the place that is also referred in the error message, so I'm pretty sure that the response will be incorrectly truncated to 1972 bytes and that's why the deserialization fails afterwards

[martinbonnin]

@adamkobor can you check with ktor if you receive the full ByteArray ?

    val client = io.ktor.client.HttpClient(Js)
    val response = client.post(serverUrl) {
      setBody("""
        {
          "query": "..."
        }
      """.trimIndent())
    }
    check(response.body<ByteArray>().size == expectedSize)

[adamkobor]

Hey @martinbonnin thanks for my late reply, I was able to try it, I'm not even able to read the response body if I provide ByteArray as a type parameter to body(), I get: Expected 1688, actual 5538

However if I say

println(response.body<String>())

it emits the whole response as a String, without any error, regardless if I use gzip on the server or not.

[martinbonnin]

The codepath for ByteArray and String are different in Ktor (see DefaultTransform). I'm not expert there but I assume they use the reified type to choose how to convert.

In all cases, if you can reproduce with the latest Ktor EAP, file a Youtrack there, there's not much we can do in Apollo Kotlin itself

[adamkobor]

@martinbonnin I can confirm that I cannot reproduce the error above with 2.2.2-eap-575

[martinbonnin]

Can you file a Youtrack there? If you have the reproducer and everything, you'll have more context than I do.

[adamkobor]

@martinbonnin I might have miss something, but I think that the issue is already fixed in the EAP build of ktor

[martinbonnin]

Gosh, sorry, I read to fast! Can you still reproduce something with Apollo Kotlin then?

[adamkobor]

No, it works like a charm, so probably you'll be OK once 2.2.2 is out there and you bump the version

[martinbonnin]

No, it works like a charm, so probably you'll be OK once 2.2.2 is out there and you bump the version

Gotcha 👍 . Thanks for the heads up!

[adamkobor]

@martinbonnin kindly asking if you have any ETA on this? (since ktor 2.2.2 is out now)

[martinbonnin]

Pull request: https://github.com/apollographql/apollo-kotlin/pull/4627

I guess we could do a release in the coming days with ktor 2.2.2 + data builders for interfaces. @BoD any objections?

[BoD]

Yup that sounds good!

[martinbonnin]

Fixed with 3.7.4: https://github.com/apollographql/apollo-kotlin/releases/tag/v3.7.4