Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

All backends write headers as UTF-8 #2108

Merged
merged 8 commits into from
Mar 19, 2024
Merged

All backends write headers as UTF-8 #2108

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
68136c6
making sure all backends write headers using utf-8
ghik Mar 15, 2024
982702e
disabled non-ascii filename test for backends which don't support it
ghik Mar 15, 2024
7e739bf
using OkHttpHttpTest base class in OkHttpMonixHttpTest
ghik Mar 15, 2024
c3e42e3
more deduplication of common flags in backend tests
ghik Mar 18, 2024
58cf7ee
bump sttp-model version
ghik Mar 18, 2024
a28934b
dropped Scala 2.11 support
ghik Mar 18, 2024
eff130e
bump scalanative version
ghik Mar 19, 2024
b5b5757
minor optimization
ghik Mar 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...ts-ce2/src/test/scala/sttp/client3/asynchttpclient/cats/AsyncHttpClientCatsHttpTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ package sttp.client3.asynchttpclient.cats

import cats.effect.IO
import sttp.client3._
import sttp.client3.asynchttpclient.AsyncHttpClientHttpTest
import sttp.client3.impl.cats.CatsTestBase
import sttp.client3.testing.HttpTest

class AsyncHttpClientCatsHttpTest extends HttpTest[IO] with CatsTestBase {
class AsyncHttpClientCatsHttpTest extends AsyncHttpClientHttpTest[IO] with CatsTestBase {
override val backend: SttpBackend[IO, Any] = AsyncHttpClientCatsBackend[IO]().unsafeRunSync()

"illegal url exceptions" - {
Expand Down
4 changes: 2 additions & 2 deletions ...d/cats/src/test/scala/sttp/client3/asynchttpclient/cats/AsyncHttpClientCatsHttpTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ package sttp.client3.asynchttpclient.cats

import cats.effect.IO
import sttp.client3._
import sttp.client3.asynchttpclient.AsyncHttpClientHttpTest
import sttp.client3.impl.cats.{CatsRetryTest, CatsTestBase}
import sttp.client3.testing.HttpTest

class AsyncHttpClientCatsHttpTest extends HttpTest[IO] with CatsRetryTest with CatsTestBase {
class AsyncHttpClientCatsHttpTest extends AsyncHttpClientHttpTest[IO] with CatsRetryTest with CatsTestBase {
override val backend: SttpBackend[IO, Any] = AsyncHttpClientCatsBackend[IO]().unsafeRunSync()

"illegal url exceptions" - {
Expand Down
4 changes: 2 additions & 2 deletions .../fs2-ce2/src/test/scala/sttp/client3/asynchttpclient/fs2/AsyncHttpClientFs2HttpTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ package sttp.client3.asynchttpclient.fs2

import cats.effect.{Blocker, IO}
import sttp.client3.SttpBackend
import sttp.client3.asynchttpclient.AsyncHttpClientHttpTest
import sttp.client3.impl.cats.CatsTestBase
import sttp.client3.testing.HttpTest

import scala.concurrent.ExecutionContext.global

class AsyncHttpClientFs2HttpTest extends HttpTest[IO] with CatsTestBase {
class AsyncHttpClientFs2HttpTest extends AsyncHttpClientHttpTest[IO] with CatsTestBase {
override val backend: SttpBackend[IO, Any] =
AsyncHttpClientFs2Backend[IO](Blocker.liftExecutionContext(global)).unsafeRunSync()

Expand Down
4 changes: 2 additions & 2 deletions ...kend/fs2/src/test/scala/sttp/client3/asynchttpclient/fs2/AsyncHttpClientFs2HttpTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ package sttp.client3.asynchttpclient.fs2

import cats.effect.IO
import sttp.client3.SttpBackend
import sttp.client3.asynchttpclient.AsyncHttpClientHttpTest
import sttp.client3.impl.cats.{CatsTestBase, TestIODispatcher}
import sttp.client3.testing.HttpTest

class AsyncHttpClientFs2HttpTest extends HttpTest[IO] with TestIODispatcher with CatsTestBase {
class AsyncHttpClientFs2HttpTest extends AsyncHttpClientHttpTest[IO] with TestIODispatcher with CatsTestBase {
override val backend: SttpBackend[IO, Any] =
AsyncHttpClientFs2Backend[IO](dispatcher = dispatcher).unsafeRunSync()

Expand Down
5 changes: 3 additions & 2 deletions ...re/src/test/scala/sttp/client3/asynchttpclient/future/AsyncHttpClientFutureHttpTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package sttp.client3.asynchttpclient.future

import sttp.client3.SttpBackend
import sttp.client3.testing.{ConvertToFuture, HttpTest}
import sttp.client3.asynchttpclient.AsyncHttpClientHttpTest
import sttp.client3.testing.ConvertToFuture

import scala.concurrent.Future

class AsyncHttpClientFutureHttpTest extends HttpTest[Future] {
class AsyncHttpClientFutureHttpTest extends AsyncHttpClientHttpTest[Future] {

override val backend: SttpBackend[Future, Any] = AsyncHttpClientFutureBackend()
override implicit val convertToFuture: ConvertToFuture[Future] = ConvertToFuture.future
Expand Down
6 changes: 3 additions & 3 deletions ...onix/src/test/scala/sttp/client3/asynchttpclient/monix/AsyncHttpClientMonixHttpTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
package sttp.client3.asynchttpclient.monix

import java.util.concurrent.TimeoutException

import monix.eval.Task
import sttp.client3._
import sttp.client3.impl.monix.convertMonixTaskToFuture
import sttp.client3.testing.{ConvertToFuture, HttpTest}
import sttp.client3.testing.ConvertToFuture
import monix.execution.Scheduler.Implicits.global
import sttp.client3.asynchttpclient.AsyncHttpClientHttpTest

import scala.concurrent.duration._

class AsyncHttpClientMonixHttpTest extends HttpTest[Task] {
class AsyncHttpClientMonixHttpTest extends AsyncHttpClientHttpTest[Task] {
override val backend: SttpBackend[Task, Any] = AsyncHttpClientMonixBackend().runSyncUnsafe()
override implicit val convertToFuture: ConvertToFuture[Task] = convertMonixTaskToFuture

Expand Down
5 changes: 3 additions & 2 deletions ...az/src/test/scala/sttp/client3/asynchttpclient/scalaz/AsyncHttpClientScalazHttpTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ package sttp.client3.asynchttpclient.scalaz

import scalaz.concurrent.Task
import sttp.client3.SttpBackend
import sttp.client3.asynchttpclient.AsyncHttpClientHttpTest
import sttp.client3.impl.scalaz.convertScalazTaskToFuture
import sttp.client3.testing.{ConvertToFuture, HttpTest}
import sttp.client3.testing.ConvertToFuture

class AsyncHttpClientScalazHttpTest extends HttpTest[Task] {
class AsyncHttpClientScalazHttpTest extends AsyncHttpClientHttpTest[Task] {

override val backend: SttpBackend[Task, Any] = AsyncHttpClientScalazBackend().unsafePerformSync
override implicit val convertToFuture: ConvertToFuture[Task] = convertScalazTaskToFuture
Expand Down
7 changes: 7 additions & 0 deletions ...-client-backend/src/test/scala/sttp/client3/asynchttpclient/AsyncHttpClientHttpTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package sttp.client3.asynchttpclient

import sttp.client3.testing.HttpTest

abstract class AsyncHttpClientHttpTest[F[_]] extends HttpTest[F] {
override protected def supportsNonAsciiHeaderValues = false
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to extract this class because it makes it clear that it's the AsyncHttpClient that does not support non-ascii header fields, which would not be apparent if this method was overridden in every AsyncHttpClient*HttpTest.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea, but probably there are more such common flags that are set in each test class individually which could be de-duplicated here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll go through them and see what else I can extract.

5 changes: 3 additions & 2 deletions ...kend/zio/src/test/scala/sttp/client3/asynchttpclient/zio/AsyncHttpClientZioHttpTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package sttp.client3.asynchttpclient.zio

import sttp.client3._
import sttp.client3.asynchttpclient.AsyncHttpClientHttpTest
import sttp.client3.impl.zio.ZioTestBase
import sttp.client3.testing.{ConvertToFuture, HttpTest}
import sttp.client3.testing.ConvertToFuture
import zio.Task

class AsyncHttpClientZioHttpTest extends HttpTest[Task] with ZioTestBase {
class AsyncHttpClientZioHttpTest extends AsyncHttpClientHttpTest[Task] with ZioTestBase {

override val backend: SttpBackend[Task, Any] =
unsafeRunSyncOrThrow(AsyncHttpClientZioBackend())
Expand Down
3 changes: 2 additions & 1 deletion ...end/zio1/src/test/scala/sttp/client3/asynchttpclient/zio/AsyncHttpClientZioHttpTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package sttp.client3.asynchttpclient.zio

import sttp.client3._
import sttp.client3.asynchttpclient.AsyncHttpClientHttpTest
import sttp.client3.impl.zio.ZioTestBase
import sttp.client3.testing.{ConvertToFuture, HttpTest}
import zio.Task

class AsyncHttpClientZioHttpTest extends HttpTest[Task] with ZioTestBase {
class AsyncHttpClientZioHttpTest extends AsyncHttpClientHttpTest[Task] with ZioTestBase {

override val backend: SttpBackend[Task, Any] =
runtime.unsafeRun(AsyncHttpClientZioBackend())
Expand Down
2 changes: 1 addition & 1 deletion build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ val zio2Version = "2.0.10"
val zio1InteropRsVersion = "1.3.12"
val zio2InteropRsVersion = "2.0.1"

val sttpModelVersion = "1.5.5"
val sttpModelVersion = "1.7.7+0-4acff295+20240315-1203-SNAPSHOT"
val sttpSharedVersion = "1.3.16"

val logback = "ch.qos.logback" % "logback-classic" % "1.4.5"
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/scalajvm/sttp/client3/HttpURLConnectionBackend.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ class HttpURLConnectionBackend private (
case MultipartBody(_) => None
}

val headersLen = headers.getBytes(Iso88591).length
val headersLen = headers.getBytes(Utf8).length

bodyLen.map(bl => dashesLen + boundaryLen + crLfLen + headersLen + crLfLen + crLfLen + bl + crLfLen)
}
Expand All @@ -198,8 +198,8 @@ class HttpURLConnectionBackend private (

val os = c.getOutputStream
def writeMeta(s: String): Unit = {
os.write(s.getBytes(Iso88591))
total += s.getBytes(Iso88591).length.toLong
os.write(s.getBytes(Utf8))
total += s.getBytes(Utf8).length.toLong
}

partsWithHeaders.foreach { case (headers, p) =>
Expand Down
8 changes: 8 additions & 0 deletions core/src/test/scala/sttp/client3/testing/HttpTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ trait HttpTest[F[_]]
protected def supportsAutoDecompressionDisabling = true
protected def supportsDeflateWrapperChecking = true
protected def supportsEmptyContentEncoding = true
protected def supportsNonAsciiHeaderValues = true

"request parsing" - {
"Inf timeout should not throw exception" in {
Expand Down Expand Up @@ -462,6 +463,13 @@ trait HttpTest[F[_]]
req.send(backend).toFuture().map { resp => resp.body should be("p1=v1 (f1), p2=v2 (f2)") }
}

if(supportsNonAsciiHeaderValues) {
"send a multipart message with non-ascii filenames" in {
val req = mp.multipartBody(multipart("p1", "v1").fileName("fó1"), multipart("p2", "v2").fileName("fó2"))
req.send(backend).toFuture().map { resp => resp.body should be("p1=v1 (fó1), p2=v2 (fó2)") }
}
}

"send a multipart message with binary data and filename" in {
val binaryPart = {
multipart("p1", "v1".getBytes)
Expand Down
2 changes: 2 additions & 0 deletions okhttp-backend/monix/src/test/scala/sttp/client3/okhttp/monix/OkHttpMonixHttpTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ class OkHttpMonixHttpTest extends HttpTest[Task] {

override def supportsDeflateWrapperChecking = false

override def supportsNonAsciiHeaderValues = false
ghik marked this conversation as resolved.
Show resolved Hide resolved

override def timeoutToNone[T](t: Task[T], timeoutMillis: Int): Task[Option[T]] =
t.map(Some(_))
.timeout(timeoutMillis.milliseconds)
Expand Down
4 changes: 2 additions & 2 deletions okhttp-backend/src/test/scala/sttp/client3/okhttp/OkHttpFutureHttpTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ package sttp.client3.okhttp

import sttp.capabilities.WebSockets
import sttp.client3.SttpBackend
import sttp.client3.testing.{ConvertToFuture, HttpTest}
import sttp.client3.testing.ConvertToFuture

import scala.concurrent.Future

class OkHttpFutureHttpTest extends HttpTest[Future] {
class OkHttpFutureHttpTest extends OkHttpHttpTest[Future] {

override val backend: SttpBackend[Future, WebSockets] = OkHttpFutureBackend()
override implicit val convertToFuture: ConvertToFuture[Future] = ConvertToFuture.future
Expand Down
7 changes: 7 additions & 0 deletions okhttp-backend/src/test/scala/sttp/client3/okhttp/OkHttpHttpTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package sttp.client3.okhttp

import sttp.client3.testing.HttpTest

abstract class OkHttpHttpTest[F[_]] extends HttpTest[F] {
override protected def supportsNonAsciiHeaderValues = false
}
4 changes: 2 additions & 2 deletions okhttp-backend/src/test/scala/sttp/client3/okhttp/OkHttpSyncHttpTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package sttp.client3.okhttp

import sttp.capabilities.WebSockets
import sttp.client3.testing.{ConvertToFuture, HttpTest}
import sttp.client3.testing.ConvertToFuture
import sttp.client3.{Identity, SttpBackend}

class OkHttpSyncHttpTest extends HttpTest[Identity] {
class OkHttpSyncHttpTest extends OkHttpHttpTest[Identity] {
override val backend: SttpBackend[Identity, WebSockets] = OkHttpSyncBackend()

override implicit val convertToFuture: ConvertToFuture[Identity] = ConvertToFuture.id
Expand Down
Loading