=htc fix off-by-one errors in header name and value length verification

This commit is contained in:
Mathias 2015-10-02 12:57:53 +02:00
parent 1bfff80f6a
commit 78a5a21819
2 changed files with 22 additions and 21 deletions

View file

@ -138,7 +138,7 @@ private[engine] final class HttpHeaderParser private (
}
private def parseRawHeader(input: ByteString, lineStart: Int, cursor: Int, nodeIx: Int): Int = {
val colonIx = scanHeaderNameAndReturnIndexOfColon(input, lineStart, lineStart + maxHeaderNameLength)(cursor)
val colonIx = scanHeaderNameAndReturnIndexOfColon(input, lineStart, lineStart + 1 + maxHeaderNameLength)(cursor)
val headerName = asciiString(input, lineStart, colonIx)
try {
val valueParser = new RawHeaderValueParser(headerName, maxHeaderValueLength, headerValueCacheLimit(headerName))
@ -146,7 +146,7 @@ private[engine] final class HttpHeaderParser private (
parseHeaderLine(input, lineStart)(cursor, nodeIx)
} catch {
case OutOfTrieSpaceException // if we cannot insert we drop back to simply creating new header instances
val (headerValue, endIx) = scanHeaderValue(input, colonIx + 1, colonIx + 1 + maxHeaderValueLength)()
val (headerValue, endIx) = scanHeaderValue(input, colonIx + 1, colonIx + maxHeaderValueLength + 3)()
resultHeader = RawHeader(headerName, headerValue.trim)
endIx
}
@ -449,7 +449,7 @@ private[http] object HttpHeaderParser {
extends HeaderValueParser(headerName, maxValueCount) {
def apply(input: ByteString, valueStart: Int, onIllegalHeader: ErrorInfo Unit): (HttpHeader, Int) = {
// TODO: optimize by running the header value parser directly on the input ByteString (rather than an extracted String)
val (headerValue, endIx) = scanHeaderValue(input, valueStart, valueStart + maxHeaderValueLength)()
val (headerValue, endIx) = scanHeaderValue(input, valueStart, valueStart + maxHeaderValueLength + 2)()
val trimmedHeaderValue = headerValue.trim
val header = HeaderParser.parseFull(headerName, trimmedHeaderValue, settings) match {
case Right(h) h
@ -464,33 +464,32 @@ private[http] object HttpHeaderParser {
class RawHeaderValueParser(headerName: String, maxHeaderValueLength: Int, maxValueCount: Int)
extends HeaderValueParser(headerName, maxValueCount) {
def apply(input: ByteString, valueStart: Int, onIllegalHeader: ErrorInfo Unit): (HttpHeader, Int) = {
val (headerValue, endIx) = scanHeaderValue(input, valueStart, valueStart + maxHeaderValueLength)()
val (headerValue, endIx) = scanHeaderValue(input, valueStart, valueStart + maxHeaderValueLength + 2)()
RawHeader(headerName, headerValue.trim) -> endIx
}
}
@tailrec private def scanHeaderNameAndReturnIndexOfColon(input: ByteString, start: Int,
maxHeaderNameEndIx: Int)(ix: Int = start): Int =
if (ix < maxHeaderNameEndIx)
@tailrec private def scanHeaderNameAndReturnIndexOfColon(input: ByteString, start: Int, limit: Int)(ix: Int = start): Int =
if (ix < limit)
byteChar(input, ix) match {
case ':' ix
case c if tchar(c) scanHeaderNameAndReturnIndexOfColon(input, start, maxHeaderNameEndIx)(ix + 1)
case c if tchar(c) scanHeaderNameAndReturnIndexOfColon(input, start, limit)(ix + 1)
case c fail(s"Illegal character '${escape(c)}' in header name")
}
else fail(s"HTTP header name exceeds the configured limit of ${maxHeaderNameEndIx - start} characters")
else fail(s"HTTP header name exceeds the configured limit of ${limit - start - 1} characters")
@tailrec private def scanHeaderValue(input: ByteString, start: Int, maxHeaderValueEndIx: Int)(sb: JStringBuilder = null, ix: Int = start): (String, Int) = {
@tailrec private def scanHeaderValue(input: ByteString, start: Int, limit: Int)(sb: JStringBuilder = null, ix: Int = start): (String, Int) = {
def spaceAppended = (if (sb != null) sb else new JStringBuilder(asciiString(input, start, ix))).append(' ')
if (ix < maxHeaderValueEndIx)
if (ix < limit)
byteChar(input, ix) match {
case '\t' scanHeaderValue(input, start, maxHeaderValueEndIx)(spaceAppended, ix + 1)
case '\t' scanHeaderValue(input, start, limit)(spaceAppended, ix + 1)
case '\r' if byteChar(input, ix + 1) == '\n'
if (WSP(byteChar(input, ix + 2))) scanHeaderValue(input, start, maxHeaderValueEndIx)(spaceAppended, ix + 3)
if (WSP(byteChar(input, ix + 2))) scanHeaderValue(input, start, limit)(spaceAppended, ix + 3)
else (if (sb != null) sb.toString else asciiString(input, start, ix), ix + 2)
case c if c >= ' ' scanHeaderValue(input, start, maxHeaderValueEndIx)(if (sb != null) sb.append(c) else sb, ix + 1)
case c if c >= ' ' scanHeaderValue(input, start, limit)(if (sb != null) sb.append(c) else sb, ix + 1)
case c fail(s"Illegal character '${escape(c)}' in header value")
}
else fail(s"HTTP header value exceeds the configured limit of ${maxHeaderValueEndIx - start} characters")
else fail(s"HTTP header value exceeds the configured limit of ${limit - start - 2} characters")
}
def fail(summary: String) = throw new ParsingException(StatusCodes.BadRequest, ErrorInfo(summary))

View file

@ -22,8 +22,8 @@ class HttpHeaderParserSpec extends WordSpec with Matchers with BeforeAndAfterAll
val testConf: Config = ConfigFactory.parseString("""
akka.event-handlers = ["akka.testkit.TestEventListener"]
akka.loglevel = ERROR
akka.http.parsing.max-header-name-length = 20
akka.http.parsing.max-header-value-length = 21
akka.http.parsing.max-header-name-length = 60
akka.http.parsing.max-header-value-length = 1000
akka.http.parsing.header-cache.Host = 300""")
val system = ActorSystem(getClass.getSimpleName, testConf)
@ -157,13 +157,15 @@ class HttpHeaderParserSpec extends WordSpec with Matchers with BeforeAndAfterAll
}
"produce an error message for lines with a too-long header name" in new TestSetup() {
the[ParsingException] thrownBy parseLine("123456789012345678901: foo\r\nx") should have message
"HTTP header name exceeds the configured limit of 20 characters"
noException should be thrownBy parseLine("123456789012345678901234567890123456789012345678901234567890: foo\r\nx")
the[ParsingException] thrownBy parseLine("1234567890123456789012345678901234567890123456789012345678901: foo\r\nx") 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() {
the[ParsingException] thrownBy parseLine("foo: 1234567890123456789012\r\nx") should have message
"HTTP header value exceeds the configured limit of 21 characters"
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
"HTTP header value exceeds the configured limit of 1000 characters"
}
"continue parsing raw headers even if the overall cache capacity is reached" in new TestSetup() {