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

Submitter:  Giuseppe Modugno <giusloq>
Submitted:  Mon 27 Nov 2017 09:02:15 AM UTC
   
 
Category:  apps Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  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>
Group administrator
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>
Group administrator
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>
Group administrator
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>
Group administrator
Thu 01 Feb 2018 06:53:03 PM UTC, comment #4: 

Giuseppe, anything you can say to answer my questions?

Simon Goldschmidt <goldsimon>
Group administrator
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>
Group administrator
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>
Group administrator
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>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

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)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    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.13-f8d8.
    Corresponding source code