Allow for custom management of "Invalid request URI"

[ThomasTJdev]

Case

With Mummy 0.4.x, the commit below implements a connection drop on an invalid request URI due to the new parsing of URI elements.

Problem

Running Mummy behind nginx and making a GET request for localhost:8080/badparam?test=% will drop the connection within Mummy with the log-message "Dropped connection, invalid request URI," causing nginx to send a 502 result. In Mummy versions prior to 0.4.x, this type of error was ignored, and the route was served, allowing for a custom-formatted page and logging the bad URI in the backend.

I want control over my webserver and incoming requests. I like that Mummy parses the parameters for me, but I don't want it to automatically decide whether to drop the connection without giving me the option to provide custom management.

Question

Can we make Mummy less restrictive in this approach? Either by passing the data to the route without the parameters or, at least, using the errorHandler in some way?

Blame

https://github.com/guzba/mummy/commit/b05657228add4b152dc9d2fa8dd8847cab781ed2#diff-1e13710d6692a0c54d414e3ba123be6e4b788ce3d95abc4f9d4c9db4cec43c94R823-R829

[guzba]

Are you receiving valid requests with invalid URIs?

I understand the sentiment of wanting to receive all requests, however there are many cases where invalid requests are dropped since there is no way to make sense of them. Previously mummy did not decode the URI before doing routing, which would break for things that included spaces in routes for example, eg "/my path/" would not match "my%20path".

In this case, an invalid URI cannot be properly routed. Calling the errorHandler is reasonable, but I am missing some understanding of a real concrete scenario to base this around. It could appear to be a bunch of plumbing and statefulness for a request that no client ever sees?

For example, a request with an invalid chunked encoding is dropped. There is no way to make sense of the body. Passing that invalid encoded body to a handler and expecting it to figure it out seems very unexpected. Each handler would need some sort of "is the body actually valid" check? Is the invalid body stored somewhere other than the valid body? Where is the valid-ness made available? etc. Same goes for .path and URI.

Having a good solid real-world situation will I think clarify what is best to do here -- eg is errorHandler actually good or is maybe path: Option[string] better than path: string so you can know if it isn't present it was not valid?

[guzba]

Relevant: https://github.com/golang/go/issues/21548

[ThomasTJdev]

Thanks for the feedback and the Go-link, Ryan!

Maybe it's just a me-use-case :) ? I've been accustomed to receiving the requests and managing them all, so suddenly not having that possibility was the origin of the issue.

I don't have a use case for using the test=% in the example above; it's more about the backend management:

1) Log management and incident tracking: Receiving a 502 error in the Nginx logs raises a flag when it can't be matched with the internal logging system. Is the application down, or was it a bad request? Having authenticated the user and parsing the url /api/v1/user-finance-options?iban=xx&swift=% should raise a red-flag and provide tracking info (instantiationInfo()) and not just a 502.

2) Identifying bad inputs: Of course, previous fuzzing and testing should have taken care of user inputs, buuuut if something was missed, allowing the request to pass through would enable me to determine whether it was actually a bad request. If there are multiple parameters, some good and some bad, then I would prefer to handle the good ones and just log the bad ones.

And I think you're right, the errorHandler would not be the right place; the Option or a new errorURI/RouteHandler would be better; personally I would prefer a handler. Unless it's just a me-problem ;)