Open ocramz opened 6 months ago
Hi, if I understand correctly addressing this would require these steps:
ScottyException
, e.g. InvalidHeader Text Text Text
(header, value, error message). Perhaps it could be refined into something like InvalidHeaderName Text
and InvalidHeaderValue Text
or create a sum type etc.Options
to allow users to specify whether headers should be validated, e.g. validateHeaders
. It needs to be decided whether validation is on by default (breaking change) or not.changeHeader
such that when validateHeaders
is set, the name and value of the header are validated.\NUL
, \n
, \r
It would also be possible to let the user decide what characters to accept, e.g. by adding a validHeaderChar :: Char -> Bool
predicate to Options
instead.
Before writing the necessary code I'd like to hear your opinions :)
Instead of my original suggestion, we could also implement the whole validation logic as a WAI middleware
So basically we replace the
Modify changeHeader such that when validateHeaders is set, the name and value of the header are validated.
by
Add a middleware that validates name and value of each response header when
validateHeaders
is set
? I can make a PR for that.
@fumieval you would prefer a WAI middleware I believe? I can implement this but I would prefer to have your go-ahead
Personally I think putting this in changeHeader
is more transparent API behavior but that's a nitpick
@pbrinkmeier I believe the middleware approach makes implementation much easier. I don't have a strong opinion about the user-facing API, but how about adding validateHeaders :: Bool
field to Options
, and make it insert the middleware if it's True?
@fumieval but then, why use a flag at all when the user can just insert a middleware?
@ocramz I'd personally prefer letting users insert the middleware, but I thought it might be an option if we care about ease of use.
Okay, so should we add the middleware to Web.Scotty
? Or a seperate module perhaps? I will get working on the implementation and we can move it around before merging.
+1 to exporting the middleware from Web.Scotty (IMO the right place for this middleware is https://hackage.haskell.org/package/wai-extra, but it shouldn't stop going forward in scotty)
While implementing this I noticed that scottyExceptionHandler
doesn't handle exceptions thrown in middlewares, so I guess we could simply return a 500 with an appropriate error text in the middlware?
Edit: At that point I could also move the PR to wai-extra
as it becomes completely decoupled from scotty
.
I just added a PR at wai-extra
to address this issue: https://github.com/yesodweb/wai/pull/990
The PR in wai-extra
is merged by now, so we could add a comment to the docs referencing that middleware.
Originally posted by @fumieval in https://github.com/scotty-web/scotty/issues/359#issuecomment-1859634869
Reference: #92