Re: [patch] httpd: don't add date header if already set

[Ian Sutton]

committed, thanks

Re: [patch] httpd: don't add date header if already set

[Florian Obser]

On Sun, Jul 30, 2017 at 09:43:09PM -0400, Ted Unangst wrote:
> Florian Obser wrote:
> > OK florian@
> > 
> > p.s. whoever commits this please add extra ( ) around the && term,
> > Apparently clang no longer supports operator precedence *sigh*.
> > 
> > I'm wondering if it also warns for a * b + c
> > 
> > /usr/src/usr.sbin/httpd/server_fcgi.c:665:56: warning: '&&' within '||'
> >   [-Wlogical-op-parentheses]
> > if ((kv = kv_find(>http_headers, )) == NULL &&
> > ~~^~
> > /usr/src/usr.sbin/httpd/server_fcgi.c:665:56: note: place parentheses 
> > around the
> >   '&&' expression to silence this warning
> > if ((kv = kv_find(>http_headers, )) == NULL &&
> >   ^
> > 1 warning generated.
> 
> am i reading it wrong? it looks there are extra parens around ||.
> 

you are right, the diff as presented does not warn and is correct.

looks like I picked the wrong week to quit sniffing glue...
for some reason I hand applied the diff which resulted in this:

/* Date header is mandatory and should be added as late as possible */
key.kv_key = "Date";
if ((kv = kv_find(>http_headers, )) == NULL &&
server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
kv_add(>http_headers, "Date", tmbuf) == NULL)
return (-1);

and then clang warns...

> 
> > 
> > 
> > On Sat, Jul 29, 2017 at 09:11:14PM -0700, Nick Owens wrote:
> > > ping?
> > > 
> > > On Jul 18, 2017 19:01, "Nick Owens"  wrote:
> > > 
> > > hello tech@,
> > > 
> > > here is a diff that will cause httpd's fcgi code to not set the HTTP
> > > date header if it has already been set. the code i am using for an fcgi
> > > server
> > > (https://github.com/golang/go/blob/master/src/net/http/fcgi/child.go#L102)
> > > unconditionally sets the Date header, so with httpd there is a
> > > duplicate "Date:" header in responses.
> > > 
> > > quick glances at lighttpd and apache2 seem to agree with this behavior.
> > > 
> > > Index: server_fcgi.c
> > > ===
> > > RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
> > > retrieving revision 1.74
> > > diff -u -p -u -p -r1.74 server_fcgi.c
> > > --- server_fcgi.c   21 Jan 2017 11:32:04 -  1.74
> > > +++ server_fcgi.c   18 Jul 2017 21:31:01 -
> > > @@ -661,8 +661,10 @@ server_fcgi_header(struct client *clt, u
> > > }
> > > 
> > > /* Date header is mandatory and should be added as late as 
> > > possible
> > > */
> > > -   if (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
> > > -   kv_add(>http_headers, "Date", tmbuf) == NULL)
> > > +   key.kv_key = "Date";
> > > +   if ((kv = kv_find(>http_headers, )) == NULL &&
> > > +   (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
> > > +   kv_add(>http_headers, "Date", tmbuf) == NULL))
> > > return (-1);
> > > 
> > > /* Write initial header (fcgi might append more) */
> > 
> > -- 
> > I'm not entirely sure you are real.
> > 
> 

-- 
I'm not entirely sure you are real.

Re: [patch] httpd: don't add date header if already set

[Ted Unangst]

Florian Obser wrote:
> OK florian@
> 
> p.s. whoever commits this please add extra ( ) around the && term,
> Apparently clang no longer supports operator precedence *sigh*.
> 
> I'm wondering if it also warns for a * b + c
> 
> /usr/src/usr.sbin/httpd/server_fcgi.c:665:56: warning: '&&' within '||'
>   [-Wlogical-op-parentheses]
> if ((kv = kv_find(>http_headers, )) == NULL &&
> ~~^~
> /usr/src/usr.sbin/httpd/server_fcgi.c:665:56: note: place parentheses around 
> the
>   '&&' expression to silence this warning
> if ((kv = kv_find(>http_headers, )) == NULL &&
>   ^
> 1 warning generated.

am i reading it wrong? it looks there are extra parens around ||.


> 
> 
> On Sat, Jul 29, 2017 at 09:11:14PM -0700, Nick Owens wrote:
> > ping?
> > 
> > On Jul 18, 2017 19:01, "Nick Owens"  wrote:
> > 
> > hello tech@,
> > 
> > here is a diff that will cause httpd's fcgi code to not set the HTTP
> > date header if it has already been set. the code i am using for an fcgi
> > server
> > (https://github.com/golang/go/blob/master/src/net/http/fcgi/child.go#L102)
> > unconditionally sets the Date header, so with httpd there is a
> > duplicate "Date:" header in responses.
> > 
> > quick glances at lighttpd and apache2 seem to agree with this behavior.
> > 
> > Index: server_fcgi.c
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
> > retrieving revision 1.74
> > diff -u -p -u -p -r1.74 server_fcgi.c
> > --- server_fcgi.c   21 Jan 2017 11:32:04 -  1.74
> > +++ server_fcgi.c   18 Jul 2017 21:31:01 -
> > @@ -661,8 +661,10 @@ server_fcgi_header(struct client *clt, u
> > }
> > 
> > /* Date header is mandatory and should be added as late as possible
> > */
> > -   if (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
> > -   kv_add(>http_headers, "Date", tmbuf) == NULL)
> > +   key.kv_key = "Date";
> > +   if ((kv = kv_find(>http_headers, )) == NULL &&
> > +   (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
> > +   kv_add(>http_headers, "Date", tmbuf) == NULL))
> > return (-1);
> > 
> > /* Write initial header (fcgi might append more) */
> 
> -- 
> I'm not entirely sure you are real.
> 

Re: [patch] httpd: don't add date header if already set

[Florian Obser]

OK florian@

p.s. whoever commits this please add extra ( ) around the && term,
Apparently clang no longer supports operator precedence *sigh*.

I'm wondering if it also warns for a * b + c

/usr/src/usr.sbin/httpd/server_fcgi.c:665:56: warning: '&&' within '||'
  [-Wlogical-op-parentheses]
if ((kv = kv_find(>http_headers, )) == NULL &&
~~^~
/usr/src/usr.sbin/httpd/server_fcgi.c:665:56: note: place parentheses around the
  '&&' expression to silence this warning
if ((kv = kv_find(>http_headers, )) == NULL &&
  ^
1 warning generated.


On Sat, Jul 29, 2017 at 09:11:14PM -0700, Nick Owens wrote:
> ping?
> 
> On Jul 18, 2017 19:01, "Nick Owens"  wrote:
> 
> hello tech@,
> 
> here is a diff that will cause httpd's fcgi code to not set the HTTP
> date header if it has already been set. the code i am using for an fcgi
> server
> (https://github.com/golang/go/blob/master/src/net/http/fcgi/child.go#L102)
> unconditionally sets the Date header, so with httpd there is a
> duplicate "Date:" header in responses.
> 
> quick glances at lighttpd and apache2 seem to agree with this behavior.
> 
> Index: server_fcgi.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
> retrieving revision 1.74
> diff -u -p -u -p -r1.74 server_fcgi.c
> --- server_fcgi.c   21 Jan 2017 11:32:04 -  1.74
> +++ server_fcgi.c   18 Jul 2017 21:31:01 -
> @@ -661,8 +661,10 @@ server_fcgi_header(struct client *clt, u
> }
> 
> /* Date header is mandatory and should be added as late as possible
> */
> -   if (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
> -   kv_add(>http_headers, "Date", tmbuf) == NULL)
> +   key.kv_key = "Date";
> +   if ((kv = kv_find(>http_headers, )) == NULL &&
> +   (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
> +   kv_add(>http_headers, "Date", tmbuf) == NULL))
> return (-1);
> 
> /* Write initial header (fcgi might append more) */

-- 
I'm not entirely sure you are real.

Re: [patch] httpd: don't add date header if already set

[Nick Owens]

ping?

On Jul 18, 2017 19:01, "Nick Owens"  wrote:

hello tech@,

here is a diff that will cause httpd's fcgi code to not set the HTTP
date header if it has already been set. the code i am using for an fcgi
server
(https://github.com/golang/go/blob/master/src/net/http/fcgi/child.go#L102)
unconditionally sets the Date header, so with httpd there is a
duplicate "Date:" header in responses.

quick glances at lighttpd and apache2 seem to agree with this behavior.

Index: server_fcgi.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
retrieving revision 1.74
diff -u -p -u -p -r1.74 server_fcgi.c
--- server_fcgi.c   21 Jan 2017 11:32:04 -  1.74
+++ server_fcgi.c   18 Jul 2017 21:31:01 -
@@ -661,8 +661,10 @@ server_fcgi_header(struct client *clt, u
}

/* Date header is mandatory and should be added as late as possible
*/
-   if (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
-   kv_add(>http_headers, "Date", tmbuf) == NULL)
+   key.kv_key = "Date";
+   if ((kv = kv_find(>http_headers, )) == NULL &&
+   (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
+   kv_add(>http_headers, "Date", tmbuf) == NULL))
return (-1);

/* Write initial header (fcgi might append more) */

[patch] httpd: don't add date header if already set

[Nick Owens]

hello tech@,

here is a diff that will cause httpd's fcgi code to not set the HTTP 
date header if it has already been set. the code i am using for an fcgi 
server 
(https://github.com/golang/go/blob/master/src/net/http/fcgi/child.go#L102) 
unconditionally sets the Date header, so with httpd there is a
duplicate "Date:" header in responses.

quick glances at lighttpd and apache2 seem to agree with this behavior.

Index: server_fcgi.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
retrieving revision 1.74
diff -u -p -u -p -r1.74 server_fcgi.c
--- server_fcgi.c   21 Jan 2017 11:32:04 -  1.74
+++ server_fcgi.c   18 Jul 2017 21:31:01 -
@@ -661,8 +661,10 @@ server_fcgi_header(struct client *clt, u
}
 
/* Date header is mandatory and should be added as late as possible */
-   if (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
-   kv_add(>http_headers, "Date", tmbuf) == NULL)
+   key.kv_key = "Date";
+   if ((kv = kv_find(>http_headers, )) == NULL &&
+   (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
+   kv_add(>http_headers, "Date", tmbuf) == NULL))
return (-1);
 
/* Write initial header (fcgi might append more) */