Re: [PATCH]: Sending HTTP_BAD_REQUEST to client in case of bad request

Hi,

ok, what about putting simple return statement instead of goto error?

In case of queued for sending, http_send_reply() will register a
callback to receive_request() with a state sending_reply which will
close the connection afterwards. So it means that http_send_reply()
will destroy the connection anyway.

However, above will not work, i.e. client will get a 400 reply but the
connection won't be closed because we have p->persistent_conn = 1 as
default in client_create() which I think has to be changed into 0. Who
said that HTTP 1.1 connection should default to keep-alive anyway?

Btw, in order for others to understand what this is all about, given
that localhost:13013 is your smsbox up and running, try the following:

> telnet localhost 13013
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
asdf
Connection closed by foreign host.

Which means that it violates http, gives no indication to client, etc.
etc., so this should definitely be changed. Proposed way is the
following:

> telnet localhost 13013
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
asdf
HTTP/1.1 400 Bad Request
Server: Kannel/cvs-20060308
Content-Length: 0

Connection closed by foreign host.

On 3/23/06, Alexander Malysh <[email protected]> wrote:
> Hi,
>
> no go with your patch. http_send_reply is a queuing function. that means
> when response could not be sent at once (e.g. client too slow) response
> sending will be registered (queued) in fdset. but in your patch you will
> destroy httpclient with goto error statement.
>
> You should split http_send_reply function in a helper function which
> just prepare response and main function that send response queued as
> earlier. Then here instead of sending through http_send_reply you do
> something like:
>
> Octstr *resp = http_prepare_reply(...)
> conn_send(resp);
> goto error;
>
> Thanks,
> Alex
>
> Dziugas Baltrunas schrieb:
> > Hi list,
> >
> > in case of client sends us malformed URL (such as
> > /cgi-bin/sendsms?user=test&pass=test&to=12345 &text=test), attached
> > patch sends a HTTP 400 Bad request error instead of simply closing the
> > socket thus giving no indication to the client.
> >
> > --
> > Dziugas
> >
> >
> > ------------------------------------------------------------------------
> >
> > Index: gwlib/http.c
> > ===================================================================
> > RCS file: /home/cvs/gateway/gwlib/http.c,v
> > retrieving revision 1.240
> > diff -u -r1.240 http.c
> > --- gwlib/http.c      5 Mar 2006 14:37:26 -0000       1.240
> > +++ gwlib/http.c      22 Mar 2006 11:03:25 -0000
> > @@ -2097,8 +2097,10 @@
> >           ret = parse_request_line(&client->method, &client->url,
> >                                       &client->use_version_1_0, line);
> >           octstr_destroy(line);
> > -         if (ret == -1)
> > -             goto error;
> > +         if (ret == -1) {
> > +             http_send_reply(client, HTTP_BAD_REQUEST, NULL, NULL);
> > +             goto error;
> > +         }
> >           /*
> >            * RFC2616 (4.3) says we should read a message body if there
> >            * is one, even on GET requests.
>
>
>


--
Dziugas