Hi,
how about attached patch? it should work ok but untested.
Thanks,
Alex
Dziugas Baltrunas wrote:
> 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 -a -u -p -r1.240 http.c
--- gwlib/http.c 5 Mar 2006 14:37:26 -0000 1.240
+++ gwlib/http.c 23 Mar 2006 18:52:14 -0000
@@ -2097,8 +2097,18 @@ static void receive_request(Connection *
ret = parse_request_line(&client->method, &client->url,
&client->use_version_1_0, line);
octstr_destroy(line);
- if (ret == -1)
- goto error;
+ /* client sent bad request? */
+ if (ret == -1) {
+ /*
+ * mark client as not persistent in order to destroy connection
+ * afterwards
+ */
+ client->persistent_conn = 0;
+ /* unregister connection, http_send_reply handle this */
+ conn_unregister(conn);
+ http_send_reply(client, HTTP_BAD_REQUEST, NULL, NULL);
+ return;
+ }
/*
* RFC2616 (4.3) says we should read a message body if there
* is one, even on GET requests.
