search

apache / pekko-http

The Streaming-first HTTP server/module of Apache Pekko
https://pekko.apache.org/
Apache License 2.0
149 stars 36 forks source link

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

Open He-Pin opened 1 year ago

He-Pin commented 1 year ago

Picked from: https://discuss.lightbend.com/t/internal-server-error-triggered-by-accept-charset-header-with-unexpected-value/10427

Which expects the header to be ignored.

pjfanning commented 1 year ago

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.

mdedetrich commented 1 year ago

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

jrudolph commented 1 year ago

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
jrudolph commented 1 year ago

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).

pjfanning commented 1 year ago

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.

raboof commented 3 weeks ago

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.