Repository: http4s/http4s
Pull request #5051 (2021-08-09 10:06:31)

Allow empty `Access-Control-{Allow, Expose}-Headers`
Per the [fetch spec](https://fetch.spec.whatwg.org/#http-new-header-syntax), these headers can be empty. Unfortunately I don't think binary compatibility can be preserved.


Patch:
diff --git a/.jvmopts b/.jvmopts
index 87a06f6e1b..27f46cc3dd 100644
--- a/.jvmopts
+++ b/.jvmopts
@@ -3,4 +3,4 @@
 -Xmx6G
 -XX:ReservedCodeCacheSize=250M
 -XX:+TieredCompilation
--XX:+UseG1GC
+-XX:+UseParallelGC
diff --git a/core/shared/src/main/scala/org/http4s/headers/Access-Control-Allow-Headers.scala b/core/shared/src/main/scala/org/http4s/headers/Access-Control-Allow-Headers.scala
index e281feb800..9893da05aa 100644
--- a/core/shared/src/main/scala/org/http4s/headers/Access-Control-Allow-Headers.scala
+++ b/core/shared/src/main/scala/org/http4s/headers/Access-Control-Allow-Headers.scala
@@ -20,17 +20,19 @@ package headers
 import org.http4s.Header
 import org.typelevel.ci._
 import org.http4s.internal.parsing.Rfc7230
-import cats.data.NonEmptyList
 
 object `Access-Control-Allow-Headers` {
-  def apply(head: CIString, tail: CIString*): `Access-Control-Allow-Headers` =
-    apply(NonEmptyList(head, tail.toList))
+  def apply(values: CIString*): `Access-Control-Allow-Headers` =
+    apply(values.toList)
+
+  val empty: `Access-Control-Allow-Headers` = `Access-Control-Allow-Headers`(Nil)
 
   def parse(s: String): ParseResult[`Access-Control-Allow-Headers`] =
     ParseResult.fromParser(parser, "Invalid Access-Control-Allow-Headers headers")(s)
 
+  // https://fetch.spec.whatwg.org/#http-new-header-syntax (as of commit 613aad98fb19bf44fc95c4e9a332706d68795da8)
   private[http4s] val parser =
-    Rfc7230.headerRep1(Rfc7230.token.map(CIString(_))).map(`Access-Control-Allow-Headers`(_))
+    Rfc7230.headerRep(Rfc7230.token.map(CIString(_))).map(`Access-Control-Allow-Headers`(_))
 
   implicit val headerInstance: Header[`Access-Control-Allow-Headers`, Header.Recurring] =
     Header.createRendered(
@@ -39,8 +41,8 @@ object `Access-Control-Allow-Headers` {
       parse
     )
 
-  implicit val headerSemigroupInstance: cats.Semigroup[`Access-Control-Allow-Headers`] =
-    (a, b) => `Access-Control-Allow-Headers`(a.values.concatNel(b.values))
+  implicit val headerMonoidInstance: cats.Monoid[`Access-Control-Allow-Headers`] =
+    cats.Monoid.instance(empty, (a, b) => `Access-Control-Allow-Headers`(a.values ++ b.values))
 }
 
-final case class `Access-Control-Allow-Headers`(values: NonEmptyList[CIString])
+final case class `Access-Control-Allow-Headers`(values: List[CIString])
diff --git a/core/shared/src/main/scala/org/http4s/headers/Access-Control-Expose-Headers.scala b/core/shared/src/main/scala/org/http4s/headers/Access-Control-Expose-Headers.scala
index ed8b417747..8c97c1d5fd 100644
--- a/core/shared/src/main/scala/org/http4s/headers/Access-Control-Expose-Headers.scala
+++ b/core/shared/src/main/scala/org/http4s/headers/Access-Control-Expose-Headers.scala
@@ -17,20 +17,21 @@
 package org.http4s
 package headers
 
-import cats.data.NonEmptyList
 import org.http4s.internal.parsing.Rfc7230
 import org.typelevel.ci._
 
 object `Access-Control-Expose-Headers` {
+  def apply(values: CIString*): `Access-Control-Expose-Headers` =
+    apply(values.toList)
 
-  def apply(head: CIString, tail: CIString*): `Access-Control-Expose-Headers` =
-    apply(NonEmptyList(head, tail.toList))
+  val empty: `Access-Control-Expose-Headers` = `Access-Control-Expose-Headers`(Nil)
 
   def parse(s: String): ParseResult[`Access-Control-Expose-Headers`] =
     ParseResult.fromParser(parser, "Invalid Access-Control-Allow-Headers header")(s)
 
+  // https://fetch.spec.whatwg.org/#http-new-header-syntax (as of commit 613aad98fb19bf44fc95c4e9a332706d68795da8)
   private[http4s] val parser =
-    Rfc7230.headerRep1(Rfc7230.token.map(CIString(_))).map(`Access-Control-Expose-Headers`(_))
+    Rfc7230.headerRep(Rfc7230.token.map(CIString(_))).map(`Access-Control-Expose-Headers`(_))
 
   implicit val headerInstance: Header[`Access-Control-Expose-Headers`, Header.Recurring] =
     Header.createRendered(
@@ -39,8 +40,8 @@ object `Access-Control-Expose-Headers` {
       parse
     )
 
-  implicit val headerSemigroupInstance: cats.Semigroup[`Access-Control-Expose-Headers`] =
-    (a, b) => `Access-Control-Expose-Headers`(a.values.concatNel(b.values))
+  implicit val headerMonoidInstance: cats.Monoid[`Access-Control-Expose-Headers`] =
+    cats.Monoid.instance(empty, (a, b) => `Access-Control-Expose-Headers`(a.values ++ b.values))
 }
 
-final case class `Access-Control-Expose-Headers`(values: NonEmptyList[CIString])
+final case class `Access-Control-Expose-Headers`(values: List[CIString])
diff --git a/core/shared/src/main/scala/org/http4s/internal/parsing/Rfc7230.scala b/core/shared/src/main/scala/org/http4s/internal/parsing/Rfc7230.scala
index 9db4f55930..bfe5778deb 100644
--- a/core/shared/src/main/scala/org/http4s/internal/parsing/Rfc7230.scala
+++ b/core/shared/src/main/scala/org/http4s/internal/parsing/Rfc7230.scala
@@ -77,6 +77,9 @@ private[http4s] object Rfc7230 {
     between(char('('), cText.orElse(quotedPair).orElse(comment).rep0.string, char(')'))
   }
 
+  def headerRep[A](element: Parser[A]): Parser0[List[A]] =
+    headerRep1(element).?.map(_.fold(List.empty[A])(_.toList))
+
   /* `1#element => *( "," OWS ) element *( OWS "," [ OWS element ] )` */
   def headerRep1[A](element: Parser[A]): Parser[NonEmptyList[A]] = {
     val prelude = (char(',') <* ows).rep0
diff --git a/core/shared/src/main/scala/org/http4s/util/Renderable.scala b/core/shared/src/main/scala/org/http4s/util/Renderable.scala
index 659f1347f6..a389ae1412 100644
--- a/core/shared/src/main/scala/org/http4s/util/Renderable.scala
+++ b/core/shared/src/main/scala/org/http4s/util/Renderable.scala
@@ -97,6 +97,12 @@ object Renderer {
         writer.addNel(values)
     }
 
+  implicit def listRenderer[T: Renderer]: Renderer[List[T]] =
+    new Renderer[List[T]] {
+      override def render(writer: Writer, values: List[T]): writer.type =
+        writer.addList(values)
+    }
+
   implicit def setRenderer[T: Renderer]: Renderer[Set[T]] =
     new Renderer[Set[T]] {
       override def render(writer: Writer, values: Set[T]): writer.type =
@@ -191,6 +197,18 @@ trait Writer {
     append(end)
   }
 
+  def addList[T: Renderer](
+      s: List[T],
+      sep: String = ", ",
+      start: String = "",
+      end: String = ""): this.type =
+    NonEmptyList.fromList(s) match {
+      case Some(s) => addNel(s, sep, start, end)
+      case None =>
+        append(start)
+        append(end)
+    }
+
   def addNel[T: Renderer](
       s: NonEmptyList[T],
       sep: String = ", ",
diff --git a/laws/src/main/scala/org/http4s/laws/discipline/ArbitraryInstances.scala b/laws/src/main/scala/org/http4s/laws/discipline/ArbitraryInstances.scala
index 102f8317f4..564cb2d959 100644
--- a/laws/src/main/scala/org/http4s/laws/discipline/ArbitraryInstances.scala
+++ b/laws/src/main/scala/org/http4s/laws/discipline/ArbitraryInstances.scala
@@ -518,16 +518,16 @@ private[http4s] trait ArbitraryInstances extends Ip4sArbitraryInstances {
       : Arbitrary[headers.`Access-Control-Allow-Headers`] =
     Arbitrary {
       for {
-        values <- nonEmptyListOf(genToken.map(CIString(_)))
-      } yield headers.`Access-Control-Allow-Headers`(NonEmptyList.of(values.head, values.tail: _*))
+        values <- listOf(genToken.map(CIString(_)))
+      } yield headers.`Access-Control-Allow-Headers`(values)
     }
 
   implicit val http4sTestingArbitraryForAccessControlExposeHeaders
       : Arbitrary[headers.`Access-Control-Expose-Headers`] =
     Arbitrary {
       for {
-        values <- nonEmptyListOf(genToken.map(CIString(_)))
-      } yield headers.`Access-Control-Expose-Headers`(NonEmptyList.of(values.head, values.tail: _*))
+        values <- listOf(genToken.map(CIString(_)))
+      } yield headers.`Access-Control-Expose-Headers`(values)
     }
 
   implicit val http4sTestingArbitraryForRetryAfterHeader: Arbitrary[headers.`Retry-After`] =


Test patch:
diff --git a/tests/shared/src/test/scala/org/http4s/parser/SimpleHeadersSpec.scala b/tests/shared/src/test/scala/org/http4s/parser/SimpleHeadersSpec.scala
index 3d12f5366c..9b22b0ca81 100644
--- a/tests/shared/src/test/scala/org/http4s/parser/SimpleHeadersSpec.scala
+++ b/tests/shared/src/test/scala/org/http4s/parser/SimpleHeadersSpec.scala
@@ -44,33 +44,37 @@ class SimpleHeadersSpec extends Http4sSuite {
   test("parse Access-Control-Allow-Headers") {
 
     val header = `Access-Control-Allow-Headers`(
-      NonEmptyList.of(
-        ci"Accept",
-        ci"Expires",
-        ci"X-Custom-Header",
-        ci"*"
-      )
+      ci"Accept",
+      ci"Expires",
+      ci"X-Custom-Header",
+      ci"*"
     )
 
     assertEquals(`Access-Control-Allow-Headers`.parse(header.toRaw1.value), Right(header))
 
     val invalidHeaderValue = "(non-token-name), non[&token]name"
     assert(`Access-Control-Allow-Headers`.parse(invalidHeaderValue).isLeft)
+
+    assertEquals(
+      `Access-Control-Allow-Headers`.parse(""),
+      Right(`Access-Control-Allow-Headers`.empty))
   }
 
   test("parse Access-Control-Expose-Headers") {
     val header = `Access-Control-Expose-Headers`(
-      NonEmptyList.of(
-        ci"Content-Length",
-        ci"Authorization",
-        ci"X-Custom-Header",
-        ci"*"
-      )
+      ci"Content-Length",
+      ci"Authorization",
+      ci"X-Custom-Header",
+      ci"*"
     )
     assertEquals(`Access-Control-Expose-Headers`.parse(header.toRaw1.value), Right(header))
 
     val invalidHeaderValue = "(non-token-name), non[&token]name"
     assert(`Access-Control-Expose-Headers`.parse(invalidHeaderValue).isLeft)
+
+    assertEquals(
+      `Access-Control-Expose-Headers`.parse(""),
+      Right(`Access-Control-Expose-Headers`.empty))
   }
 
   test("parse Connection") {