Re: [libmicrohttpd] size MHD_SIZE_UNKNOWN should never be returned in header

[Brecht Sanders]

Excellent!
Apparently I had that still in there from the 0.3.x days.
However, I want to keep compatibility, so I will use #if MHD_VERSION < 
0x00040000 in my program around MHD_create_response_from_callback.

By the way, your includes break Windows unless you do it like this:

#include <stdarg.h>
#ifdef __WIN32__
 #include <ws2tcpip.h>
#else
 #include <sys/select.h>
 #include <sys/socket.h>
#endif

Why don't you include those in microhttpd.h so no other headers are 
required to be pre-included?
Maybe #ifdef __WIN32__ should be something more general to also include 
Windows 64-bit versions...
Regards
   Brecht

Christian Grothoff wrote:
> Hi!
>
> Just looking at your testcase, you are using "(size_t)-1" instead of 
> MHD_SIZE_UNKNOWN right when you call MHD_create_response_from_callback.  That 
> is known to not work.  If I change this to MHD_SIZE_UNKNOWN and compile it 
> against MHD 0.4.6, I get the correct result:
>
> HTTP/1.1 200 OK
> Transfer-Encoding: chunked
> Content-Type: text/plain
> Date: Thu, 01 Apr 2010 21:37:35 GMT
>
> Hello world!
>
> I've attached the updated test-code (I also had to add a few #includes to get 
> it to compile using
>
> $ gcc -g -o test test_libmicrohttpd.c -lmicrohttpd -lcurl
>
> Note that MHD_SIZE_UNKNOWN is new since MHD 0.4.1, so using earlier versions 
> won't even compile.  If your version of Debian still has 0.3.x, I can only 
> suggest to update (especially given that you need MHD_SIZE_UNKNOWN).
>
> Best,
>
> Christian
>
> On Thursday 01 April 2010 01:52:55 pm Brecht Sanders wrote:
>   
> > Hello Christian,
> > Thanks for the quick response.
> > I looked in my code to make sure I wasn't missing anything.
> > Can you tell me if the MHD_ContentReaderCallback function needs to
> > return 0 or -1 on end of data?
> >
> > Then I wrote the attached test program in order to have a testcase.
> > On Windows (32-bit, compiled with MinGW) it returns:
> >
> >     HTTP/1.1 200 OK
> >     Content-Length: 4294967295
> >     Content-Type: text/plain
> >     Date: Thu, 01 Apr 2010 11:42:52 GMT
> >
> >     Hello world!
> >
> > Then I ran it on 32-bit Linux (Debian Lenny 5.0.3) and I got:
> >
> >     HTTP/1.1 200 OK
> >     Transfer-Encoding: chunked
> >     Content-Type: text/plain
> >     Date: Thu, 01 Apr 2010 11:48:41 GMT
> >
> >     Hello world!
> >
> > So there is definitely a difference: not only Content-Length is returned
> > but also Transfer-Encoding: chunked is missing.
> > Or is this due to Debian being on an older version of libmicrohttpd
> > (0.3.1-1)?
> >
> > I hope this helps in finding the source of the problem.
> > Regards
> >     Brecht
> >
> > Christian Grothoff wrote:
> >     
> > > Hi!
> > >
> > > I've looked all over the code and I cannot find any size_t/uint64_t
> > > confusion that would explain this.  If you were using MHD_SIZE_UNKNOWN in
> > > conjunction with "MHD_create_response_from_data", bad things would happen
> > > (since that's not allowed by the API), but other than that I cannot see
> > > how this could be: "total_size" is set ONLY in the
> > > "create_response_from_callback" and then compared directly with
> > > MHD_SIZE_UNKNOWN.
> > >
> > > If you have a testcase (not with IE but using libcurl as a client) that
> > > checks for this and can reproduce it (at least on 32-bit archs), that
> > > would be very helpful.   As it stands, I cannot reproduce this -- and
> > > your patch, as you mention, is certainly rather unclean (and would break
> > > 4 GB -1byte transfers).
> > >
> > > Happy hacking!
> > >
> > > Christian
> > >
> > > On Thursday 01 April 2010 10:15:09 am Brecht Sanders wrote:
> > >       
> > > > Hi,
> > > > I use libmicrohttpd on Windows (compiled under MSYS/MinGW).
> > > > I have been looking at why my dynamic pages (using
> > > > MHD_create_response_from_callback) would not display on Internet
> > > > Explorer (version 6) while they were fine on  Firefox.
> > > > Finally I found it: the header returned includes:
> > > >     Content-Length: 4294967295
> > > > This is 0xFFFFFFFF.
> > > > In the code I see:
> > > >     #define MHD_SIZE_UNKNOWN  ((uint64_t) -1LL)
> > > > which probably corresponds, assuming you use uint64_t for size
> > > > everywhere and don't mix with size_t.
> > > > My first thought to fix this was to not return the length if it is
> > > > MHD_SIZE_UNKNOWN.
> > > > However, on my platform the (MHD_SIZE_UNKNOWN !=
> > > > connection->response->total_size) comparison didn't seem to match.
> > > > I worked around it for now with the patch below using size_t typecasts.
> > > > However I suspect an underlying problem where size gets truncated from
> > > > uint64_t to a smaller type (maybe size_t) somewhere else.
> > > > So my patch is not the perfect fix in case you will have file larger
> > > > than 4G.
> > > > Regards
> > > >     Brecht Sanders
> > > >
> > > >
> > > > patch -ulb src/daemon/connection.c << EOF
> > > > --- src/daemon/connection.c  2010-03-11 13:34:20 +0100
> > > > +++ src/daemon/connection.c  2010-04-01 10:02:50 +0200
> > > > @@ -498,4 +498,5 @@
> > > >      }
> > > > -  else if (NULL == MHD_get_response_header (connection->response,
> > > > -
> > > > MHD_HTTP_HEADER_CONTENT_LENGTH))
> > > > +  else if ((NULL == MHD_get_response_header (connection->response,
> > > > +
> > > > MHD_HTTP_HEADER_CONTENT_LENGTH)) &&
> > > > +           ((size_t)MHD_SIZE_UNKNOWN !=
> > > > (size_t)connection->response->total_size))
> > > >      {
> > > > EOF
> > > >