akka / akka-http

The Streaming-first HTTP server/module of Akka
https://doc.akka.io/libraries/akka-http/current/
Other
1.34k stars 594 forks source link

ResponseRendering.scala doesn't respect server settings when logging suppression warnings #2520

Closed dmarticus closed 4 years ago

dmarticus commented 5 years ago

As shown in the title, it looks like ResponseRendering.scala doesn't respect the server settings when calling the renderHeaders method (the suppressionWarning method doesn't accept the settings as an argument either, see https://github.com/akka/akka-http/blob/f57ec755a9107a8c1c25412ecde3149a91261e7c/akka-http2-support/src/main/scala/akka/http/impl/engine/http2/ResponseRendering.scala#L86)

This seems like a bug to me because it seems like it would be great to suppress those warnings by turning down the error verbosity in the akka config, but since this method never references the server settings, changing the error verbosity in the akka config has no effect on these warnings.

I think I know how to fix this, though, and I can submit a PR that addresses this change if we decide it's a worthy fix.

Thanks!

raboof commented 5 years ago

Which server setting did you expect to suppress these warnings?

You might be thinking of illegal-header-warnings, but in that case I think it is intentional: that controls warnings "in case an incoming message (request or response) contains an HTTP header which cannot be parsed" (https://doc.akka.io/docs/akka-http/current/configuration.html). The line you are quoting, however, deals with the outgoing response message at the server side.

(I did not look at your PR in an effort to respect your employers' copyright, I'd be happy to look at it when you have cleared it with them)

jrudolph commented 4 years ago

I'd say the warning is warranted most of the times. It is only given if there's an mistake in your server code in that you add headers that you shouldn't be adding to a response.