zio / zio-http

A next-generation Scala framework for building scalable, correct, and efficient HTTP clients and servers
https://zio.dev/zio-http
Apache License 2.0
787 stars 396 forks source link

Why is warning header added by default? #2701

Closed DLakomy closed 1 month ago

DLakomy commented 7 months ago

I've noticed that the apps using ZIO HTTP send a warning header on error, containing some information about the backend implementation details (it explicitly mentions ZIO HTTP).

ScalaCLI example:

//> using scala 3.3.1
//> using dep dev.zio::zio-http:3.0.0-RC4

import zio.*
import zio.http.*

object Application extends ZIOAppDefault:

  private val routes =
    Method.GET / "test" -> Handler.internalServerError

  override def run =
    Server
      .serve(
        // this will remove the "warning" header if the route above is hit
        // in case of "not found" the "warning" header is added anyway
        routes.toHttpApp @@ Middleware.removeHeader("warning")
      )
      .provide(
        Server.defaultWithPort(8080)
      )

Some responses:

$ curl -v localhost:8080/test # no "warning" header - good
…
< HTTP/1.1 500 Internal Server Error
< content-length: 0
…
$ curl -v localhost:8080/notexists # "warning" header is present, not what I want
…
< HTTP/1.1 404 Not Found
< warning: 404 ZIO HTTP /notexists
< content-length: 0
…

Exposing backend implementation details (as I've said, this header mentions ZIO HTTP) can be considered a security issue. Moreover, if I'm not mistaken, the warning header is deprecated or soon to be deprecated (https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-cache-06#section-5.5).

After reading the documentation and looking for similar issues on GitHub I still don't know how to get rid of these headers.

Is there a reason why this is the default behaviour, not opt-in? Is it possible to opt-out? I think such headers shouldn't be added by default.

I understand that I'm expressing a personal opinion, not a proper technical issue; however, I thought that it's worth raising anyway, as the answer may serve as a future reference, in case someone has a similar problem.

987Nabil commented 7 months ago

I had issues with this before. The impl. is also not that clean. I think the right way would be, to have opt-in for error messages in the response body.

jdegoes commented 4 months ago

/bounty $50 to make it opt-in.

algora-pbc[bot] commented 4 months ago

💎 $50 bounty • ZIO

Steps to solve:

  1. Start working: Comment /attempt #2701 with your implementation plan
  2. Submit work: Create a pull request including /claim #2701 in the PR body to claim the bounty
  3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

Thank you for contributing to zio/zio-http!

Add a bountyShare on socials

Attempt Started (GMT+0) Solution
🟢 @987Nabil #3050
ologbonowiwi commented 4 months ago

I removed the warning header on #2897, but I don't know if we need a flag or something to decide whether to include it.

If this is the case, where should such a flag be? This is my first contribution to the project, so I still need to familiarize myself with the logic and where this stuff is. 😅

Also, if we choose to make it possible to keep the warning header (via flag), I'll add a condition using the flag + remove "ZIO HTTP" from this line https://github.com/zio/zio-http/blob/f070e41a348ab24d4ba2720da7d60b6972d2f902/zio-http/shared/src/main/scala/zio/http/Response.scala#L141

Exposing backend implementation details (as I've said, this header mentions ZIO HTTP) can be considered a security issue.

This will solve this part of the issue.

algora-pbc[bot] commented 1 month ago

🎉🎈 @987Nabil has been awarded $50! 🎈🎊