Re: httpd(8): fix location duplicate detection

2020-10-26 Thread Denis Fondras
On Mon, Oct 26, 2020 at 09:28:54AM +0100, m...@fn.de wrote:
> Ping. Latest diff below.
> 

OK denis@

I will commit tonight if nobody stands against.
Thank you.

> Index: usr.sbin/httpd/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> retrieving revision 1.118
> diff -u -p -u -p -r1.118 parse.y
> --- usr.sbin/httpd/parse.y11 Oct 2020 03:21:44 -  1.118
> +++ usr.sbin/httpd/parse.y26 Oct 2020 08:26:48 -
> @@ -587,8 +587,10 @@ serveroptsl  : LISTEN ON STRING opttls po
>   struct server   *s = NULL;
>  
>   TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
> + /* Compare locations of same parent server */
>   if ((s->srv_conf.flags & SRVFLAG_LOCATION) &&
> - s->srv_conf.id == srv_conf->id &&
> + s->srv_conf.parent_id ==
> + srv_conf->parent_id &&
>   strcmp(s->srv_conf.location,
>   srv_conf->location) == 0)
>   break;
> 
> 
> On 2020-10-11 12:00, m...@fn.de wrote:
> > Ping. Updated diff below.
> > 
> > ---
> > Index: usr.sbin/httpd/parse.y
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> > retrieving revision 1.118
> > diff -u -p -u -p -r1.118 parse.y
> > --- usr.sbin/httpd/parse.y  11 Oct 2020 03:21:44 -  1.118
> > +++ usr.sbin/httpd/parse.y  11 Oct 2020 09:52:34 -
> > @@ -588,7 +588,8 @@ serveroptsl : LISTEN ON STRING opttls po
> >  
> > TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
> > if ((s->srv_conf.flags & SRVFLAG_LOCATION) &&
> > -   s->srv_conf.id == srv_conf->id &&
> > +   s->srv_conf.parent_id ==
> > +   srv_conf->parent_id &&
> > strcmp(s->srv_conf.location,
> > srv_conf->location) == 0)
> > break;
> > ---
> > 
> > On 2020-09-26 08:57, m...@fn.de wrote:
> >> During httpd setup I realized that duplicate location names are not
> >> being detected even though I remembered having seen a corresponding
> >> piece of code in 'usr.sbin/httpd/parse.y' the other day.  As far
> >> as I understand, the comparison 's->srv_conf.id == srv_conf->id'
> >> can never be true as a newly created location ID would never match
> >> the ID of any existing location.
> >>
> >> To check whether or not I was right, I recompiled httpd with DEBUG
> >> enabled and tried to start the server with the following (actually
> >> invalid) httpd.conf:
> >>
> >> 
> >> server "testserver" {
> >>  listen on 127.0.0.1 port www
> >>  location "/foo" { block }
> >>  location "/foo" { block }
> >> }
> >> 
> >>
> >> # httpd -vvd
> >> startup
> >> adding location "/foo" for "testserver[2]"
> >> adding location "/foo" for "testserver[3]"
> >> adding server "testserver[1]"
> >> 
> >> (httpd running)
> >>
> >> I guess the intention was to compare the new location name with all
> >> other location names available under the same parent server.  I
> >> accomplished this by applying the patch at the bottom of this
> >> message.  After recompiling, httpd startup terminates as expected.
> >>
> >> # httpd -vvd
> >> startup
> >> adding location "/foo" for "testserver[2]"
> >> /etc/httpd.conf:4: location "/foo" defined twice
> >> .
> >> logger exiting, pid 98967
> >> server exiting, pid 27723
> >> server exiting, pid 78507
> >> server exiting, pid 25743
> >>
> >>
> >> comments? OK?
> >>
> >> ---
> >>
> >> Index: usr.sbin/httpd/parse.y
> >> ===
> >> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> >> retrieving revision 1.117
> >> diff -u -p -u -p -r1.117 parse.y
> >> --- usr.sbin/httpd/parse.y 26 Aug 2020 06:50:20 -  1.117
> >> +++ usr.sbin/httpd/parse.y 26 Sep 2020 06:03:52 -
> >> @@ -581,7 +581,8 @@ serveroptsl: LISTEN ON STRING opttls po
> >>  
> >>TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
> >>if ((s->srv_conf.flags & SRVFLAG_LOCATION) &&
> >> -  s->srv_conf.id == srv_conf->id &&
> >> +  s->srv_conf.parent_id ==
> >> +  srv_conf->parent_id &&
> >>strcmp(s->srv_conf.location,
> >>

Re: httpd(8): fix location duplicate detection

2020-10-26 Thread mpfr
Ping. Latest diff below.

Index: usr.sbin/httpd/parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.118
diff -u -p -u -p -r1.118 parse.y
--- usr.sbin/httpd/parse.y  11 Oct 2020 03:21:44 -  1.118
+++ usr.sbin/httpd/parse.y  26 Oct 2020 08:26:48 -
@@ -587,8 +587,10 @@ serveroptsl: LISTEN ON STRING opttls po
struct server   *s = NULL;
 
TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
+   /* Compare locations of same parent server */
if ((s->srv_conf.flags & SRVFLAG_LOCATION) &&
-   s->srv_conf.id == srv_conf->id &&
+   s->srv_conf.parent_id ==
+   srv_conf->parent_id &&
strcmp(s->srv_conf.location,
srv_conf->location) == 0)
break;


On 2020-10-11 12:00, m...@fn.de wrote:
> Ping. Updated diff below.
> 
> ---
> Index: usr.sbin/httpd/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> retrieving revision 1.118
> diff -u -p -u -p -r1.118 parse.y
> --- usr.sbin/httpd/parse.y11 Oct 2020 03:21:44 -  1.118
> +++ usr.sbin/httpd/parse.y11 Oct 2020 09:52:34 -
> @@ -588,7 +588,8 @@ serveroptsl   : LISTEN ON STRING opttls po
>  
>   TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
>   if ((s->srv_conf.flags & SRVFLAG_LOCATION) &&
> - s->srv_conf.id == srv_conf->id &&
> + s->srv_conf.parent_id ==
> + srv_conf->parent_id &&
>   strcmp(s->srv_conf.location,
>   srv_conf->location) == 0)
>   break;
> ---
> 
> On 2020-09-26 08:57, m...@fn.de wrote:
>> During httpd setup I realized that duplicate location names are not
>> being detected even though I remembered having seen a corresponding
>> piece of code in 'usr.sbin/httpd/parse.y' the other day.  As far
>> as I understand, the comparison 's->srv_conf.id == srv_conf->id'
>> can never be true as a newly created location ID would never match
>> the ID of any existing location.
>>
>> To check whether or not I was right, I recompiled httpd with DEBUG
>> enabled and tried to start the server with the following (actually
>> invalid) httpd.conf:
>>
>> 
>> server "testserver" {
>>  listen on 127.0.0.1 port www
>>  location "/foo" { block }
>>  location "/foo" { block }
>> }
>> 
>>
>> # httpd -vvd
>> startup
>> adding location "/foo" for "testserver[2]"
>> adding location "/foo" for "testserver[3]"
>> adding server "testserver[1]"
>> 
>> (httpd running)
>>
>> I guess the intention was to compare the new location name with all
>> other location names available under the same parent server.  I
>> accomplished this by applying the patch at the bottom of this
>> message.  After recompiling, httpd startup terminates as expected.
>>
>> # httpd -vvd
>> startup
>> adding location "/foo" for "testserver[2]"
>> /etc/httpd.conf:4: location "/foo" defined twice
>> .
>> logger exiting, pid 98967
>> server exiting, pid 27723
>> server exiting, pid 78507
>> server exiting, pid 25743
>>
>>
>> comments? OK?
>>
>> ---
>>
>> Index: usr.sbin/httpd/parse.y
>> ===
>> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
>> retrieving revision 1.117
>> diff -u -p -u -p -r1.117 parse.y
>> --- usr.sbin/httpd/parse.y   26 Aug 2020 06:50:20 -  1.117
>> +++ usr.sbin/httpd/parse.y   26 Sep 2020 06:03:52 -
>> @@ -581,7 +581,8 @@ serveroptsl  : LISTEN ON STRING opttls po
>>  
>>  TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
>>  if ((s->srv_conf.flags & SRVFLAG_LOCATION) &&
>> -s->srv_conf.id == srv_conf->id &&
>> +s->srv_conf.parent_id ==
>> +srv_conf->parent_id &&
>>  strcmp(s->srv_conf.location,
>>  srv_conf->location) == 0)
>>  break;
>>
> 



Re: httpd(8): fix location duplicate detection

2020-10-11 Thread mpfr
Ping. Updated diff below.

---
Index: usr.sbin/httpd/parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.118
diff -u -p -u -p -r1.118 parse.y
--- usr.sbin/httpd/parse.y  11 Oct 2020 03:21:44 -  1.118
+++ usr.sbin/httpd/parse.y  11 Oct 2020 09:52:34 -
@@ -588,7 +588,8 @@ serveroptsl : LISTEN ON STRING opttls po
 
TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
if ((s->srv_conf.flags & SRVFLAG_LOCATION) &&
-   s->srv_conf.id == srv_conf->id &&
+   s->srv_conf.parent_id ==
+   srv_conf->parent_id &&
strcmp(s->srv_conf.location,
srv_conf->location) == 0)
break;
---

On 2020-09-26 08:57, m...@fn.de wrote:
> During httpd setup I realized that duplicate location names are not
> being detected even though I remembered having seen a corresponding
> piece of code in 'usr.sbin/httpd/parse.y' the other day.  As far
> as I understand, the comparison 's->srv_conf.id == srv_conf->id'
> can never be true as a newly created location ID would never match
> the ID of any existing location.
> 
> To check whether or not I was right, I recompiled httpd with DEBUG
> enabled and tried to start the server with the following (actually
> invalid) httpd.conf:
> 
> 
> server "testserver" {
>  listen on 127.0.0.1 port www
>  location "/foo" { block }
>  location "/foo" { block }
> }
> 
> 
> # httpd -vvd
> startup
> adding location "/foo" for "testserver[2]"
> adding location "/foo" for "testserver[3]"
> adding server "testserver[1]"
> 
> (httpd running)
> 
> I guess the intention was to compare the new location name with all
> other location names available under the same parent server.  I
> accomplished this by applying the patch at the bottom of this
> message.  After recompiling, httpd startup terminates as expected.
> 
> # httpd -vvd
> startup
> adding location "/foo" for "testserver[2]"
> /etc/httpd.conf:4: location "/foo" defined twice
> .
> logger exiting, pid 98967
> server exiting, pid 27723
> server exiting, pid 78507
> server exiting, pid 25743
> 
> 
> comments? OK?
> 
> ---
> 
> Index: usr.sbin/httpd/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> retrieving revision 1.117
> diff -u -p -u -p -r1.117 parse.y
> --- usr.sbin/httpd/parse.y26 Aug 2020 06:50:20 -  1.117
> +++ usr.sbin/httpd/parse.y26 Sep 2020 06:03:52 -
> @@ -581,7 +581,8 @@ serveroptsl   : LISTEN ON STRING opttls po
>  
>   TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
>   if ((s->srv_conf.flags & SRVFLAG_LOCATION) &&
> - s->srv_conf.id == srv_conf->id &&
> + s->srv_conf.parent_id ==
> + srv_conf->parent_id &&
>   strcmp(s->srv_conf.location,
>   srv_conf->location) == 0)
>   break;
>