diff mbox

[OpenWrt-Devel] uhttpd: chunked output from cgi script

Message ID 56152229.7010009@gmail.com
State Rejected
Headers show

Commit Message

Luigi Tarenga Oct. 7, 2015, 1:46 p.m. UTC
Hello list,
I'm working on a CGI written in lua and I'm using uhttpd as webserver on 
a tp-link AP.
I'm using external CGI interpreter, not embedded lua.

I need that the output from my CGI is chunked since I would like to 
avoid to buffer the
whole response and calculate the "Content-Length".

I discovered that uhttpd try to send chunked response if the response is 
a normal 200 OK
and the method is a GET. This is my case but unfortunately I ended with 
a strange behavior
where sometimes the GET return with chunked content and sometimes not.

I have an html page that call the CGI with a jquery method:
     <script>
       $(document).ready(function(){
          $("#catdiv").load("../cgi-bin/board.lua?noheader=1");
       });
     </script>


the board.lua script return  a piece of html code (without headers and 
footers).
The output of the CGI is always the same between test.

Debugging the http traffic with firefox I see that the GET of board.lua 
take about 33ms
when the output is chunked and 20s (the full keep-alive timeout) when 
the output is not
chunked. I see the response headers and it contain Transfer-Encoding: 
"chunked" directive
in one case on not in the other. From client side everything seems 
correctly following what
the websever gives back.

If I directly call cgi-bin/board.lua it seems to always return in 33ms. 
Debugging the code I came up
with the idea that where there are multiple GET on one http session if 
one GET is solved with
a 204 or 304 that all other GET can't be chunked anymore. In fact my 
html page load other pages
(css, js, etc about 5 GET in one page refresh).

I tried with a simple patch (I'm sure this is not the best solution 
though) that seems to correct
my problem.



I would like to know if this have sense, if it has been corrected in 
other uhttpd version or if my patch
broke with some RFC recommendation.

my openwrt version is:
DISTRIB_ID='OpenWrt'
DISTRIB_RELEASE='Chaos Calmer'
DISTRIB_REVISION='r46910'
DISTRIB_CODENAME='chaos_calmer'
DISTRIB_TARGET='ar71xx/generic'
DISTRIB_DESCRIPTION='OpenWrt Chaos Calmer 15.05'
DISTRIB_TAINTS='no-all no-ipv6'

thank you in advance
Luigi

Comments

Jo-Philipp Wich Oct. 7, 2015, 10:21 p.m. UTC | #1
Hi Luigi,

thank you for your patch proposal, it prompted me to take a deeper look
into the problem.

The issue should be fixed with the following commit:

http://nbd.name/gitweb.cgi?p=uhttpd2.git;a=commitdiff;h=7ed2edc40dd6d0171266f3bfbc96466e1d25e3cd


Regards,
Jow
Luigi Tarenga Oct. 8, 2015, 10:47 a.m. UTC | #2
thank you Jow for the promptly feedback.
I tried to better understand the problem and now I'm reading at your commit.
My patch works but is logically broken. I'm trying to produce a better one.

If I'm not wrong the problem is this (without applying your commit):
the client request handling code go though 3 steps (callback):
CLIENT_STATE_INIT -> client_init_cb ()
CLIENT_STATE_HEADER -> client_header_cb ()
CLIENT_STATE_DATA -> client_data_cb ()

actually in the INIT portion we test for the http_code 204 or 304 but 
this is wrong,
we still don't know if the client cached file is updated or not and I 
think the chunking
code apply only to data, not headers. a 204 message have no data.
this test should be done in the HEADER phase (uf_file_data call -> 
uf_file_if_unmodified_since)

If for testing purpose we completely skip this test I think it's safe 
because in the
HEADER phase (file.c line 617) if we are going to handle a file request 
we disable
chunked writes. If we are going to handle a script this does not happens.

to this add the fact that when we are going to execute a script after a 
file has been
handled there is no place where cl->http_code get resetted. this trigger 
the bug.
(for scripts only req->redirect_status = 200 is executed, file.c line 827)

I'm going to try a new simple patch and then your suggested one.
I will update you.

regards
Luigi


On 10/08/2015 12:21 AM, Jo-Philipp Wich wrote:
> Hi Luigi,
>
> thank you for your patch proposal, it prompted me to take a deeper look
> into the problem.
>
> The issue should be fixed with the following commit:
>
> http://nbd.name/gitweb.cgi?p=uhttpd2.git;a=commitdiff;h=7ed2edc40dd6d0171266f3bfbc96466e1d25e3cd
>
>
> Regards,
> Jow
>
>
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
diff mbox

Patch

--- file.c    2015-10-07 15:39:45.169056639 +0200
+++ file.c    2015-10-07 15:39:55.859056443 +0200
@@ -807,9 +807,10 @@ 
          return true;

      d = dispatch_find(url, pi);
-    if (d)
+    if (d) {
+        cl->request.respond_chunked = true;
          uh_invoke_handler(cl, d, url, pi);
-    else
+    } else
          uh_file_request(cl, url, pi, tb);

      return true;