Repository: http4s/http4s (scala)

Issue:
Update `Origin` header to current standard or document decision not to do so
The [model for the `Origin` header](https://github.com/http4s/http4s/blob/main/core/src/main/scala/org/http4s/headers/Origin.scala) allows a non-empty list of hosts, citing [RFC 6454](http://tools.ietf.org/html/rfc6454#section-7) but the [current standard](https://fetch.spec.whatwg.org/#origin-header) allows only one host, noting that this supplants the definition in the RFC.

I can think of the following options:

1. Update the models to only allow one host, and have the parser handle extra hosts by:
  a. ignoring them
  b. raising an error
2. Add a third subtype to the model, `SingleHost`, with a comment noting that `HostList` is not spec-compliant
3. Headers are type-classes now! Why not provide strict or permissive versions? 
4. Leave the behaviour as is, with a comment added to document the decision.

I would favour option 1a, but my view obviously doesn't deserve any special weight.

Pull request:
Resolves #5009.

I've also taken the opportunity to backtify the `null` value for consistency with the rest of the library, to stop parsing empty values as `null` (it's not allowed by the spec), and generally to simplify the parsing implementation.

Patch:
diff --git a/core/shared/src/main/scala/org/http4s/headers/Origin.scala b/core/shared/src/main/scala/org/http4s/headers/Origin.scala
index eeca09dcb9a..abefb94c3ce 100644
--- a/core/shared/src/main/scala/org/http4s/headers/Origin.scala
+++ b/core/shared/src/main/scala/org/http4s/headers/Origin.scala
@@ -17,9 +17,8 @@
 package org.http4s
 package headers
 
-import cats.data.NonEmptyList
-import cats.parse.{Parser, Parser0, Rfc5234}
-import org.http4s.Uri.RegName
+import cats.parse.Parser.{`end`, char, string}
+import cats.parse.Parser
 import org.http4s.util.{Renderable, Writer}
 import org.typelevel.ci._
 
@@ -28,18 +27,15 @@ sealed abstract class Origin
 object Origin {
   // An Origin header may be the string "null", representing an "opaque origin":
   // https://stackoverflow.com/questions/42239643/when-does-firefox-set-the-origin-header-to-null-in-post-requests
-  case object Null extends Origin
-
-  // If the Origin is not "null", it is a non-empty list of Hosts:
-  // http://tools.ietf.org/html/rfc6454#section-7
-  final case class HostList(hosts: NonEmptyList[Host]) extends Origin
+  case object `null` extends Origin
 
   // A host in an Origin header isn't a full URI.
   // It only contains a scheme, a host, and an optional port.
   // Hence we re-used parts of the Uri class here, but we don't use a whole Uri:
   // http://tools.ietf.org/html/rfc6454#section-7
   final case class Host(scheme: Uri.Scheme, host: Uri.Host, port: Option[Int] = None)
-      extends Renderable {
+      extends Origin
+      with Renderable {
     def toUri: Uri =
       Uri(scheme = Some(scheme), authority = Some(Uri.Authority(host = host, port = port)))
 
@@ -47,29 +43,16 @@ object Origin {
       toUri.render(writer)
   }
 
-  private[http4s] val parser: Parser0[Origin] = {
-    import Parser.{`end`, char, string, until}
-    import Rfc5234.{alpha, digit}
+  private[http4s] val parser: Parser[Origin] = {
+    import Uri.Parser.{host, port, scheme}
 
-    val unknownScheme =
-      alpha ~ Parser.oneOf(List(alpha, digit, char('+'), char('-'), char('.'))).rep0
-    val http = string("http")
-    val https = string("https")
-    val scheme = List(https, http, unknownScheme)
-      .reduceLeft(_ orElse _)
-      .string
-      .map(Uri.Scheme.unsafeFromString)
-    val stringHost = until(char(':').orElse(`end`)).map(RegName.apply)
-    val bracketedIpv6 = char('[') *> Uri.Parser.ipv6Address <* char(']')
-    val host = List(bracketedIpv6, Uri.Parser.ipv4Address, stringHost).reduceLeft(_ orElse _)
-    val port = char(':') *> digit.rep.string.map(_.toInt)
-    val nullHost = (string("null") *> `end`).orElse(`end`).as(Origin.Null)
+    val nullP = (string("null") *> `end`).as(Origin.`null`)
 
-    val singleHost = ((scheme <* string("://")) ~ host ~ port.?).map { case ((sch, host), port) =>
-      Origin.Host(sch, host, port)
+    val hostP = ((scheme <* string("://")) ~ host ~ (char(':') *> port).?.map(_.flatten)).map {
+      case ((sch, host), port) => Origin.Host(sch, host, port)
     }
 
-    nullHost.orElse(singleHost.repSep(char(' ')).map(hosts => Origin.HostList(hosts)))
+    nullP | hostP
   }
 
   def parse(s: String): ParseResult[Origin] =
@@ -82,18 +65,10 @@ object Origin {
         new Renderable {
           def render(writer: Writer): writer.type =
             v match {
-              case HostList(hosts) =>
-                writer << hosts.head
-                hosts.tail.foreach { host =>
-                  writer << " "
-                  writer << host
-                }
-                writer
-              case Null => writer << "null"
+              case h: Host => writer << h
+              case `null` => writer << "null"
             }
-
         },
       parse
     )
-
 }


Test patch:
diff --git a/tests/shared/src/test/scala/org/http4s/parser/OriginHeaderSuite.scala b/tests/shared/src/test/scala/org/http4s/parser/OriginHeaderSuite.scala
index 1acbc603c29..d0ed99eb8c3 100644
--- a/tests/shared/src/test/scala/org/http4s/parser/OriginHeaderSuite.scala
+++ b/tests/shared/src/test/scala/org/http4s/parser/OriginHeaderSuite.scala
@@ -17,7 +17,6 @@
 package org.http4s
 package parser
 
-import cats.data.NonEmptyList
 import com.comcast.ip4s._
 import org.http4s.headers.Origin
 import org.http4s.syntax.all._
@@ -30,28 +29,23 @@ class OriginHeaderSuite extends munit.FunSuite {
   val hostString2 = "https://127.0.0.1"
 
   test("Origin value method should Render a host with a port number") {
-    val origin: Origin = Origin.HostList(NonEmptyList.of(host1))
+    val origin: Origin = host1
     assertEquals(origin.value, hostString1)
   }
 
   test("Origin value method should Render a host without a port number") {
-    val origin: Origin = Origin.HostList(NonEmptyList.of(host2))
+    val origin: Origin = host2
     assertEquals(origin.value, hostString2)
   }
 
-  test("Origin value method should Render a list of multiple hosts") {
-    val origin: Origin = Origin.HostList(NonEmptyList.of(host1, host2))
-    assertEquals(origin.value, s"$hostString1 $hostString2")
-  }
-
   test("Origin value method should Render an empty origin") {
-    val origin: Origin = Origin.Null
+    val origin: Origin = Origin.`null`
     assertEquals(origin.value, "null")
   }
 
   test("OriginHeader parser should Parse a host with a port number") {
     val text = hostString1
-    val origin = Origin.HostList(NonEmptyList.of(host1))
+    val origin = host1
     val headers = Headers(("Origin", text))
     val extracted = headers.get[Origin]
     assertEquals(extracted, Some(origin))
@@ -59,33 +53,31 @@ class OriginHeaderSuite extends munit.FunSuite {
 
   test("OriginHeader parser should Parse a host without a port number") {
     val text = hostString2
-    val origin = Origin.HostList(NonEmptyList.of(host2))
+    val origin = host2
     val headers = Headers(("Origin", text))
     val extracted = headers.get[Origin]
     assertEquals(extracted, Some(origin))
   }
 
-  test("OriginHeader parser should Parse a list of multiple hosts") {
-    val text = s"$hostString1 $hostString2"
-    val origin = Origin.HostList(NonEmptyList.of(host1, host2))
+  test("OriginHeader parser should Parse a 'null' origin") {
+    val text = "null"
+    val origin = Origin.`null`
     val headers = Headers(("Origin", text))
     val extracted = headers.get[Origin]
     assertEquals(extracted, Some(origin))
   }
 
-  test("OriginHeader parser should Parse an empty origin") {
-    val text = ""
-    val origin = Origin.Null
+  test("OriginHeader should fail on a list of multiple hosts") {
+    val text = s"$hostString1 $hostString2"
     val headers = Headers(("Origin", text))
     val extracted = headers.get[Origin]
-    assertEquals(extracted, Some(origin))
+    assertEquals(extracted, None)
   }
 
-  test("OriginHeader parser should Parse a 'null' origin") {
-    val text = "null"
-    val origin = Origin.Null
+  test("OriginHeader should fail on an empty string") {
+    val text = ""
     val headers = Headers(("Origin", text))
     val extracted = headers.get[Origin]
-    assertEquals(extracted, Some(origin))
+    assertEquals(extracted, None)
   }
 }