As mentioned in akka#106, parsers should also accept LF end of line a…

…s good as CRLF. * Add tests verifying `\n` alongside `\r\n` in all modified parsers * Duplicate specs to test either with `\r\n` and with `\n` (force to test in both cases for future developments)
grizio · Jan 28, 2017 · 1c654c6 · 1c654c6
1 parent 70dadad
commit 1c654c6
Show file tree

Hide file tree

Showing 9 changed files with 72 additions and 46 deletions.
diff --git a/akka-http-core/src/main/scala/akka/http/impl/engine/parsing/HttpHeaderParser.scala b/akka-http-core/src/main/scala/akka/http/impl/engine/parsing/HttpHeaderParser.scala
@@ -463,6 +463,7 @@ private[http] object HttpHeaderParser {
     insertInGoodOrder(specializedHeaderValueParsers)()
     insertInGoodOrder(predefinedHeaders.sorted)()
     parser.insert(ByteString("\r\n"), EmptyHeader)()
+    parser.insert(ByteString("\n"), EmptyHeader)()
     parser
   }
 
@@ -522,6 +523,9 @@ private[http] object HttpHeaderParser {
         case '\r' if byteChar(input, ix + 1) == '\n' ⇒
           if (WSP(byteChar(input, ix + 2))) scanHeaderValue(hhp, input, start, limit, log, mode)(appended(' '), ix + 3)
           else (if (sb != null) sb.toString else asciiString(input, start, ix), ix + 2)
+        case '\n' ⇒
+          if (WSP(byteChar(input, ix + 1))) scanHeaderValue(hhp, input, start, limit, log, mode)(appended(' '), ix + 2)
+          else (if (sb != null) sb.toString else asciiString(input, start, ix), ix + 1)
         case c ⇒
           var nix = ix + 1
           val nsb =

diff --git a/akka-http-core/src/main/scala/akka/http/impl/engine/parsing/HttpMessageParser.scala b/akka-http-core/src/main/scala/akka/http/impl/engine/parsing/HttpMessageParser.scala
@@ -242,6 +242,7 @@ private[http] trait HttpMessageParser[Output >: MessageOutput <: ParserOutput] {
           case c if CharacterClasses.HEXDIG(c) ⇒ parseSize(cursor + 1, size * 16 + CharUtils.hexValue(c))
           case ';' if cursor > offset ⇒ parseChunkExtensions(size.toInt, cursor + 1)()
           case '\r' if cursor > offset && byteChar(input, cursor + 1) == '\n' ⇒ parseChunkBody(size.toInt, "", cursor + 2)
+          case '\n' if cursor > offset ⇒ parseChunkBody(size.toInt, "", cursor + 1)
           case c ⇒ failEntityStream(s"Illegal character '${escape(c)}' in chunk start")
         }
       } else failEntityStream(s"HTTP chunk size exceeds the configured limit of ${settings.maxChunkSize} bytes")

diff --git a/akka-http-core/src/main/scala/akka/http/impl/engine/parsing/HttpRequestParser.scala b/akka-http-core/src/main/scala/akka/http/impl/engine/parsing/HttpRequestParser.scala
@@ -73,6 +73,8 @@ private[http] final class HttpRequestParser(
       cursor = parseProtocol(input, cursor)
       if (byteChar(input, cursor) == '\r' && byteChar(input, cursor + 1) == '\n')
         parseHeaderLines(input, cursor + 2)
+      else if (byteChar(input, cursor) == '\n')
+        parseHeaderLines(input, cursor + 1)
       else onBadProtocol
     }
 

diff --git a/akka-http-core/src/main/scala/akka/http/impl/engine/parsing/HttpResponseParser.scala b/akka-http-core/src/main/scala/akka/http/impl/engine/parsing/HttpResponseParser.scala
@@ -89,11 +89,12 @@ private[http] class HttpResponseParser(protected val settings: ParserSettings, p
       @tailrec def skipReason(idx: Int): Int =
         if (idx - startIdx <= maxResponseReasonLength)
           if (byteChar(input, idx) == '\r' && byteChar(input, idx + 1) == '\n') idx + 2
+          else if (byteChar(input, idx) == '\n') idx + 1
           else skipReason(idx + 1)
         else throw new ParsingException("Response reason phrase exceeds the configured limit of " +
           maxResponseReasonLength + " characters")
       skipReason(startIdx)
-    } else if (byteChar(input, cursor + 3) == '\r' && byteChar(input, cursor + 4) == '\n') {
+    } else if (byteChar(input, cursor + 3) == '\n' || byteChar(input, cursor + 3) == '\r' && byteChar(input, cursor + 4) == '\n') {
       throw new ParsingException("Status code misses trailing space")
     } else badStatusCode
   }

diff --git a/...ttp-core/src/main/scala/akka/http/impl/engine/parsing/SpecializedHeaderValueParsers.scala b/...ttp-core/src/main/scala/akka/http/impl/engine/parsing/SpecializedHeaderValueParsers.scala
@@ -26,6 +26,7 @@ private object SpecializedHeaderValueParsers {
         else if (DIGIT(c)) recurse(ix + 1, result * 10 + c - '0')
         else if (WSP(c)) recurse(ix + 1, result)
         else if (c == '\r' && byteChar(input, ix + 1) == '\n') (`Content-Length`(result), ix + 2)
+        else if (c == '\n') (`Content-Length`(result), ix + 1)
         else fail("Illegal `Content-Length` header value")
       }
       recurse()

diff --git a/...ttp-core/src/test/scala/akka/http/impl/engine/parsing/ContentLengthHeaderParserSpec.scala b/...ttp-core/src/test/scala/akka/http/impl/engine/parsing/ContentLengthHeaderParserSpec.scala
@@ -9,9 +9,9 @@ import akka.util.ByteString
 import akka.http.scaladsl.model.headers.`Content-Length`
 import akka.http.impl.engine.parsing.SpecializedHeaderValueParsers.ContentLengthParser
 
-class ContentLengthHeaderParserSpec extends WordSpec with Matchers {
+abstract class ContentLengthHeaderParserSpec(mode: String, newLine: String) extends WordSpec with Matchers {
 
-  "specialized ContentLength parser" should {
+  s"specialized ContentLength parser (mode: $mode)" should {
     "accept zero" in {
       parse("0") shouldEqual 0L
     }
@@ -30,8 +30,12 @@ class ContentLengthHeaderParserSpec extends WordSpec with Matchers {
   }
 
   def parse(bigint: String): Long = {
-    val (`Content-Length`(length), _) = ContentLengthParser(null, ByteString(bigint + "\r\n").compact, 0, _ ⇒ ())
+    val (`Content-Length`(length), _) = ContentLengthParser(null, ByteString(bigint + newLine).compact, 0, _ ⇒ ())
     length
   }
 
 }
+
+class ContentLengthHeaderParserCRLFSpec extends ContentLengthHeaderParserSpec("CRLF", "\r\n")
+
+class ContentLengthHeaderParserLFSpec extends ContentLengthHeaderParserSpec("LF", "\n")
diff --git a/akka-http-core/src/test/scala/akka/http/impl/engine/parsing/HttpHeaderParserSpec.scala b/akka-http-core/src/test/scala/akka/http/impl/engine/parsing/HttpHeaderParserSpec.scala
@@ -20,7 +20,7 @@ import akka.http.impl.model.parser.CharacterClasses
 import akka.http.impl.util._
 import akka.testkit.TestKit
 
-class HttpHeaderParserSpec extends WordSpec with Matchers with BeforeAndAfterAll {
+abstract class HttpHeaderParserSpec(mode: String, newLine: String) extends WordSpec with Matchers with BeforeAndAfterAll {
 
   val testConf: Config = ConfigFactory.parseString("""
     akka.event-handlers = ["akka.testkit.TestEventListener"]
@@ -30,7 +30,7 @@ class HttpHeaderParserSpec extends WordSpec with Matchers with BeforeAndAfterAll
     akka.http.parsing.header-cache.Host = 300""")
   val system = ActorSystem(getClass.getSimpleName, testConf)
 
-  "The HttpHeaderParser" should {
+  s"The HttpHeaderParser (mode: $mode)" should {
     "insert the 1st value" in new TestSetup(primed = false) {
       insert("Hello", 'Hello)
       check {
@@ -111,39 +111,40 @@ class HttpHeaderParserSpec extends WordSpec with Matchers with BeforeAndAfterAll
     }
 
     "retrieve the EmptyHeader" in new TestSetup() {
-      parseAndCache("\r\n")() shouldEqual EmptyHeader
+      parseAndCache(newLine)() shouldEqual EmptyHeader
     }
 
     "retrieve a cached header with an exact header name match" in new TestSetup() {
-      parseAndCache("Connection: close\r\nx")() shouldEqual Connection("close")
+      parseAndCache(s"Connection: close${newLine}x")() shouldEqual Connection("close")
     }
 
     "retrieve a cached header with a case-insensitive header-name match" in new TestSetup() {
-      parseAndCache("Connection: close\r\nx")("coNNection: close\r\nx") shouldEqual Connection("close")
+      parseAndCache(s"Connection: close${newLine}x")(s"coNNection: close${newLine}x") shouldEqual Connection("close")
     }
 
     "parse and cache a modelled header" in new TestSetup() {
-      parseAndCache("Host: spray.io:123\r\nx")("HOST: spray.io:123\r\nx") shouldEqual Host("spray.io", 123)
+      parseAndCache(s"Host: spray.io:123${newLine}x")(s"HOST: spray.io:123${newLine}x") shouldEqual Host("spray.io", 123)
     }
 
     "parse and cache an invalid modelled header as RawHeader" in new TestSetup() {
-      parseAndCache("Content-Type: abc:123\r\nx")() shouldEqual RawHeader("content-type", "abc:123")
-      parseAndCache("Origin: localhost:8080\r\nx")() shouldEqual RawHeader("origin", "localhost:8080")
+      parseAndCache(s"Content-Type: abc:123${newLine}x")() shouldEqual RawHeader("content-type", "abc:123")
+      parseAndCache(s"Origin: localhost:8080${newLine}x")() shouldEqual RawHeader("origin", "localhost:8080")
     }
 
     "parse and cache an X-Forwarded-For with a hostname in it as a RawHeader" in new TestSetup() {
-      parseAndCache("X-Forwarded-For: 1.2.3.4, akka.io\r\nx")() shouldEqual RawHeader("x-forwarded-for", "1.2.3.4, akka.io")
+      parseAndCache(s"X-Forwarded-For: 1.2.3.4, akka.io${newLine}x")() shouldEqual RawHeader("x-forwarded-for", "1.2.3.4, akka.io")
     }
 
     "parse and cache an X-Real-Ip with a hostname as it's value as a RawHeader" in new TestSetup() {
-      parseAndCache("X-Real-Ip: akka.io\r\nx")() shouldEqual RawHeader("x-real-ip", "akka.io")
+      parseAndCache(s"X-Real-Ip: akka.io${newLine}x")() shouldEqual RawHeader("x-real-ip", "akka.io")
     }
     "parse and cache a raw header" in new TestSetup(primed = false) {
       insert("hello: bob", 'Hello)
-      val (ixA, headerA) = parseLine("Fancy-Pants: foo\r\nx")
-      val (ixB, headerB) = parseLine("Fancy-pants: foo\r\nx")
+      val (ixA, headerA) = parseLine(s"Fancy-Pants: foo${newLine}x")
+      val (ixB, headerB) = parseLine(s"Fancy-pants: foo${newLine}x")
+      val newLineWithHyphen = if (newLine == "\r\n") """\r-\n""" else """\n"""
       check {
-        """ ┌─f-a-n-c-y---p-a-n-t-s-:-(Fancy-Pants)- -f-o-o-\r-\n- *Fancy-Pants: foo
+        s""" ┌─f-a-n-c-y---p-a-n-t-s-:-(Fancy-Pants)- -f-o-o-${newLineWithHyphen}- *Fancy-Pants: foo
           |-h-e-l-l-o-:- -b-o-b- 'Hello
           |""" → parser.formatTrie
       }
@@ -153,35 +154,35 @@ class HttpHeaderParserSpec extends WordSpec with Matchers with BeforeAndAfterAll
     }
 
     "parse and cache a modelled header with line-folding" in new TestSetup() {
-      parseAndCache("Connection: foo,\r\n bar\r\nx")("Connection: foo,\r\n bar\r\nx") shouldEqual Connection("foo", "bar")
+      parseAndCache(s"Connection: foo,${newLine} bar${newLine}x")(s"Connection: foo,${newLine} bar${newLine}x") shouldEqual Connection("foo", "bar")
     }
 
     "parse and cache a header with a tab char in the value" in new TestSetup() {
-      parseAndCache("Fancy: foo\tbar\r\nx")() shouldEqual RawHeader("Fancy", "foo bar")
+      parseAndCache(s"Fancy: foo\tbar${newLine}x")() shouldEqual RawHeader("Fancy", "foo bar")
     }
 
     "parse and cache a header with UTF8 chars in the value" in new TestSetup() {
-      parseAndCache("2-UTF8-Bytes: árvíztűrő ütvefúrógép\r\nx")() shouldEqual RawHeader("2-UTF8-Bytes", "árvíztűrő ütvefúrógép")
-      parseAndCache("3-UTF8-Bytes: The € or the $?\r\nx")() shouldEqual RawHeader("3-UTF8-Bytes", "The € or the $?")
-      parseAndCache("4-UTF8-Bytes: Surrogate pairs: \uD801\uDC1B\uD801\uDC04\uD801\uDC1B!\r\nx")() shouldEqual
+      parseAndCache(s"2-UTF8-Bytes: árvíztűrő ütvefúrógép${newLine}x")() shouldEqual RawHeader("2-UTF8-Bytes", "árvíztűrő ütvefúrógép")
+      parseAndCache(s"3-UTF8-Bytes: The € or the $$?${newLine}x")() shouldEqual RawHeader("3-UTF8-Bytes", "The € or the $?")
+      parseAndCache(s"4-UTF8-Bytes: Surrogate pairs: \uD801\uDC1B\uD801\uDC04\uD801\uDC1B!${newLine}x")() shouldEqual
         RawHeader("4-UTF8-Bytes", "Surrogate pairs: \uD801\uDC1B\uD801\uDC04\uD801\uDC1B!")
     }
 
     "produce an error message for lines with an illegal header name" in new TestSetup() {
-      the[ParsingException] thrownBy parseLine(" Connection: close\r\nx") should have message "Illegal character ' ' in header name"
-      the[ParsingException] thrownBy parseLine("Connection : close\r\nx") should have message "Illegal character ' ' in header name"
-      the[ParsingException] thrownBy parseLine("Connec/tion: close\r\nx") should have message "Illegal character '/' in header name"
+      the[ParsingException] thrownBy parseLine(s" Connection: close${newLine}x") should have message "Illegal character ' ' in header name"
+      the[ParsingException] thrownBy parseLine(s"Connection : close${newLine}x") should have message "Illegal character ' ' in header name"
+      the[ParsingException] thrownBy parseLine(s"Connec/tion: close${newLine}x") should have message "Illegal character '/' in header name"
     }
 
     "produce an error message for lines with a too-long header name" in new TestSetup() {
-      noException should be thrownBy parseLine("123456789012345678901234567890123456789012345678901234567890: foo\r\nx")
-      the[ParsingException] thrownBy parseLine("1234567890123456789012345678901234567890123456789012345678901: foo\r\nx") should have message
+      noException should be thrownBy parseLine(s"123456789012345678901234567890123456789012345678901234567890: foo${newLine}x")
+      the[ParsingException] thrownBy parseLine(s"1234567890123456789012345678901234567890123456789012345678901: foo${newLine}x") should have message
         "HTTP header name exceeds the configured limit of 60 characters"
     }
 
     "produce an error message for lines with a too-long header value" in new TestSetup() {
-      noException should be thrownBy parseLine(s"foo: ${nextRandomString(nextRandomAlphaNumChar, 1000)}\r\nx")
-      the[ParsingException] thrownBy parseLine(s"foo: ${nextRandomString(nextRandomAlphaNumChar, 1001)}\r\nx") should have message
+      noException should be thrownBy parseLine(s"foo: ${nextRandomString(nextRandomAlphaNumChar, 1000)}${newLine}x")
+      the[ParsingException] thrownBy parseLine(s"foo: ${nextRandomString(nextRandomAlphaNumChar, 1001)}${newLine}x") should have message
         "HTTP header value exceeds the configured limit of 1000 characters"
     }
 
@@ -192,7 +193,7 @@ class HttpHeaderParserSpec extends WordSpec with Matchers with BeforeAndAfterAll
         RawHeader(name, value)
       }
       randomHeaders.take(300).foldLeft(0) {
-        case (acc, rawHeader) ⇒ acc + parseAndCache(rawHeader.toString + "\r\nx", rawHeader)
+        case (acc, rawHeader) ⇒ acc + parseAndCache(rawHeader.toString + s"${newLine}x", rawHeader)
       } should be < 300 // number of cache hits is smaller headers successfully parsed
     }
 
@@ -203,7 +204,7 @@ class HttpHeaderParserSpec extends WordSpec with Matchers with BeforeAndAfterAll
           port = nextRandomInt(1000, 10000))
       }
       randomHostHeaders.take(300).foldLeft(0) {
-        case (acc, header) ⇒ acc + parseAndCache(header.toString + "\r\nx", header)
+        case (acc, header) ⇒ acc + parseAndCache(header.toString + s"${newLine}x", header)
       } should be < 300 // number of cache hits is smaller headers successfully parsed
     }
 
@@ -214,7 +215,7 @@ class HttpHeaderParserSpec extends WordSpec with Matchers with BeforeAndAfterAll
           value = nextRandomString(nextRandomAlphaNumChar, 1000))
       }
       randomHostHeaders.take(100).foldLeft(0) {
-        case (acc, header) ⇒ acc + parseAndCache(header.toString + "\r\nx", header)
+        case (acc, header) ⇒ acc + parseAndCache(header.toString + s"${newLine}x", header)
       } should be < 300 // number of cache hits is smaller headers successfully parsed
     }
 
@@ -224,7 +225,7 @@ class HttpHeaderParserSpec extends WordSpec with Matchers with BeforeAndAfterAll
         RawHeader("Fancy", value)
       }
       randomHeaders.take(20).foldLeft(0) {
-        case (acc, rawHeader) ⇒ acc + parseAndCache(rawHeader.toString + "\r\nx", rawHeader)
+        case (acc, rawHeader) ⇒ acc + parseAndCache(rawHeader.toString + s"${newLine}x", rawHeader)
       } shouldEqual 12 // configured default per-header cache limit
     }
 
@@ -233,7 +234,7 @@ class HttpHeaderParserSpec extends WordSpec with Matchers with BeforeAndAfterAll
         `User-Agent`(nextRandomString(nextRandomAlphaNumChar, nextRandomInt(4, 16)))
       }
       randomHeaders.take(40).foldLeft(0) {
-        case (acc, header) ⇒ acc + parseAndCache(header.toString + "\r\nx", header)
+        case (acc, header) ⇒ acc + parseAndCache(header.toString + s"${newLine}x", header)
       } shouldEqual 12 // configured default per-header cache limit
     }
   }
@@ -287,3 +288,7 @@ class HttpHeaderParserSpec extends WordSpec with Matchers with BeforeAndAfterAll
       if (sb.length < len) nextRandomString(charGen, len, sb.append(charGen())) else sb.toString
   }
 }
+
+class HttpHeaderParserCRLFSpec extends HttpHeaderParserSpec("CRLF", "\r\n")
+
+class HttpHeaderParserLFSpec extends HttpHeaderParserSpec("LF", "\n")
diff --git a/akka-http-core/src/test/scala/akka/http/impl/engine/parsing/RequestParserSpec.scala b/akka-http-core/src/test/scala/akka/http/impl/engine/parsing/RequestParserSpec.scala
@@ -31,7 +31,7 @@ import akka.http.scaladsl.util.FastFuture
 import akka.http.scaladsl.util.FastFuture._
 import akka.testkit.TestKit
 
-class RequestParserSpec extends FreeSpec with Matchers with BeforeAndAfterAll {
+abstract class RequestParserSpec(mode: String, newLine: String) extends FreeSpec with Matchers with BeforeAndAfterAll {
   val testConf: Config = ConfigFactory.parseString("""
     akka.event-handlers = ["akka.testkit.TestEventListener"]
     akka.loglevel = WARNING
@@ -44,7 +44,7 @@ class RequestParserSpec extends FreeSpec with Matchers with BeforeAndAfterAll {
   val BOLT = HttpMethod.custom("BOLT", safe = false, idempotent = true, requestEntityAcceptance = Expected)
   implicit val materializer = ActorMaterializer()
 
-  "The request parsing logic should" - {
+  s"The request parsing logic should (mode: $mode)" - {
     "properly parse a request" - {
       "with no headers and no body" in new Test {
         """GET / HTTP/1.0
@@ -275,8 +275,8 @@ class RequestParserSpec extends FreeSpec with Matchers with BeforeAndAfterAll {
 
         val x = NotEnoughDataException
         val numChunks = 12000 // failed starting from 4000 with sbt started with `-Xss2m`
-        val oneChunk = "1\r\nz\n"
-        val manyChunks = (oneChunk * numChunks) + "0\r\n"
+        val oneChunk = s"1${newLine}z\n"
+        val manyChunks = (oneChunk * numChunks) + s"0${newLine}"
 
         val parser = newParser
         val result = multiParse(newParser)(Seq(prep(start + manyChunks)))
@@ -596,8 +596,12 @@ class RequestParserSpec extends FreeSpec with Matchers with BeforeAndAfterAll {
       data.limit(100000).runWith(Sink.seq)
         .fast.recover { case _: NoSuchElementException ⇒ Nil }
 
-    def prep(response: String) = response.stripMarginWithNewline("\r\n")
+    def prep(response: String) = response.stripMarginWithNewline(newLine)
   }
 
   def source[T](elems: T*): Source[T, NotUsed] = Source(elems.toList)
 }
+
+class RequestParserCRLFSpec extends RequestParserSpec("CRLF", "\r\n")
+
+class RequestParserLFSpec extends RequestParserSpec("LF", "\n")