pjfanning commented on code in PR #575:
URL: https://github.com/apache/pekko-http/pull/575#discussion_r1671907247
##########
http-core/src/main/scala/org/apache/pekko/http/scaladsl/model/HttpHeader.scala:
##########
@@ -99,10 +99,11 @@ object HttpHeader {
case HeaderParser.RuleNotFound => ParsingResult.Ok(RawHeader(name,
preProcessedValue), Nil)
}
case Failure(error) =>
- val info = (error match {
- case e: ParseError => parser.parseError(e)
- case e => parser.failure(e)
- }).info
+ val info = error match {
+ case e: ParseError => parser.parseError(e).info
+ case e if e.getMessage == null => ErrorInfo()
Review Comment:
I think this code should be reverted and the change should be in
HeaderParser.scala instead. I have the change in #576. I created 576 to test if
that approach works but I'd prefer to let you complete this PR. View 576 as a
set of suggested changes for this PR.
##########
http-core/src/test/scala/org/apache/pekko/http/impl/model/parser/HttpHeaderSpec.scala:
##########
@@ -876,6 +876,11 @@ class HttpHeaderSpec extends AnyFreeSpec with Matchers {
parse("User-Agent", "(" * 10000).errors.head shouldEqual
ErrorInfo("Illegal HTTP header 'User-Agent': Illegal header value",
"Header comment nested too deeply")
}
+
+ "should not broken when header-value is null" in {
+ parse("Content-Disposition", null).errors.head.isInstanceOf[ErrorInfo]
shouldBe true
Review Comment:
Please use:
```
parse("Content-Disposition", null).errors.head shouldBe an[ErrorInfo]
```
Your version will return an error like `false os not true` if it fails. My
version will return a much more informative message.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@pekko.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@pekko.apache.org
For additional commands, e-mail: notifications-h...@pekko.apache.org
