500 crash when response contains a header which starts with ":"

[visox]

An endpoint like this

      case _ @Method.GET -> _ / "failing" =>
        ZIO.succeed(
          Response(
            headers = Headers(Header.Custom(":status", "200"))
          )
        )

should not fail when called or am i missing something ?

It crashes with 500, the error is only visible in client (so curl or postman). I cant handle/catch the error in code as there is nothing to catch.

I found this problem when i mapped all headers from some other service to the client (my service being the middleman)

Also it seems it does not matter if its :status or :xx

zioV = "2.0.+"
zioHttpV = "3.0.0-RC2"

[jdegoes]

/bounty $50

[algora-pbc[bot]]

💎 $50 bounty • ZIO

Steps to solve:

1. Start working: Comment /attempt #2227 with your implementation plan
2. Submit work: Create a pull request including /claim #2227 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 bounty • Share on socials

Attempt	Started (GMT+0)	Solution
🟡 @afrmgv	Jun 30, 2023, 6:42:54 PM	WIP

[vigoo]

I'm not sure we should support header names starting with :, according to https://www.rfc-editor.org/rfc/rfc9110.html#name-tokens that's an invalid header name (header name is a token)

The exception should be catchable with http.catchAllCauseZIO

[Jesse-Bakker]

Headers starting with : are indeed invalid, but HTTP/2 has pseudo-header fields which start with : and replace the message start line from HTTP/1, see HTTP/2 pseudo-header fields.

[abcpro1]

It doesn't look like http2 support has landed in main yet. But even then, the correct way to set the response status would be like:

Response(
  status = Status.Ok,
)

[afrmgv]

/attempt #2227

The error produced by Netty in the DefaultHttpHeaders.

int index = HttpHeaderValidationUtil.validateToken(name);
if (index != -1) {
    throw new IllegalArgumentException("a header name can only contain \"token\" characters, " +
                        "but found invalid character 0x" + Integer.toHexString(name.charAt(index)) +
                        " at index " + index + " of header '" + name + "'.");
}

And the : character explicitly not allowed in the header name in the HttpHeaderValidationUtil.

 BitSet128 tokenChars = new BitSet128()
                .range('0', '9').range('a', 'z').range('A', 'Z') // Alphanumeric.
                .bits('-', '.', '_', '~') // Unreserved characters.
                .bits('!', '#', '$', '%', '&', '\'', '*', '+', '^', '`', '|'); // Token special characters.

If we check the codec for httpv2, we can see that it contains pseudo-header support.

if (hasPseudoHeaderFormat(name)) {
     final Http2Headers.PseudoHeaderName pseudoHeader = getPseudoHeader(name);
     ....

That why I think that support of : in headers is not a bug because zio-http currently does not support http2 netty codec.

But about error handling, the current implementation of ServerInboundHandler in non-fatal error trying to use withDefaultErrorResponse to generate default response, that's why we can't catch this error by catchAllCauseZIO. Do we need to push this error up?

    response.body match {
      case body: Body.UnsafeBytes =>
        try {
          fastEncode(response, body.unsafeAsArray)
        } catch {
          case NonFatal(e) => fastEncode(withDefaultErrorResponse(null, Some(e)).freeze, Array.emptyByteArray)
        }
      case _                      => false
    }

[algora-pbc[bot]]

@afrmgv: Reminder that in 7 days the bounty will become up for grabs, so please submit a pull request before then 🙏