HTTP/2 error when parsing percent-encoded query string about pekko-http HOT 12 OPEN

@pjfanning I know, I just wanted to let them know as well.

from pekko-http.

Ill see if I have time for this, the fix doesn't seem that hard (just need to add an extra case where the exceptions are caught).

I'll have a look quickly, also where to put the tests, and let you know.

from pekko-http.

thanks @mkurz for the detailed description of the issue

It is probably still useful for you log this in akka-http project too but it's good not to assume that we will be following issues raised in akka projects (especially, since we can't copy any of the solutions)

from pekko-http.

@jrudolph @mdedetrich is this something that we would like to fix before a pekko-http 1.0.0 release?

from pekko-http.

Ill try and look into this tomorrow, doesn't seem to hard to solve since @mkurz did an excellent job in specifying how to solve the bug

from pekko-http.

Alternately @mkurz would you want to contribute the fix, it seems like you are ontop of things here?

from pekko-http.

In any case, it's not a blocker. It's somewhat unfortunate that some error cases fail more extremely than desired but it does not change the outcome a lot. Btw. the nice error message is also disabled by default for HTTP/1.1 so it's mostly about the status code.

from pekko-http.

I am sorry, I have so much other work to do I don't have the capacity right now to work on this issue here.

from pekko-http.

I am sorry, I have so much other work to do I don't have the capacity right now to work on this issue here.

No worries

In any case, it's not a blocker. It's somewhat unfortunate that some error cases fail more extremely than desired but it does not change the outcome a lot. Btw. the nice error message is also disabled by default for HTTP/1.1 so it's mostly about the status code.

Ill see if I have time for this, the fix doesn't seem that hard (just need to add an extra case where the exceptions are caught).

from pekko-http.

added 1.0.0 milestone marker but it's a nice to have as opposed to a release blocker

from pekko-http.

There are several complications here:

• our HTTP/2 client does not allow send invalid URIs (Raw-Request-URI not yet supported)
• there's currently no way to signal non-fatal parsing errors (should we treat it non-fatal, though?) to the demuxer, so
  in any case the connection will be torn down after sending the response, together with all other ongoing streams. This
  seems to make it more reasonable to handle these HTTP-layer errors (as opposed to HTTP/2-framing errors) with HTTP
  error codes which however means that we need to find ways to signal those errors through the layers (i.e. introduce
  another synthetic frame type).

So, nothing to fix quickly, but I'll have a look into it when I find some time.

from pekko-http.

there's currently no way to signal non-fatal parsing errors (should we treat it non-fatal, though?) to the demuxer, so
in any case the connection will be torn down after sending the response, together with all other ongoing streams. This
seems to make it more reasonable to handle these HTTP-layer errors (as opposed to HTTP/2-framing errors) with HTTP
error codes which however means that we need to find ways to signal those errors through the layers (i.e. introduce
another synthetic frame type).

So I don't know HTTP2 implementation very in depth, but this to me appears like a non-fatal/business logic error (i.e. it happens when the client sends a malformed URI so its in the same class of errors like validation). If as you say there is no way currently for the HTTP2 to gracefully handle business/validation errors, we should probably fix that first before approaching this.

@pjfanning Considering whats just been said I think its safer to park this for now and remove it from the 1.0.0 milestone

from pekko-http.