[Seaside] bad request

[Philippe Marschall]

On Mon, Dec 23, 2013 at 4:25 PM, Otto Behrens <otto at finworks.biz> wrote:
> Hi,
>
> Vulnerability tests that ran against our site showed that URL's with
> percentage encoded UTF8 characters creates a 500 (internal server
> error) response, where I think they should actually respond with 404
> or 405.

I think in this very specific case 400 (bad request would be the way to go).

> Some examples:
>
> GET /% breaks when WAUrl class #decodePercent: tries to read off the
> end of the stream.
> GET /%C0 breaks for the same reason, but in GRPharoUtf8CodecStream |
> next: (more cases in there!)
> GET /%C0%10 raises GRInvalidUtf8Error.
> GET /%C0%AE returns 404 (not found).
>
> I'd like to improve on this, if you agree.
>
> I need some guidance on how to improve regarding the construction of
> the request. This is the code in WAServerAdaptor | contextFor:
>
> It creates a request (self requestFor: aNativeRequest) without
> handling exceptions. This means that the outer exception handler
> catches it and returns a 500.

So right now FastCGI already turns it into a 500?

> I am not sure where to handle errors such as these and to elegantly
> ensure that the response is "bad request" if the request could not be
> parsed. I also see that there's a nice badRequest method on
> WAResponse, without any senders in my image. Perhaps this is some
> perform: magic that I can't trace.

That's simply a convenience method for setting the status code.

> Any help will be appreciated.

There could be all sorts of things that could go wrong causing an
exception. The http version for example could be "A.B", cookie parsing
could fail and URL decoding could fail [1].
Should the response generated in the exception case be a WAResponse or
a "native" response? WAResponse response may be a bit tricky because
you don't have WARequestContext because you don't have WARequest. So
it's probably going to be "native" response, this also avoids the
other issue of what you're going to do if response conversion
(#responseFrom:) throws some kind of exception.
So in the end all what you do is use a 400 instead of a 500 for some
exception classes.

 [1] http://code.google.com/p/seaside/issues/detail?id=764

Cheers
Philippe