[Seaside] bad request

[Otto Behrens]

>> This is my best conclusion - please comment:
>> For someone providing a web application, the server implementer can either
>> 1) be strict respond with 400 (bad request) if it finds something it
>> dislikes in the request (either the http method + URL, headers or
>> body),
>> or 2) ignore inappropriate parts of the request. Of course, some parts
>> of the request can't be ignored, so one will have to distinguish
>> carefully.
>
> I generally prefer option 1) because this way when something goes
> wrong on the server the client gets notified. Otherwise client errors
> can be really annoying to debug.

Yes, agree. I could not find "invalid cookie" kind of responses when
searching for how others solve this kind of problem though. Having
said this, I'd also rather respond with 'bad request' if I find
anything I don't like in the request.

I suppose we also have to distinguish between unable to parse the
cookie header and an unexpected key or value in the cookie header. If
the server receives a key that it does not recognise, should it
respond with bad request?

> process: aNativeRequest
>     ^ [ self basicProcess ]
>         on: self badRequestExceptionSet
>         do: [ :e | self badRequestFor: e ]
>
> badRequestExceptionSet
>   ^ GRInvalidUtf8Error, WAInvalidUrlSyntaxError
>
> badRequestFor: anError
>   "default implementation, trigger the servers internal error handler
>   sublcasses can answer a 'native' bad request (HTTP 400) respone"
>   anError signal

This means that every subclass of WAServerAdaptor have to implement a
native bad request response. Is there an elegant (i.e. non monkey
patched) way for me to customize this response? That's why I thought
the seaside framework must handle it, because I can customise there.

>> It seems if I want to handle any of these errors (consistently), I
>> have to modify Comanche, Zinc and FastCGI adaptors.
>
> Yes

Ok, would be soo nice if I can do it once in my application though
(because I do it there already).

>> I think we should allow the Seaside framework to handle these errors
>> as well. I understand giving the WAResponse the responsibility to
>> handle it can be tricky, but it may just be the right place.
>> Currently, it does handle some of the 400 range of errors (eg. not
>> found).
>
> WAResponse does not handle 400. Somebody somewhere creates a 400
> response just like somebody somewhere creates a 200 response.

Sorry, the request handler may create a WAResponse with code 400.

>> This is my suggestion: Create the WARequestContext on the native
>> request without parsing it.
>
> I'm not liking it. The propose of WARequestContext is that it holds on
> to (among other things) a WARequest and a WAResponse. We just got an
> exeption when trying to create a WARequest. This smells like having a
> partially initialized object that will just cause other exceptions
> further down the road.

Yes, I share your discomfort. But, the same problem applies to the
response. When creating the context, the response is empty. Exceptions
raised during the construction of the response may leave it in a bad
state if it is not properly handled, not true?

It would be better then to parse it in one shot then. And set the
WARequest to be bad could be another option. The request handler can
then take care of a bad request.

>> Parsing the URL effectively comes down to handleFiltered: on
>> WADispatcher that will try to get the url via the consumer on the
>> request. If parsing it breaks, the request is marked as a bad request.
>> The dispatcher could pass to the default handler. The parsing errors
>> of the other parts of the request would then go to the handler
>> dispatched to. And that could in turn be dispatched to an error
>> handler.
>
> No, the URL is parsed in WAServerAdaptor >> #requestUrlFor:

I meant that if we delay the parsing, #requestUrlFor: would be sent at
a later stage.