apache/pekko-http

[feature request] Internal server error triggered by “Accept-Charset” header with unexpected value

He-Pin opened this issue · 6 comments

Could you ask for the stacktrace so that we don't have to run the samples for ourselves?

As long as the unexpected header doesn't cause a server side crash - this would likely to be quite a minor issue.

As long as the unexpected header doesn't cause a server side crash - this would likely to be quite a minor issue.

If we are leaking a stack trace to the client this can be a serious security issue

Yes, this is a bug but only a minor issue.

Here's a stack trace (though, wrapped with another exception to understand how that exception can be triggered):

[info] java.lang.IllegalArgumentException: Unsupported charset: asd
[info] 	at org.apache.pekko.http.scaladsl.model.HttpCharset$$anonfun$nioCharset$1.applyOrElse(HttpCharset.scala:68)
[info] 	at org.apache.pekko.http.scaladsl.model.HttpCharset$$anonfun$nioCharset$1.applyOrElse(HttpCharset.scala:67)
[info] 	at scala.util.Failure.recover(Try.scala:233)
[info] 	at org.apache.pekko.http.scaladsl.model.HttpCharset.nioCharset(HttpCharset.scala:67)
[info] 	at org.apache.pekko.http.scaladsl.model.HttpEntity$.apply(HttpEntity.scala:310)
[info] 	at org.apache.pekko.http.scaladsl.marshalling.PredefinedToEntityMarshallers.$anonfun$stringMarshaller$1(PredefinedToEntityMarshallers.scala:58)
[info] 	at org.apache.pekko.http.scaladsl.marshalling.Marshaller$$anon$3.$anonfun$apply$2(Marshaller.scala:161
[info] 	at org.apache.pekko.http.scaladsl.marshalling.Marshalling$WithOpenCharset.$anonfun$map$6(Marshaller.scala:219)
[info] 	at org.apache.pekko.http.scaladsl.marshalling.Marshal$$anonfun$selectMarshallingForContentType$2.$anonfun$applyOrElse$1(Marshal.scala:39)
[info] 	at org.apache.pekko.http.scaladsl.marshalling.Marshal.$anonfun$toResponseFor$1(Marshal.scala:84)
[info] 	at org.apache.pekko.http.scaladsl.util.FastFuture$.$anonfun$map$1(FastFuture.scala:31)
[info] 	at org.apache.pekko.http.scaladsl.util.FastFuture$.strictTransform$1(FastFuture.scala:49)
[info] 	at org.apache.pekko.http.scaladsl.util.FastFuture$.transformWith$extension(FastFuture.scala:53)
[info] 	at org.apache.pekko.http.scaladsl.util.FastFuture$.map$extension(FastFuture.scala:31)
[info] 	at org.apache.pekko.http.scaladsl.marshalling.Marshal.toResponseFor(Marshal.scala:68)
[info] 	at org.apache.pekko.http.scaladsl.marshalling.ToResponseMarshallable.apply(ToResponseMarshallable.scala:26)
[info] 	at org.apache.pekko.http.scaladsl.marshalling.ToResponseMarshallable.apply$(ToResponseMarshallable.scala:25)
[info] 	at org.apache.pekko.http.scaladsl.marshalling.ToResponseMarshallable$$anon$1.apply(ToResponseMarshallable.scala:31)
[info] 	at org.apache.pekko.http.scaladsl.server.RequestContextImpl.complete(RequestContextImpl.scala:51)
[info] 	at org.apache.pekko.http.scaladsl.server.directives.RouteDirectives.$anonfun$complete$1(RouteDirectives.scala:61)
[info] 	at org.apache.pekko.http.scaladsl.server.StandardRoute$$anon$1.apply(StandardRoute.scala:28)
[info] 	at org.apache.pekko.http.scaladsl.server.StandardRoute$$anon$1.apply(StandardRoute.scala:28)
[info] 	at org.apache.pekko.http.scaladsl.server.directives.BasicDirectives.$anonfun$textract$2(BasicDirectives.scala:173)
[info] 	at org.apache.pekko.http.scaladsl.server.directives.BasicDirectives.$anonfun$textract$2(BasicDirectives.scala:173)
[info] 	at org.apache.pekko.http.scaladsl.server.directives.BasicDirectives.$anonfun$mapRouteResultWith$2(BasicDirectives.scala:86)
[info] 	at org.apache.pekko.http.scaladsl.server.directives.ExecutionDirectives.handle$1(ExecutionDirectives.scala:63)
[info] 	at org.apache.pekko.http.scaladsl.server.directives.ExecutionDirectives.$anonfun$handleRejections$4(ExecutionDirectives.scala:72)
[info] 	at org.apache.pekko.http.scaladsl.server.directives.BasicDirectives$$anonfun$recoverRejectionsWith$1.applyOrElse(BasicDirectives.scala:110)
[info] 	at org.apache.pekko.http.scaladsl.server.directives.BasicDirectives$$anonfun$recoverRejectionsWith$1.applyOrElse(BasicDirectives.scala:110)
[info] 	at org.apache.pekko.http.scaladsl.server.directives.BasicDirectives.$anonfun$mapRouteResultWithPF$1(BasicDirectives.scala:98)
[info] 	at org.apache.pekko.http.scaladsl.util.FastFuture$.strictTransform$1(FastFuture.scala:49)
[info] 	at org.apache.pekko.http.scaladsl.util.FastFuture$.transformWith$extension(FastFuture.scala:53)
[info] 	at org.apache.pekko.http.scaladsl.util.FastFuture$.flatMap$extension(FastFuture.scala:34)
[info] 	at org.apache.pekko.http.scaladsl.server.directives.BasicDirectives.$anonfun$mapRouteResultWith$2(BasicDirectives.scala:86)
[info] 	at org.apache.pekko.http.scaladsl.server.directives.BasicDirectives.$anonfun$textract$2(BasicDirectives.scala:173)
[info] 	at org.apache.pekko.http.scaladsl.server.directives.ExecutionDirectives.$anonfun$handleExceptions$2(ExecutionDirectives.scala:42)
[info] 	at org.apache.pekko.http.scaladsl.server.Route$.$anonfun$createAsyncHandler$1(Route.scala:127)
[info] 	at org.apache.pekko.stream.impl.fusing.MapAsyncUnordered$$anon$31.onPush(Ops.scala:1443)
[info] 	at org.apache.pekko.stream.impl.fusing.GraphInterpreter.processPush(GraphInterpreter.scala:555)
[info] 	at org.apache.pekko.stream.impl.fusing.GraphInterpreter.processEvent(GraphInterpreter.scala:506)
[info] 	at org.apache.pekko.stream.impl.fusing.GraphInterpreter.execute(GraphInterpreter.scala:400)
[info] 	at org.apache.pekko.stream.impl.fusing.GraphInterpreterShell.runBatch(ActorGraphInterpreter.scala:662)
[info] 	at org.apache.pekko.stream.impl.fusing.GraphInterpreterShell$AsyncInput.execute(ActorGraphInterpreter.scala:532)
[info] 	at org.apache.pekko.stream.impl.fusing.GraphInterpreterShell.processEvent(ActorGraphInterpreter.scala:637)
[info] 	at org.apache.pekko.stream.impl.fusing.ActorGraphInterpreter.org$apache$pekko$stream$impl$fusing$ActorGraphInterpreter$$processEvent(ActorGraphInterpreter.scala:813)
[info] 	at org.apache.pekko.stream.impl.fusing.ActorGraphInterpreter$$anonfun$receive$1.applyOrElse(ActorGraphInterpreter.scala:831)
[info] 	at org.apache.pekko.actor.Actor.aroundReceive(Actor.scala:547)
[info] 	at org.apache.pekko.actor.Actor.aroundReceive$(Actor.scala:545)
[info] 	at org.apache.pekko.stream.impl.fusing.ActorGraphInterpreter.aroundReceive(ActorGraphInterpreter.scala:729)
[info] 	at org.apache.pekko.actor.ActorCell.receiveMessage(ActorCell.scala:590)
[info] 	at org.apache.pekko.actor.ActorCell.invoke(ActorCell.scala:557)
[info] 	at org.apache.pekko.dispatch.Mailbox.processMailbox(Mailbox.scala:280)
[info] 	at org.apache.pekko.dispatch.Mailbox.run(Mailbox.scala:241)
[info] 	at org.apache.pekko.dispatch.Mailbox.exec(Mailbox.scala:253)
[info] 	at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
[info] 	at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
[info] 	at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
[info] 	at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:175)
[info] Caused by: java.nio.charset.UnsupportedCharsetException: asd
[info] 	at java.nio.charset.Charset.forName(Charset.java:531)
[info] 	at org.apache.pekko.http.scaladsl.model.HttpCharset$.$anonfun$findNioCharset$1(HttpCharset.scala:90)
[info] 	at scala.util.Try$.apply(Try.scala:210)
[info] 	at org.apache.pekko.http.scaladsl.model.HttpCharset$.findNioCharset(HttpCharset.scala:90)
[info] 	at org.apache.pekko.http.scaladsl.model.HttpCharset.<init>(HttpCharset.scala:64)
[info] 	at org.apache.pekko.http.scaladsl.model.HttpCharset$.apply(HttpCharset.scala:62)
[info] 	at org.apache.pekko.http.scaladsl.model.HttpCharset$.custom(HttpCharset.scala:88)
[info] 	at org.apache.pekko.http.impl.model.parser.CommonActions.$anonfun$getCharset$1(CommonActions.scala:71)
[info] 	at scala.Option.getOrElse(Option.scala:201)
[info] 	at org.apache.pekko.http.impl.model.parser.CommonActions.getCharset(CommonActions.scala:71)
[info] 	at org.apache.pekko.http.impl.model.parser.CommonActions.getCharset$(CommonActions.scala:68)
[info] 	at org.apache.pekko.http.impl.model.parser.HeaderParser.getCharset(HeaderParser.scala:37)
[info] 	at org.apache.pekko.http.impl.model.parser.AcceptCharsetHeader.charset$minusrange$minusdef(AcceptCharsetHeader.scala:38)
[info] 	at org.apache.pekko.http.impl.model.parser.AcceptCharsetHeader.charset$minusrange$minusdef$(AcceptCharsetHeader.scala:37)
[info] 	at org.apache.pekko.http.impl.model.parser.HeaderParser.charset$minusrange$minusdef(HeaderParser.scala:37)
[info] 	at org.apache.pekko.http.impl.model.parser.AcceptCharsetHeader.charset$minusrange$minusdecl(AcceptCharsetHeader.scala:29)
[info] 	at org.apache.pekko.http.impl.model.parser.AcceptCharsetHeader.charset$minusrange$minusdecl$(AcceptCharsetHeader.scala:28)
[info] 	at org.apache.pekko.http.impl.model.parser.HeaderParser.charset$minusrange$minusdecl(HeaderParser.scala:37)
[info] 	at org.apache.pekko.http.impl.model.parser.AcceptCharsetHeader.rec$2(AcceptCharsetHeader.scala:25)
[info] 	at org.apache.pekko.http.impl.model.parser.AcceptCharsetHeader.accept$minuscharset(AcceptCharsetHeader.scala:24)
[info] 	at org.apache.pekko.http.impl.model.parser.AcceptCharsetHeader.accept$minuscharset$(AcceptCharsetHeader.scala:24)
[info] 	at org.apache.pekko.http.impl.model.parser.HeaderParser.accept$minuscharset(HeaderParser.scala:37)
[info] 	at org.apache.pekko.http.impl.model.parser.HeaderParser$$anon$1$$anon$60.$anonfun$apply$59(HeaderParser.scala:141)
[info] 	at org.parboiled2.Parser.__run(Parser.scala:125)
[info] 	at org.apache.pekko.http.impl.model.parser.HeaderParser$$anon$1$$anon$60.apply(HeaderParser.scala:141)
[info] 	at org.parboiled2.DynamicRuleDispatch.$anonfun$apply$1(DynamicRuleDispatch.scala:36)
[info] 	at scala.Option.map(Option.scala:242)
[info] 	at org.parboiled2.DynamicRuleDispatch.apply(DynamicRuleDispatch.scala:36)
[info] 	at org.parboiled2.DynamicRuleDispatch.apply$(DynamicRuleDispatch.scala:35)
[info] 	at org.apache.pekko.http.impl.model.parser.HeaderParser$$anon$1.apply(HeaderParser.scala:141)
[info] 	at org.apache.pekko.http.impl.model.parser.HeaderParser$.$anonfun$lookupParser$1(HeaderParser.scala:126)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpHeaderParser$ModeledHeaderValueParser.apply(HttpHeaderParser.scala:570)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpHeaderParser.startValueBranch$1(HttpHeaderParser.scala:125)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpHeaderParser.parseHeaderLine(HttpHeaderParser.scala:145)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpMessageParser.parseHeaderLines(HttpMessageParser.scala:158)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpMessageParser.parseHeaderLines$(HttpMessageParser.scala:148)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpRequestParser$$anon$1.parseHeaderLines(HttpRequestParser.scala:59)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpRequestParser$$anon$1.parseMessage(HttpRequestParser.scala:94)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpMessageParser.startNewMessage(HttpMessageParser.scala:124)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpMessageParser.startNewMessage$(HttpMessageParser.scala:122)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpRequestParser$$anon$1.startNewMessage(HttpRequestParser.scala:59)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpMessageParser.$anonfun$state$1(HttpMessageParser.scala:49)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpMessageParser.run$1(HttpMessageParser.scala:82)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpMessageParser.parseBytes(HttpMessageParser.scala:96)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpMessageParser.parseBytes$(HttpMessageParser.scala:80)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpRequestParser$$anon$1.parseBytes(HttpRequestParser.scala:59)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpMessageParser.parseSessionBytes(HttpMessageParser.scala:78)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpMessageParser.parseSessionBytes$(HttpMessageParser.scala:73)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpRequestParser$$anon$1.parseSessionBytes(HttpRequestParser.scala:59)
[info] 	at org.apache.pekko.http.impl.engine.parsing.HttpRequestParser$$anon$1.onPush(HttpRequestParser.scala:71)
[info] 	at org.apache.pekko.stream.impl.fusing.GraphInterpreter.processPush(GraphInterpreter.scala:555)
[info] 	at org.apache.pekko.stream.impl.fusing.GraphInterpreter.execute(GraphInterpreter.scala:433)
[info] 	... 17 more

A simple but lazy fix would be to reject/ignore Accept-Charset headers like that if we cannot do anything with them.

A more involved and potentially performance-impacting fix would be to make sure to not assume that we have a nioCharset for every charset we receive and try to reject from those layers. Since this is used from many places, it is quite hard to fix properly ensuring that we can create proper responses. Maybe we should just ignore those headers if the charset cannot be resolved. (Which makes sense, because we will not be able to produce a response in the given charset anyway).

Seems low priority to me. Failing to process the request due to the bad charset value is not too bad. But ignoring the exception and proceeding as if no charset was provided looks like a good solution.

https://www.rfc-editor.org/rfc/rfc9110.html#field.accept-charset even mentions Accept-Charset was deprecated because of the near-ubiquity of UTF-8, and doesn't seem to put any requirements on the server to honour the charset (it merely 'indicates preferences') - so ignoring 'invalid' values sounds fine to me as well.