well-typed / grapesy

Native Haskell gRPC client and server based on `http2`
Other
32 stars 4 forks source link

Relax server request checking #48

Closed mstksg closed 5 months ago

mstksg commented 5 months ago

Feel free to not use this as-is, but just putting this here for reference. We are implementing a server that has to respond to some non-conforming clients, and these changes remove some request preconditions.

Here is why I feel these changes are justified:

  1. content-type: application/grpc+....: while it should always be the serialization format, this is actually technically ignored, because the serialization format is determined by the path and not by the content-type header. With that in mind, the GRPC spec does enforce either application/grpc or application/grpc+...., so it feels okay to still reject wildly incorrect content-types. The non-conforming clients at least send application/grpc+something, so this change is just enough to let them work.
  2. Not validating ascii header values: Some of the non-conforming clients are sending blank header values. This one is explicitly forbidden by http2 spec. Grapesy does an explicit check before wrapping the values in UnsafeAsciiValue. This change to remove the explicit check is a bit iffy, but I could argue that it could be justified because this is a part of the http2 standard, not the grpc standard. As such, http2 should only be giving end-users (like grapesy) valid http2 header values, so we can assume that they are valid by the time they get to grapesy. Any invalid headers that get through could be considered undefined behavior.

    Alternatively we could remove the check for non-empty value only in the case of server request validation, essentially making 0-length AsciiValues be undefined behavior, but non-ascii AsciiValues still be rejected. This would allow our non-conforming clients to work, although somehow feels even more hacky.

edsko commented 5 months ago

Superseded by #51 , which relaxes the checks also but is not quite as permissive as this PR is: