patchlwIP - A Lightweight TCP/IP stack - Patches: patch #9500, Avoid sending Content-Length...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #9500: Avoid sending Content-Length header if data is not ready yet

Submitted by:  Giuseppe Modugno <giusloq>
Submitted on:  Mon 27 Nov 2017 09:02:15 AM UTC  
 
Category:  apps Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  Simon Goldschmidt <goldsimon> Open/Closed:  Closed
Planned Release:  2.1.0

( Jump to the original submission)

Wed 07 Feb 2018 08:29:41 PM UTC, comment #9: 

Giuseppe, thanks for insisting. Your requested functionality was only a little refactoring away. Now my example works: content length (and with it persistent connection setting) is generated when sending the header (when fs_canread_custom() returned 1).

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Mon 05 Feb 2018 07:17:04 PM UTC, comment #8: 

I have understood that by now.

> My suggestion is to postpone the calculation of Content Length
> header when httpd really needs it, so when the header is really
> sent, that could be at a later time.


Implementing this would need a greater rework already: the header is sent directly, even if using delayed read. So currently, content length is "calculated" when it is needed.

That's why I proposed to implement delayed open. You could then even delay searching for a file name. And of course in your case, you could delay open until the file size is known.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Mon 05 Feb 2018 12:32:15 PM UTC, comment #7: 

> What's unclear to me is: why do you know a file exists (you return OK from fs_open_custom), but don't know its size? Maybe
> we would need some kind of delayed open mechanism instead...


First of all, I'm talking about file with dynamic content, such as JSON data.  Suppose the server receives a HTTP request from a client.  The request is about data that the server doesn't know immediately.  It needs to retrive the answer from another device, maybe connected through a RS485 bus.

We can't block fs_open() function until all data are received from the "slow" serial bus.  So we immediately open the file, but without knowing the real size.

Clearing persistent flag works, with the drawback to lost the persistent connection, that is always a Good Thing.

Another possibility is to delay the content length calculation.  Actually the Content Length header is generated and calculated in get_httpd_headers() function (at the moment of fs_open), but it will be really sent during http_send_headers().  If the file is not immediately ready (as in my example), http_send_headers() will be delayed.

My suggestion is to postpone the calculation of Content Length header when httpd really needs it, so when the header is really sent, that could be at a later time.

Giuseppe Modugno <giusloq>
Sat 03 Feb 2018 09:03:44 PM UTC, comment #6: 

There wer more bugs than this one. Should be fixed now, see 'examples/httpd/fs_example' in contrib.

As a result, custom files need the FS_FILE_FLAGS_HEADER_PERSISTENT flag for their http connection to be persistent. So if you don't know the file size on open, set it to 0 and don't set the persistent flag.

What's unclear to me is: why do you know a file exists (you return OK from fs_open_custom), but don't know its size? Maybe we would need some kind of delayed open mechanism instead...

Anyway, it should work now with the current code.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 02 Feb 2018 08:47:08 AM UTC, comment #5: 

Forget comment #2, I see what you were talking about.

I don't think having file->data == NULL will work, though. At least not for SSI. I'd rather only check file->flags for FS_FILE_FLAGS_HEADER_INCLUDED.

SSI needs rework for the changed httpd_cgi_handler() callback, too.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 01 Feb 2018 06:53:03 PM UTC, comment #4: 

Giuseppe, anything you can say to answer my questions?

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 16 Jan 2018 09:02:25 PM UTC, comment #3: 

In other words, could you try to add a better description of what needs to be changed and why? For my file system abstraction layer, this works. I need to understand why it seems to not work for you.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Sun 17 Dec 2017 09:20:06 PM UTC, comment #2: 

> If keep-alive support is activated, when all the data is send,
> the connection stays open and the client can't detect the end
> of file


What I read here is "please don't push this patch, it breaks keep-alive support". Maybe you should have filed a bug report, not a patch? Or would you mind providing a working patch?

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 28 Nov 2017 01:53:50 PM UTC, comment #1: 

This patch works if keep-alive is not enabled.

If keep-alive support is activated, when all the data is send, the connection stays open and the client can't detect the end of file (in case Content-Length isn't send).

Giuseppe Modugno <giusloq>
Mon 27 Nov 2017 09:02:15 AM UTC, original submission:  

diff --git a/src/apps/httpd/httpd.c b/src/apps/httpd/httpd.c
index 0ac8d49..112c7a8 100644
--- a/src/apps/httpd/httpd.c
+++ b/src/apps/httpd/httpd.c
@@ -936,7 +936,7 @@ get_http_headers(struct http_state hs, const char uri)
     add_content_len = 0; /* @todo: get maximum file length from SSI */
   } else
 #endif /* LWIP_HTTPD_SSI */
-    if ((hs->handle == NULL) ||
+    if ((hs->handle == NULL) || (hs->handle->data == NULL) ||
         ((hs->handle->flags & (FS_FILE_FLAGS_HEADER_INCLUDED | FS_FILE_FLAGS_HEADER_PERSISTENT)) == FS_FILE_FLAGS_HEADER_INCLUDED)) {
       add_content_len = 0;
     }

Giuseppe Modugno <giusloq>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by giusloq (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-02-03 goldsimon StatusNone => Done
        Assigned toNone => goldsimon
        Open/ClosedOpen => Closed
    2018-01-02 goldsimon Planned ReleaseNone => 2.1.0

    Back to the top


    Powered by Savane 3.5