Re: httpd response mimetype bug

2018-06-15 Thread Hiltjo Posthuma 
On Wed, Jun 13, 2018 at 08:07:11PM +0200, Reyk Floeter wrote:
> On Sat, Jan 13, 2018 at 05:23:35PM +0100, Sebastian Benoit wrote:
> > Hiltjo Posthuma(hil...@codemadness.org) on 2018.01.13 13:08:38 +0100:
> > > On Sat, Jan 13, 2018 at 09:39:44AM +0100, Anton Lindqvist wrote:
> > > > On Tue, Jan 09, 2018 at 05:38:57PM +0100, Hidv?gi G?bor wrote:
> > > > > >Synopsis: httpd reports wrong mimetype when item is in the browser 
> > > > > >cache
> > > > > >Category: httpd
> > > > > >Environment:
> > > > > System  : OpenBSD 6.2
> > > > > Details : OpenBSD 6.2 (GENERIC) #91: Wed Oct  4 00:35:21 
> > > > > MDT
> > > > > 2017
> > > > > 
> > > > > dera...@armv7.openbsd.org:/usr/src/sys/arch/armv7/compile/GENERIC
> > > > > 
> > > > > Architecture: OpenBSD.armv7
> > > > > Machine : armv7
> > > > > >Description:
> > > > > 
> > > > > httpd serves static files (eg. images) with Last-Modified http 
> > > > > header. When
> > > > > a browser next time asks whether this file changed (sends 
> > > > > If-Modified-Since
> > > > > http header) httpd responds with wrong mimetype, 'text/html' when the
> > > > > resource is in the browser cache (304 Not Modified status code).
> > > > > 
> > > > > >How-To-Repeat:
> > > > > 
> > > > > This bug is common, not arm only. When for example you open this 
> > > > > image:
> > > > > https://man.openbsd.org/openbsd.gif
> > > > > 
> > > > > in a browser with developer tools (F12) open, on the network tab you 
> > > > > can
> > > > > take a look at the response headers, mimetype is correct (image/gif). 
> > > > > After
> > > > > opening press refresh (F5) and look at the response headers again, 
> > > > > and you
> > > > > get the incorrect mimetype, 'text/html'.
> > > > > 
> > > > > >Fix:
> > > > > 
> > > > > check httpd source
> > > > 
> > > > Please try out this diff, it makes sure to set the correct MIME-type and
> > > > not respond with a body if the resource has not changed. Sending this to
> > > > tech@ as well.
> > > > 
> > > > Index: server_file.c
> > > > ===
> > > > RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> > > > retrieving revision 1.65
> > > > diff -u -p -r1.65 server_file.c
> > > > --- server_file.c   2 Feb 2017 22:19:59 -   1.65
> > > > +++ server_file.c   12 Jan 2018 19:10:20 -
> > > > @@ -230,8 +230,15 @@ server_file_request(struct httpd *env, s
> > > > goto abort;
> > > > }
> > > >  
> > > > -   if ((ret = server_file_modified_since(clt->clt_descreq, st)) != 
> > > > -1)
> > > > -   return (ret);
> > > > +   if (server_file_modified_since(clt->clt_descreq, st) == 0) {
> > > > +   media = media_find_config(env, srv_conf, path);
> > > > +   ret = server_response_http(clt, 304, media, 0,
> > > > +   st->st_mtim.tv_sec);
> > > > +   if (ret != -1)
> > > > +   goto done;
> > > > +   else
> > > > +   goto fail;
> > > > +   }
> > > >  
> > > > /* Now open the file, should be readable or we have another 
> > > > problem */
> > > > if ((fd = open(path, O_RDONLY)) == -1)
> > > > @@ -663,10 +670,10 @@ server_file_modified_since(struct http_d
> > > > if (strptime(since->kv_value,
> > > > "%a, %d %h %Y %T %Z", &tm) != NULL &&
> > > > timegm(&tm) >= st->st_mtim.tv_sec)
> > > > -   return (304);
> > > > +   return (0);
> > > > }
> > > >  
> > > > -   return (-1);
> > > > +   return (1);
> > > >  }
> > > >  
> > > >  int
> > > > 
> > > 
> > > Hey,
> > > 
> > > I've tested your patch.
> > > 
> > > When requesting a non-modified CSS file:
> > > 
> > >   #!/bin/sh
> > >   host="127.0.0.1"
> > >   port="6970"
> > >   printf 'GET /style.css HTTP/1.1\r\nHost: %s:%s\r\nIf-Modified-Since: 
> > > Sat, 16 Dec 2017 13:07:53 GMT\r\nConnection: close\r\n\r\n' "$host" 
> > > "$port" | \
> > >   nc "$host" "$port"
> > > 
> > > Full HTTP response:
> > > 
> > >   HTTP/1.1 304 Not Modified
> > >   Connection: close
> > >   Content-Length: 0
> > >   Content-Type: text/css
> > >   Date: Sat, 13 Jan 2018 11:54:13 GMT
> > >   Last-Modified: Sun, 05 Mar 2017 12:22:05 GMT
> > >   Server: OpenBSD httpd
> > > 
> > > I wonder if httpd should just omit the response header Content-Length and
> > > Content-Type entirely for this statuscode. Some httpd such as Nginx just
> > > omit them aswell.
> > 
> > rfc7230  HTTP/1.1 Message Syntax and Routing page 29f.
> > 
> >A server MAY send a Content-Length header field in a 304 (Not
> >Modified) response to a conditional GET request (Section 4.1 of
> >[RFC7232]); a server MUST NOT send Content-Length in such a response
> >   *unless its field-value equals the decimal number of octets that would*
> >have been sent in the payload body of a 200 (OK)

Re: httpd response mimetype bug

2018-06-13 Thread Reyk Floeter 
On Sat, Jan 13, 2018 at 05:23:35PM +0100, Sebastian Benoit wrote:
> Hiltjo Posthuma(hil...@codemadness.org) on 2018.01.13 13:08:38 +0100:
> > On Sat, Jan 13, 2018 at 09:39:44AM +0100, Anton Lindqvist wrote:
> > > On Tue, Jan 09, 2018 at 05:38:57PM +0100, Hidv?gi G?bor wrote:
> > > > >Synopsis: httpd reports wrong mimetype when item is in the browser 
> > > > >cache
> > > > >Category: httpd
> > > > >Environment:
> > > > System  : OpenBSD 6.2
> > > > Details : OpenBSD 6.2 (GENERIC) #91: Wed Oct  4 00:35:21 MDT
> > > > 2017
> > > > 
> > > > dera...@armv7.openbsd.org:/usr/src/sys/arch/armv7/compile/GENERIC
> > > > 
> > > > Architecture: OpenBSD.armv7
> > > > Machine : armv7
> > > > >Description:
> > > > 
> > > > httpd serves static files (eg. images) with Last-Modified http header. 
> > > > When
> > > > a browser next time asks whether this file changed (sends 
> > > > If-Modified-Since
> > > > http header) httpd responds with wrong mimetype, 'text/html' when the
> > > > resource is in the browser cache (304 Not Modified status code).
> > > > 
> > > > >How-To-Repeat:
> > > > 
> > > > This bug is common, not arm only. When for example you open this image:
> > > > https://man.openbsd.org/openbsd.gif
> > > > 
> > > > in a browser with developer tools (F12) open, on the network tab you can
> > > > take a look at the response headers, mimetype is correct (image/gif). 
> > > > After
> > > > opening press refresh (F5) and look at the response headers again, and 
> > > > you
> > > > get the incorrect mimetype, 'text/html'.
> > > > 
> > > > >Fix:
> > > > 
> > > > check httpd source
> > > 
> > > Please try out this diff, it makes sure to set the correct MIME-type and
> > > not respond with a body if the resource has not changed. Sending this to
> > > tech@ as well.
> > > 
> > > Index: server_file.c
> > > ===
> > > RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> > > retrieving revision 1.65
> > > diff -u -p -r1.65 server_file.c
> > > --- server_file.c 2 Feb 2017 22:19:59 -   1.65
> > > +++ server_file.c 12 Jan 2018 19:10:20 -
> > > @@ -230,8 +230,15 @@ server_file_request(struct httpd *env, s
> > >   goto abort;
> > >   }
> > >  
> > > - if ((ret = server_file_modified_since(clt->clt_descreq, st)) != -1)
> > > - return (ret);
> > > + if (server_file_modified_since(clt->clt_descreq, st) == 0) {
> > > + media = media_find_config(env, srv_conf, path);
> > > + ret = server_response_http(clt, 304, media, 0,
> > > + st->st_mtim.tv_sec);
> > > + if (ret != -1)
> > > + goto done;
> > > + else
> > > + goto fail;
> > > + }
> > >  
> > >   /* Now open the file, should be readable or we have another problem */
> > >   if ((fd = open(path, O_RDONLY)) == -1)
> > > @@ -663,10 +670,10 @@ server_file_modified_since(struct http_d
> > >   if (strptime(since->kv_value,
> > >   "%a, %d %h %Y %T %Z", &tm) != NULL &&
> > >   timegm(&tm) >= st->st_mtim.tv_sec)
> > > - return (304);
> > > + return (0);
> > >   }
> > >  
> > > - return (-1);
> > > + return (1);
> > >  }
> > >  
> > >  int
> > > 
> > 
> > Hey,
> > 
> > I've tested your patch.
> > 
> > When requesting a non-modified CSS file:
> > 
> > #!/bin/sh
> > host="127.0.0.1"
> > port="6970"
> > printf 'GET /style.css HTTP/1.1\r\nHost: %s:%s\r\nIf-Modified-Since: 
> > Sat, 16 Dec 2017 13:07:53 GMT\r\nConnection: close\r\n\r\n' "$host" "$port" 
> > | \
> > nc "$host" "$port"
> > 
> > Full HTTP response:
> > 
> > HTTP/1.1 304 Not Modified
> > Connection: close
> > Content-Length: 0
> > Content-Type: text/css
> > Date: Sat, 13 Jan 2018 11:54:13 GMT
> > Last-Modified: Sun, 05 Mar 2017 12:22:05 GMT
> > Server: OpenBSD httpd
> > 
> > I wonder if httpd should just omit the response header Content-Length and
> > Content-Type entirely for this statuscode. Some httpd such as Nginx just
> > omit them aswell.
> 
> rfc7230  HTTP/1.1 Message Syntax and Routing page 29f.
> 
>A server MAY send a Content-Length header field in a 304 (Not
>Modified) response to a conditional GET request (Section 4.1 of
>[RFC7232]); a server MUST NOT send Content-Length in such a response
>   *unless its field-value equals the decimal number of octets that would*
>have been sent in the payload body of a 200 (OK) response to the same
>request.
> 

So I think it is better to omit the Content-Length header.  I'm afraid
that a (potentially broken) client will wait for a body even with the
304 response.

The diff below is based on Anton's fix but without changing the code's
style to a boolean 0/1.  It also avoids an mtime in the future (same
as the other server_response_http() further below), and omits the
Content-Length by accepting a -1 size.

Reyk

Index: usr.sbin/httpd/server_fil

Re: httpd response mimetype bug

2018-01-13 Thread Sebastian Benoit 
Hiltjo Posthuma(hil...@codemadness.org) on 2018.01.13 13:08:38 +0100:
> On Sat, Jan 13, 2018 at 09:39:44AM +0100, Anton Lindqvist wrote:
> > On Tue, Jan 09, 2018 at 05:38:57PM +0100, Hidv?gi G?bor wrote:
> > > >Synopsis: httpd reports wrong mimetype when item is in the browser cache
> > > >Category: httpd
> > > >Environment:
> > > System  : OpenBSD 6.2
> > > Details : OpenBSD 6.2 (GENERIC) #91: Wed Oct  4 00:35:21 MDT
> > > 2017
> > > 
> > > dera...@armv7.openbsd.org:/usr/src/sys/arch/armv7/compile/GENERIC
> > > 
> > > Architecture: OpenBSD.armv7
> > > Machine : armv7
> > > >Description:
> > > 
> > > httpd serves static files (eg. images) with Last-Modified http header. 
> > > When
> > > a browser next time asks whether this file changed (sends 
> > > If-Modified-Since
> > > http header) httpd responds with wrong mimetype, 'text/html' when the
> > > resource is in the browser cache (304 Not Modified status code).
> > > 
> > > >How-To-Repeat:
> > > 
> > > This bug is common, not arm only. When for example you open this image:
> > > https://man.openbsd.org/openbsd.gif
> > > 
> > > in a browser with developer tools (F12) open, on the network tab you can
> > > take a look at the response headers, mimetype is correct (image/gif). 
> > > After
> > > opening press refresh (F5) and look at the response headers again, and you
> > > get the incorrect mimetype, 'text/html'.
> > > 
> > > >Fix:
> > > 
> > > check httpd source
> > 
> > Please try out this diff, it makes sure to set the correct MIME-type and
> > not respond with a body if the resource has not changed. Sending this to
> > tech@ as well.
> > 
> > Index: server_file.c
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> > retrieving revision 1.65
> > diff -u -p -r1.65 server_file.c
> > --- server_file.c   2 Feb 2017 22:19:59 -   1.65
> > +++ server_file.c   12 Jan 2018 19:10:20 -
> > @@ -230,8 +230,15 @@ server_file_request(struct httpd *env, s
> > goto abort;
> > }
> >  
> > -   if ((ret = server_file_modified_since(clt->clt_descreq, st)) != -1)
> > -   return (ret);
> > +   if (server_file_modified_since(clt->clt_descreq, st) == 0) {
> > +   media = media_find_config(env, srv_conf, path);
> > +   ret = server_response_http(clt, 304, media, 0,
> > +   st->st_mtim.tv_sec);
> > +   if (ret != -1)
> > +   goto done;
> > +   else
> > +   goto fail;
> > +   }
> >  
> > /* Now open the file, should be readable or we have another problem */
> > if ((fd = open(path, O_RDONLY)) == -1)
> > @@ -663,10 +670,10 @@ server_file_modified_since(struct http_d
> > if (strptime(since->kv_value,
> > "%a, %d %h %Y %T %Z", &tm) != NULL &&
> > timegm(&tm) >= st->st_mtim.tv_sec)
> > -   return (304);
> > +   return (0);
> > }
> >  
> > -   return (-1);
> > +   return (1);
> >  }
> >  
> >  int
> > 
> 
> Hey,
> 
> I've tested your patch.
> 
> When requesting a non-modified CSS file:
> 
>   #!/bin/sh
>   host="127.0.0.1"
>   port="6970"
>   printf 'GET /style.css HTTP/1.1\r\nHost: %s:%s\r\nIf-Modified-Since: 
> Sat, 16 Dec 2017 13:07:53 GMT\r\nConnection: close\r\n\r\n' "$host" "$port" | 
> \
>   nc "$host" "$port"
> 
> Full HTTP response:
> 
>   HTTP/1.1 304 Not Modified
>   Connection: close
>   Content-Length: 0
>   Content-Type: text/css
>   Date: Sat, 13 Jan 2018 11:54:13 GMT
>   Last-Modified: Sun, 05 Mar 2017 12:22:05 GMT
>   Server: OpenBSD httpd
> 
> I wonder if httpd should just omit the response header Content-Length and
> Content-Type entirely for this statuscode. Some httpd such as Nginx just
> omit them aswell.

rfc7230  HTTP/1.1 Message Syntax and Routing page 29f.

   A server MAY send a Content-Length header field in a 304 (Not
   Modified) response to a conditional GET request (Section 4.1 of
   [RFC7232]); a server MUST NOT send Content-Length in such a response
  *unless its field-value equals the decimal number of octets that would*
   have been sent in the payload body of a 200 (OK) response to the same
   request.

> At the moment responses with redirects (statuscode 301 and 302) also
> output a body response. Maybe it is better to handle it in server_http.c
> in the function server_abort_http() and not output the body there for some
> response status codes? I'm not sure.
> 
> -- 
> Kind regards,
> Hiltjo
>

Re: httpd response mimetype bug

2018-01-13 Thread Anton Lindqvist 
On Sat, Jan 13, 2018 at 01:08:38PM +0100, Hiltjo Posthuma wrote:
> On Sat, Jan 13, 2018 at 09:39:44AM +0100, Anton Lindqvist wrote:
> > On Tue, Jan 09, 2018 at 05:38:57PM +0100, Hidvégi Gábor wrote:
> > > >Synopsis: httpd reports wrong mimetype when item is in the browser cache
> > > >Category: httpd
> > > >Environment:
> > > System  : OpenBSD 6.2
> > > Details : OpenBSD 6.2 (GENERIC) #91: Wed Oct  4 00:35:21 MDT
> > > 2017
> > > 
> > > dera...@armv7.openbsd.org:/usr/src/sys/arch/armv7/compile/GENERIC
> > > 
> > > Architecture: OpenBSD.armv7
> > > Machine : armv7
> > > >Description:
> > > 
> > > httpd serves static files (eg. images) with Last-Modified http header. 
> > > When
> > > a browser next time asks whether this file changed (sends 
> > > If-Modified-Since
> > > http header) httpd responds with wrong mimetype, 'text/html' when the
> > > resource is in the browser cache (304 Not Modified status code).
> > > 
> > > >How-To-Repeat:
> > > 
> > > This bug is common, not arm only. When for example you open this image:
> > > https://man.openbsd.org/openbsd.gif
> > > 
> > > in a browser with developer tools (F12) open, on the network tab you can
> > > take a look at the response headers, mimetype is correct (image/gif). 
> > > After
> > > opening press refresh (F5) and look at the response headers again, and you
> > > get the incorrect mimetype, 'text/html'.
> > > 
> > > >Fix:
> > > 
> > > check httpd source
> > 
> > Please try out this diff, it makes sure to set the correct MIME-type and
> > not respond with a body if the resource has not changed. Sending this to
> > tech@ as well.
> > 
> > Index: server_file.c
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> > retrieving revision 1.65
> > diff -u -p -r1.65 server_file.c
> > --- server_file.c   2 Feb 2017 22:19:59 -   1.65
> > +++ server_file.c   12 Jan 2018 19:10:20 -
> > @@ -230,8 +230,15 @@ server_file_request(struct httpd *env, s
> > goto abort;
> > }
> >  
> > -   if ((ret = server_file_modified_since(clt->clt_descreq, st)) != -1)
> > -   return (ret);
> > +   if (server_file_modified_since(clt->clt_descreq, st) == 0) {
> > +   media = media_find_config(env, srv_conf, path);
> > +   ret = server_response_http(clt, 304, media, 0,
> > +   st->st_mtim.tv_sec);
> > +   if (ret != -1)
> > +   goto done;
> > +   else
> > +   goto fail;
> > +   }
> >  
> > /* Now open the file, should be readable or we have another problem */
> > if ((fd = open(path, O_RDONLY)) == -1)
> > @@ -663,10 +670,10 @@ server_file_modified_since(struct http_d
> > if (strptime(since->kv_value,
> > "%a, %d %h %Y %T %Z", &tm) != NULL &&
> > timegm(&tm) >= st->st_mtim.tv_sec)
> > -   return (304);
> > +   return (0);
> > }
> >  
> > -   return (-1);
> > +   return (1);
> >  }
> >  
> >  int
> > 
> 
> Hey,
> 
> I've tested your patch.
> 
> When requesting a non-modified CSS file:
> 
>   #!/bin/sh
>   host="127.0.0.1"
>   port="6970"
>   printf 'GET /style.css HTTP/1.1\r\nHost: %s:%s\r\nIf-Modified-Since: 
> Sat, 16 Dec 2017 13:07:53 GMT\r\nConnection: close\r\n\r\n' "$host" "$port" | 
> \
>   nc "$host" "$port"
> 
> Full HTTP response:
> 
>   HTTP/1.1 304 Not Modified
>   Connection: close
>   Content-Length: 0
>   Content-Type: text/css
>   Date: Sat, 13 Jan 2018 11:54:13 GMT
>   Last-Modified: Sun, 05 Mar 2017 12:22:05 GMT
>   Server: OpenBSD httpd
> 
> I wonder if httpd should just omit the response header Content-Length and
> Content-Type entirely for this statuscode. Some httpd such as Nginx just
> omit them aswell.
> 
> At the moment responses with redirects (statuscode 301 and 302) also
> output a body response. Maybe it is better to handle it in server_http.c
> in the function server_abort_http() and not output the body there for some
> response status codes? I'm not sure.

Quoting the RFC[1]:

> A server MAY send a Content-Length header field in a 304 (Not
> Modified) response to a conditional GET request ...; a server MUST NOT
> send Content-Length in such a response unless its field-value equals
> the decimal number of octets that would have been sent in the payload
> body of a 200 (OK) response to the same request.

So my previous diff was obviously wrong since the actual size must be
used if the Content-Length header is included. Including it seems to
result in a less intrusive diff. As for the Content-Type header, I can't
find any guidance on whetever it can or cannot be included.

[1] https://tools.ietf.org/html/rfc7230#section-3.3.2

Index: server_file.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
retrieving revision 1.65
diff

Re: httpd response mimetype bug

2018-01-13 Thread Hiltjo Posthuma 
On Sat, Jan 13, 2018 at 09:39:44AM +0100, Anton Lindqvist wrote:
> On Tue, Jan 09, 2018 at 05:38:57PM +0100, Hidvégi Gábor wrote:
> > >Synopsis: httpd reports wrong mimetype when item is in the browser cache
> > >Category: httpd
> > >Environment:
> > System  : OpenBSD 6.2
> > Details : OpenBSD 6.2 (GENERIC) #91: Wed Oct  4 00:35:21 MDT
> > 2017
> > 
> > dera...@armv7.openbsd.org:/usr/src/sys/arch/armv7/compile/GENERIC
> > 
> > Architecture: OpenBSD.armv7
> > Machine : armv7
> > >Description:
> > 
> > httpd serves static files (eg. images) with Last-Modified http header. When
> > a browser next time asks whether this file changed (sends If-Modified-Since
> > http header) httpd responds with wrong mimetype, 'text/html' when the
> > resource is in the browser cache (304 Not Modified status code).
> > 
> > >How-To-Repeat:
> > 
> > This bug is common, not arm only. When for example you open this image:
> > https://man.openbsd.org/openbsd.gif
> > 
> > in a browser with developer tools (F12) open, on the network tab you can
> > take a look at the response headers, mimetype is correct (image/gif). After
> > opening press refresh (F5) and look at the response headers again, and you
> > get the incorrect mimetype, 'text/html'.
> > 
> > >Fix:
> > 
> > check httpd source
> 
> Please try out this diff, it makes sure to set the correct MIME-type and
> not respond with a body if the resource has not changed. Sending this to
> tech@ as well.
> 
> Index: server_file.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> retrieving revision 1.65
> diff -u -p -r1.65 server_file.c
> --- server_file.c 2 Feb 2017 22:19:59 -   1.65
> +++ server_file.c 12 Jan 2018 19:10:20 -
> @@ -230,8 +230,15 @@ server_file_request(struct httpd *env, s
>   goto abort;
>   }
>  
> - if ((ret = server_file_modified_since(clt->clt_descreq, st)) != -1)
> - return (ret);
> + if (server_file_modified_since(clt->clt_descreq, st) == 0) {
> + media = media_find_config(env, srv_conf, path);
> + ret = server_response_http(clt, 304, media, 0,
> + st->st_mtim.tv_sec);
> + if (ret != -1)
> + goto done;
> + else
> + goto fail;
> + }
>  
>   /* Now open the file, should be readable or we have another problem */
>   if ((fd = open(path, O_RDONLY)) == -1)
> @@ -663,10 +670,10 @@ server_file_modified_since(struct http_d
>   if (strptime(since->kv_value,
>   "%a, %d %h %Y %T %Z", &tm) != NULL &&
>   timegm(&tm) >= st->st_mtim.tv_sec)
> - return (304);
> + return (0);
>   }
>  
> - return (-1);
> + return (1);
>  }
>  
>  int
> 

Hey,

I've tested your patch.

When requesting a non-modified CSS file:

#!/bin/sh
host="127.0.0.1"
port="6970"
printf 'GET /style.css HTTP/1.1\r\nHost: %s:%s\r\nIf-Modified-Since: 
Sat, 16 Dec 2017 13:07:53 GMT\r\nConnection: close\r\n\r\n' "$host" "$port" | \
nc "$host" "$port"

Full HTTP response:

HTTP/1.1 304 Not Modified
Connection: close
Content-Length: 0
Content-Type: text/css
Date: Sat, 13 Jan 2018 11:54:13 GMT
Last-Modified: Sun, 05 Mar 2017 12:22:05 GMT
Server: OpenBSD httpd

I wonder if httpd should just omit the response header Content-Length and
Content-Type entirely for this statuscode. Some httpd such as Nginx just
omit them aswell.

At the moment responses with redirects (statuscode 301 and 302) also
output a body response. Maybe it is better to handle it in server_http.c
in the function server_abort_http() and not output the body there for some
response status codes? I'm not sure.

-- 
Kind regards,
Hiltjo

Re: httpd response mimetype bug

2018-01-13 Thread Anton Lindqvist 
On Tue, Jan 09, 2018 at 05:38:57PM +0100, Hidvégi Gábor wrote:
> >Synopsis: httpd reports wrong mimetype when item is in the browser cache
> >Category: httpd
> >Environment:
> System  : OpenBSD 6.2
> Details : OpenBSD 6.2 (GENERIC) #91: Wed Oct  4 00:35:21 MDT
> 2017
> 
> dera...@armv7.openbsd.org:/usr/src/sys/arch/armv7/compile/GENERIC
> 
> Architecture: OpenBSD.armv7
> Machine : armv7
> >Description:
> 
> httpd serves static files (eg. images) with Last-Modified http header. When
> a browser next time asks whether this file changed (sends If-Modified-Since
> http header) httpd responds with wrong mimetype, 'text/html' when the
> resource is in the browser cache (304 Not Modified status code).
> 
> >How-To-Repeat:
> 
> This bug is common, not arm only. When for example you open this image:
> https://man.openbsd.org/openbsd.gif
> 
> in a browser with developer tools (F12) open, on the network tab you can
> take a look at the response headers, mimetype is correct (image/gif). After
> opening press refresh (F5) and look at the response headers again, and you
> get the incorrect mimetype, 'text/html'.
> 
> >Fix:
> 
> check httpd source

Please try out this diff, it makes sure to set the correct MIME-type and
not respond with a body if the resource has not changed. Sending this to
tech@ as well.

Index: server_file.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
retrieving revision 1.65
diff -u -p -r1.65 server_file.c
--- server_file.c   2 Feb 2017 22:19:59 -   1.65
+++ server_file.c   12 Jan 2018 19:10:20 -
@@ -230,8 +230,15 @@ server_file_request(struct httpd *env, s
goto abort;
}
 
-   if ((ret = server_file_modified_since(clt->clt_descreq, st)) != -1)
-   return (ret);
+   if (server_file_modified_since(clt->clt_descreq, st) == 0) {
+   media = media_find_config(env, srv_conf, path);
+   ret = server_response_http(clt, 304, media, 0,
+   st->st_mtim.tv_sec);
+   if (ret != -1)
+   goto done;
+   else
+   goto fail;
+   }
 
/* Now open the file, should be readable or we have another problem */
if ((fd = open(path, O_RDONLY)) == -1)
@@ -663,10 +670,10 @@ server_file_modified_since(struct http_d
if (strptime(since->kv_value,
"%a, %d %h %Y %T %Z", &tm) != NULL &&
timegm(&tm) >= st->st_mtim.tv_sec)
-   return (304);
+   return (0);
}
 
-   return (-1);
+   return (1);
 }
 
 int