#53: using proper encoding when reading the response body

7 files changed, 72 insertions, 19 deletions

diff --git a/akka-http-backend/src/main/scala/com/softwaremill/sttp/akkahttp/AkkaHttpBackend.scala b/akka-http-backend/src/main/scala/com/softwaremill/sttp/akkahttp/AkkaHttpBackend.scala
index 12d8ce0..91126e9 100644
--- a/akka-http-backend/src/main/scala/com/softwaremill/sttp/akkahttp/AkkaHttpBackend.scala
+++ b/akka-http-backend/src/main/scala/com/softwaremill/sttp/akkahttp/AkkaHttpBackend.scala
@@ -76,13 +76,21 @@ class AkkaHttpBackend private (
       .flatMap { hr =>
         val code = hr.status.intValue()
 
+        val headers = headersFromAkka(hr)
+        val charsetFromHeaders = headers
+          .find(_._1 == ContentTypeHeader)
+          .map(_._2)
+          .flatMap(encodingFromContentType)
+
         val body = if (codeIsSuccess(code)) {
-          bodyFromAkka(r.response, decodeAkkaResponse(hr)).map(Right(_))
+          bodyFromAkka(r.response, decodeAkkaResponse(hr), charsetFromHeaders)
+            .map(Right(_))
         } else {
-          bodyFromAkka(asString, decodeAkkaResponse(hr)).map(Left(_))
+          bodyFromAkka(asString, decodeAkkaResponse(hr), charsetFromHeaders)
+            .map(Left(_))
         }
 
-        body.map(Response(_, code, headersFromAkka(hr), Nil))
+        body.map(Response(_, code, headers, Nil))
       }
   }
 
@@ -102,8 +110,10 @@ class AkkaHttpBackend private (
   }
 
   private def bodyFromAkka[T](rr: ResponseAs[T, S],
-                              hr: HttpResponse): Future[T] = {
-    implicit val ec = this.ec
+                              hr: HttpResponse,
+                              charsetFromHeaders: Option[String]): Future[T] = {
+
+    implicit val ec: ExecutionContext = this.ec
 
     def asByteArray =
       hr.entity.dataBytes
@@ -123,14 +133,15 @@ class AkkaHttpBackend private (
     }
 
     rr match {
-      case MappedResponseAs(raw, g) => bodyFromAkka(raw, hr).map(g)
+      case MappedResponseAs(raw, g) =>
+        bodyFromAkka(raw, hr, charsetFromHeaders).map(g)
 
       case IgnoreResponse =>
         hr.discardEntityBytes()
         Future.successful(())
 
       case ResponseAsString(enc) =>
-        asByteArray.map(new String(_, enc))
+        asByteArray.map(new String(_, charsetFromHeaders.getOrElse(enc)))
 
       case ResponseAsByteArray =>
         asByteArray
diff --git a/async-http-client-backend/src/main/scala/com/softwaremill/sttp/asynchttpclient/AsyncHttpClientBackend.scala b/async-http-client-backend/src/main/scala/com/softwaremill/sttp/asynchttpclient/AsyncHttpClientBackend.scala
index 39c2b30..885e5a9 100644
--- a/async-http-client-backend/src/main/scala/com/softwaremill/sttp/asynchttpclient/AsyncHttpClientBackend.scala
+++ b/async-http-client-backend/src/main/scala/com/softwaremill/sttp/asynchttpclient/AsyncHttpClientBackend.scala
@@ -270,7 +270,10 @@ abstract class AsyncHttpClientBackend[R[_], S](asyncHttpClient: AsyncHttpClient,
             Try(())
 
           case ResponseAsString(enc) =>
-            Try(response.getResponseBody(Charset.forName(enc)))
+            val charset = Option(response.getHeader(ContentTypeHeader))
+              .flatMap(encodingFromContentType)
+              .getOrElse(enc)
+            Try(response.getResponseBody(Charset.forName(charset)))
 
           case ResponseAsByteArray =>
             Try(response.getResponseBodyAsBytes)
diff --git a/core/src/main/scala/com/softwaremill/sttp/HttpURLConnectionBackend.scala b/core/src/main/scala/com/softwaremill/sttp/HttpURLConnectionBackend.scala
index 083b1c3..65b1a0c 100644
--- a/core/src/main/scala/com/softwaremill/sttp/HttpURLConnectionBackend.scala
+++ b/core/src/main/scala/com/softwaremill/sttp/HttpURLConnectionBackend.scala
@@ -214,24 +214,30 @@ class HttpURLConnectionBackend private (
       .filter(_._1 != null)
       .flatMap { case (k, vv) => vv.asScala.map((k, _)) }
     val contentEncoding = Option(c.getHeaderField(ContentEncodingHeader))
+
+    val charsetFromHeaders = Option(c.getHeaderField(ContentTypeHeader))
+      .flatMap(encodingFromContentType)
+
     val code = c.getResponseCode
     val wrappedIs = wrapInput(contentEncoding, handleNullInput(is))
     val body = if (codeIsSuccess(code)) {
-      Right(readResponseBody(wrappedIs, responseAs))
+      Right(readResponseBody(wrappedIs, responseAs, charsetFromHeaders))
     } else {
-      Left(readResponseBody(wrappedIs, asString))
+      Left(readResponseBody(wrappedIs, asString, charsetFromHeaders))
     }
 
     Response(body, code, headers, Nil)
   }
 
   private def readResponseBody[T](is: InputStream,
-                                  responseAs: ResponseAs[T, Nothing]): T = {
+                                  responseAs: ResponseAs[T, Nothing],
+                                  charset: Option[String]): T = {
 
-    def asString(enc: String) = Source.fromInputStream(is, enc).mkString
+    def asString(enc: String) =
+      Source.fromInputStream(is, charset.getOrElse(enc)).mkString
 
     responseAs match {
-      case MappedResponseAs(raw, g) => g(readResponseBody(is, raw))
+      case MappedResponseAs(raw, g) => g(readResponseBody(is, raw, charset))
 
       case IgnoreResponse =>
         @tailrec def consume(): Unit = if (is.read() != -1) consume()
diff --git a/core/src/main/scala/com/softwaremill/sttp/package.scala b/core/src/main/scala/com/softwaremill/sttp/package.scala
index b641a77..f47e113 100644
--- a/core/src/main/scala/com/softwaremill/sttp/package.scala
+++ b/core/src/main/scala/com/softwaremill/sttp/package.scala
@@ -83,19 +83,27 @@ package object sttp {
   def ignore: ResponseAs[Unit, Nothing] = IgnoreResponse
 
   /**
-    * Uses `utf-8` encoding.
+    * Use the `utf-8` encoding by default, unless specified otherwise in the response headers.
     */
   def asString: ResponseAs[String, Nothing] = asString(Utf8)
+
+  /**
+    * Use the given encoding by default, unless specified otherwise in the response headers.
+    */
   def asString(encoding: String): ResponseAs[String, Nothing] =
     ResponseAsString(encoding)
   def asByteArray: ResponseAs[Array[Byte], Nothing] =
     ResponseAsByteArray
 
   /**
-    * Uses `utf-8` encoding.
+    * Use the `utf-8` encoding by default, unless specified otherwise in the response headers.
     */
   def asParams: ResponseAs[Seq[(String, String)], Nothing] =
     asParams(Utf8)
+
+  /**
+    * Use the given encoding by default, unless specified otherwise in the response headers.
+    */
   def asParams(encoding: String): ResponseAs[Seq[(String, String)], Nothing] =
     asString(encoding).map(ResponseAs.parseParams(_, encoding))
 
@@ -238,9 +246,14 @@ package object sttp {
 
   // util
 
-  private[sttp] def contentTypeWithEncoding(ct: String, enc: String) =
+  private[sttp] def contentTypeWithEncoding(ct: String, enc: String): String =
     s"$ct; charset=$enc"
 
+  private[sttp] def encodingFromContentType(ct: String): Option[String] =
+    ct.split(";").map(_.trim.toLowerCase).collectFirst {
+      case s if s.startsWith("charset=") => s.substring(8)
+    }
+
   private[sttp] def transfer(is: InputStream, os: OutputStream) {
     var read = 0
     val buf = new Array[Byte](1024)
diff --git a/docs/responses/body.rst b/docs/responses/body.rst
index 10658ed..7b45fbc 100644
--- a/docs/responses/body.rst
+++ b/docs/responses/body.rst
@@ -3,7 +3,7 @@
 Response body specification
 ===========================
 
-By default, the received response body will be read as a ``String``, using the ``UTF-8`` encoding. This is of course configurable: response bodies can be ignored, deserialized into custom types, recevied as a stream or saved to a file.
+By default, the received response body will be read as a ``String``, using the encoding specified in the ``Content-Type`` response header (and if none is specified, using ``UTF-8``). This is of course configurable: response bodies can be ignored, deserialized into custom types, received as a stream or saved to a file.
 
 How the response body will be read is part of the request definition, as already when sending the request, the backend needs to know what to do with the response. The type to which the response body should be deserialized is the second type parameter of ``RequestT``, and stored in the request definition as the ``request.response: ResponseAs[T, S]`` property.
 
diff --git a/okhttp-backend/src/main/scala/com/softwaremill/sttp/okhttp/OkHttpBackend.scala b/okhttp-backend/src/main/scala/com/softwaremill/sttp/okhttp/OkHttpBackend.scala
index a4b6b54..650d179 100644
--- a/okhttp-backend/src/main/scala/com/softwaremill/sttp/okhttp/OkHttpBackend.scala
+++ b/okhttp-backend/src/main/scala/com/softwaremill/sttp/okhttp/OkHttpBackend.scala
@@ -117,8 +117,11 @@ abstract class OkHttpBackend[R[_], S](client: OkHttpClient,
           case IgnoreResponse =>
             Try(res.close())
           case ResponseAsString(encoding) =>
+            val charset = Option(res.header(ContentTypeHeader))
+              .flatMap(encodingFromContentType)
+              .getOrElse(encoding)
             val body = Try(
-              res.body().source().readString(Charset.forName(encoding)))
+              res.body().source().readString(Charset.forName(charset)))
             res.close()
             body
           case ResponseAsByteArray =>
diff --git a/tests/src/test/scala/com/softwaremill/sttp/BasicTests.scala b/tests/src/test/scala/com/softwaremill/sttp/BasicTests.scala
index 18ff6ae..af5c5ae 100644
--- a/tests/src/test/scala/com/softwaremill/sttp/BasicTests.scala
+++ b/tests/src/test/scala/com/softwaremill/sttp/BasicTests.scala
@@ -8,7 +8,7 @@ import java.time.{ZoneId, ZonedDateTime}
 import akka.http.scaladsl.coding.{Deflate, Gzip, NoCoding}
 import akka.http.scaladsl.model.headers.CacheDirectives._
 import akka.http.scaladsl.model.headers._
-import akka.http.scaladsl.model.{DateTime, FormData, StatusCodes}
+import akka.http.scaladsl.model._
 import akka.http.scaladsl.server.Directives._
 import akka.http.scaladsl.server.Route
 import akka.http.scaladsl.server.directives.Credentials
@@ -55,6 +55,7 @@ class BasicTests
   private val binaryFile =
     new java.io.File("tests/src/test/resources/binaryfile.jpg")
   private val outPath = Paths.get("out")
+  private val textWithSpecialCharacters = "Żółć!"
 
   override val serverRoutes: Route =
     pathPrefix("echo") {
@@ -181,6 +182,13 @@ class BasicTests
             protocol = HttpProtocols.`HTTP/1.1`
           ))
       }
+    } ~ path("respond_with_iso_8859_2") {
+      get { ctx =>
+        val entity = HttpEntity(
+          MediaTypes.`text/plain`.withCharset(HttpCharset.custom("ISO-8859-2")),
+          textWithSpecialCharacters)
+        ctx.complete(HttpResponse(200, entity = entity))
+      }
     }
 
   override def port = 51823
@@ -232,6 +240,7 @@ class BasicTests
     redirectTests()
     timeoutTests()
     emptyResponseTests()
+    encodingTests()
 
     def parseResponseTests(): Unit = {
       name should "parse response as string" in {
@@ -688,6 +697,14 @@ class BasicTests
         response.body should be(Left(""))
       }
     }
+
+    def encodingTests(): Unit = {
+      name should "read response body encoded using ISO-8859-2, as specified in the header, overriding the default" in {
+        val request = sttp.get(uri"$endpoint/respond_with_iso_8859_2")
+
+        request.send().force().unsafeBody should be(textWithSpecialCharacters)
+      }
+    }
   }
 
   override protected def afterAll(): Unit = {