Repository: http4s/http4s
Pull request #7037 (2023-03-27 05:05:02)

Fix base64 parser for http headers Web-Socket-Key and Web-Socket-Accept
Base64 contains uppercase and lowercase characters (A-Z, a-z), digits (0-9), and the "+" and "/" characters, and ends with 0 to 2 "=" characters. The previously used parser does not allow the "+" and "/" characters and does not check for "=" characters at the end.


Patch:
diff --git a/core/shared/src/main/scala/org/http4s/headers/Sec-WebSocket-Accept.scala b/core/shared/src/main/scala/org/http4s/headers/Sec-WebSocket-Accept.scala
index 12eacd7a6e..8efbeb1171 100644
--- a/core/shared/src/main/scala/org/http4s/headers/Sec-WebSocket-Accept.scala
+++ b/core/shared/src/main/scala/org/http4s/headers/Sec-WebSocket-Accept.scala
@@ -17,18 +17,15 @@
 package org.http4s
 package headers
 
-import cats.parse.Parser
-import cats.parse.Parser.charIn
-import cats.parse.Rfc5234.alpha
-import cats.parse.Rfc5234.digit
+import org.http4s.internal.parsing.Rfc4648
 import org.typelevel.ci._
 import scodec.bits.ByteVector
 
 import java.util.Base64
 import scala.util.Try
 
-final class `Sec-WebSocket-Accept`(hashBytes: ByteVector) {
-  lazy val hashString: String = Base64.getEncoder().encodeToString(hashBytes.toArray)
+final case class `Sec-WebSocket-Accept`(hashedKey: ByteVector) {
+  lazy val hashString: String = Base64.getEncoder().encodeToString(hashedKey.toArrayUnsafe)
 }
 
 object `Sec-WebSocket-Accept` {
@@ -36,17 +33,13 @@ object `Sec-WebSocket-Accept` {
   def parse(s: String): ParseResult[`Sec-WebSocket-Accept`] =
     ParseResult.fromParser(parser, "Invalid Sec-WebSocket-Accept header")(s)
 
-  private[this] val tchar: Parser[Char] = charIn("=").orElse(digit).orElse(alpha)
-
-  private[this] val token: Parser[String] = tchar.rep.string
-
-  private[http4s] val parser = token.mapFilter { t =>
+  private[http4s] val parser = Rfc4648.Base64.token.mapFilter { t =>
     Try(unsafeFromString(t)).toOption
   }
 
   private def unsafeFromString(hash: String): `Sec-WebSocket-Accept` = {
     val bytes = Base64.getDecoder().decode(hash)
-    new `Sec-WebSocket-Accept`(ByteVector(bytes))
+    `Sec-WebSocket-Accept`(ByteVector.view(bytes))
   }
 
   implicit val headerInstance: Header[`Sec-WebSocket-Accept`, Header.Single] =
diff --git a/core/shared/src/main/scala/org/http4s/headers/Sec-WebSocket-Key.scala b/core/shared/src/main/scala/org/http4s/headers/Sec-WebSocket-Key.scala
index 3049d5b85f..38ef5f1de5 100644
--- a/core/shared/src/main/scala/org/http4s/headers/Sec-WebSocket-Key.scala
+++ b/core/shared/src/main/scala/org/http4s/headers/Sec-WebSocket-Key.scala
@@ -17,18 +17,15 @@
 package org.http4s
 package headers
 
-import cats.parse.Parser
-import cats.parse.Parser.charIn
-import cats.parse.Rfc5234.alpha
-import cats.parse.Rfc5234.digit
+import org.http4s.internal.parsing.Rfc4648
 import org.typelevel.ci._
 import scodec.bits.ByteVector
 
 import java.util.Base64
 import scala.util.Try
 
-final class `Sec-WebSocket-Key`(hashBytes: ByteVector) {
-  lazy val hashString: String = Base64.getEncoder().encodeToString(hashBytes.toArray)
+final case class `Sec-WebSocket-Key`(hashedKey: ByteVector) {
+  lazy val hashString: String = Base64.getEncoder().encodeToString(hashedKey.toArrayUnsafe)
 }
 
 object `Sec-WebSocket-Key` {
@@ -36,17 +33,13 @@ object `Sec-WebSocket-Key` {
   def parse(s: String): ParseResult[`Sec-WebSocket-Key`] =
     ParseResult.fromParser(parser, "Invalid Sec-WebSocket-Key header")(s)
 
-  private[this] val tchar: Parser[Char] = charIn("=+/").orElse(digit).orElse(alpha)
-
-  private[this] val token: Parser[String] = tchar.rep.string
-
-  private[http4s] val parser = token.mapFilter { t =>
+  private[http4s] val parser = Rfc4648.Base64.token.mapFilter { t =>
     Try(unsafeFromString(t)).toOption
   }
 
   private def unsafeFromString(hash: String): `Sec-WebSocket-Key` = {
     val bytes = Base64.getDecoder().decode(hash)
-    new `Sec-WebSocket-Key`(ByteVector(bytes))
+    `Sec-WebSocket-Key`(ByteVector.view(bytes))
   }
 
   implicit val headerInstance: Header[`Sec-WebSocket-Key`, Header.Single] =
diff --git a/core/shared/src/main/scala/org/http4s/internal/parsing/Rfc4648.scala b/core/shared/src/main/scala/org/http4s/internal/parsing/Rfc4648.scala
new file mode 100644
index 0000000000..3787869545
--- /dev/null
+++ b/core/shared/src/main/scala/org/http4s/internal/parsing/Rfc4648.scala
@@ -0,0 +1,37 @@
+/*
+ * Copyright 2013 http4s.org
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.http4s
+package internal.parsing
+
+import cats.parse.Parser
+import cats.parse.Parser.char
+import cats.parse.Parser.charIn
+import cats.parse.Rfc5234.alpha
+import cats.parse.Rfc5234.digit
+
+/** Common rules defined in Rfc4648
+  *
+  * @see [[https://datatracker.ietf.org/doc/html/rfc4648]]
+  */
+
+private[http4s] object Rfc4648 {
+  object Base64 {
+    /* https://datatracker.ietf.org/doc/html/rfc4648#page-5 */
+    val token: Parser[String] =
+      (charIn("+/").orElse(digit).orElse(alpha).rep ~ char('=').rep0(0, 2)).string
+  }
+}
diff --git a/laws/src/main/scala/org/http4s/laws/discipline/arbitrary.scala b/laws/src/main/scala/org/http4s/laws/discipline/arbitrary.scala
index b3ee2c695a..46ca983a0c 100644
--- a/laws/src/main/scala/org/http4s/laws/discipline/arbitrary.scala
+++ b/laws/src/main/scala/org/http4s/laws/discipline/arbitrary.scala
@@ -39,9 +39,11 @@ import org.scalacheck._
 import org.scalacheck.rng.Seed
 import org.typelevel.ci.CIString
 import org.typelevel.ci.testing.arbitraries._
+import scodec.bits.ByteVector
 
 import java.nio.charset.{Charset => NioCharset}
 import java.time._
+import java.util.Base64
 import java.util.Locale
 import java.util.concurrent.TimeUnit
 import scala.annotation.nowarn
@@ -631,6 +633,25 @@ private[discipline] trait ArbitraryInstances { this: ArbitraryInstancesBinCompat
       } yield `Transfer-Encoding`(codings)
     }
 
+  implicit val http4sTestingArbitraryForSecWebSocketAcceptHeader
+      : Arbitrary[`Sec-WebSocket-Accept`] =
+    Arbitrary {
+      Gen
+        .containerOfN[Array, Byte](20, getArbitrary[Byte])
+        .map(Base64.getEncoder().encode)
+        .map(ByteVector(_))
+        .map(`Sec-WebSocket-Accept`(_))
+    }
+
+  implicit val http4sTestingArbitraryForSecWebSocketKeyHeader: Arbitrary[`Sec-WebSocket-Key`] =
+    Arbitrary {
+      Gen
+        .containerOfN[Array, Byte](16, getArbitrary[Byte])
+        .map(Base64.getEncoder().encode)
+        .map(ByteVector(_))
+        .map(`Sec-WebSocket-Key`(_))
+    }
+
   implicit val http4sTestingArbitraryForRawHeader: Arbitrary[Header.Raw] =
     Arbitrary {
       for {


Test patch:
diff --git a/tests/shared/src/test/scala/org/http4s/headers/SecWebSocketAcceptSuite.scala b/tests/shared/src/test/scala/org/http4s/headers/SecWebSocketAcceptSuite.scala
new file mode 100644
index 0000000000..4f67951f79
--- /dev/null
+++ b/tests/shared/src/test/scala/org/http4s/headers/SecWebSocketAcceptSuite.scala
@@ -0,0 +1,31 @@
+/*
+ * Copyright 2013 http4s.org
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.http4s
+package headers
+
+import org.http4s.laws.discipline.arbitrary._
+
+class SecWebSocketAcceptSuite extends HeaderLaws {
+  checkAll("Sec-WebSocket-Accept", headerLaws[`Sec-WebSocket-Accept`])
+
+  // https://datatracker.ietf.org/doc/html/rfc6455#page-8
+  val rfc6455ExampleSecWebSocketAccept = "s3pPLMBiTxaQ9kYGzzhZRbK+xOo="
+
+  test("parser accepts RFC 6455 example Sec-WebSocket-Accept") {
+    assert(`Sec-WebSocket-Accept`.parse(rfc6455ExampleSecWebSocketAccept).isRight)
+  }
+}
diff --git a/tests/shared/src/test/scala/org/http4s/headers/SecWebSocketKeySuite.scala b/tests/shared/src/test/scala/org/http4s/headers/SecWebSocketKeySuite.scala
new file mode 100644
index 0000000000..6f4df23ad0
--- /dev/null
+++ b/tests/shared/src/test/scala/org/http4s/headers/SecWebSocketKeySuite.scala
@@ -0,0 +1,31 @@
+/*
+ * Copyright 2013 http4s.org
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.http4s
+package headers
+
+import org.http4s.laws.discipline.arbitrary._
+
+class SecWebSocketKeySuite extends HeaderLaws {
+  checkAll("Sec-WebSocket-Key", headerLaws[`Sec-WebSocket-Key`])
+
+  // https://datatracker.ietf.org/doc/html/rfc6455#page-7
+  val rfc6455ExampleSecWebSocketKey = "dGhlIHNhbXBsZSBub25jZQ=="
+
+  test("parser accepts RFC 6455 example Sec-WebSocket-Key") {
+    assert(`Sec-WebSocket-Key`.parse(rfc6455ExampleSecWebSocketKey).isRight)
+  }
+}