You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by wr...@apache.org on 2016/07/18 16:20:27 UTC

svn commit: r1753263 - in /httpd/httpd/trunk: include/httpd.h modules/http/http_protocol.c

Author: wrowe
Date: Mon Jul 18 16:20:27 2016
New Revision: 1753263

URL: http://svn.apache.org/viewvc?rev=1753263&view=rev
Log:
A whole lotta nope, if you implement HTCPCP then register your methods in init

Modified:
    httpd/httpd/trunk/include/httpd.h
    httpd/httpd/trunk/modules/http/http_protocol.c

Modified: httpd/httpd/trunk/include/httpd.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/httpd.h?rev=1753263&r1=1753262&r2=1753263&view=diff
==============================================================================
--- httpd/httpd/trunk/include/httpd.h (original)
+++ httpd/httpd/trunk/include/httpd.h Mon Jul 18 16:20:27 2016
@@ -612,9 +612,15 @@ AP_DECLARE(const char *) ap_get_server_b
 #define M_MKACTIVITY            23
 #define M_BASELINE_CONTROL      24
 #define M_MERGE                 25
-#define M_INVALID               26      /** no valid method */
-#define M_BREW                  27      /** RFC 2324: HTCPCP/1.0 */
-#define M_WHEN                  28      /** RFC 2324: HTCPCP/1.0 */
+/* Additional methods must be registered by the implementor, we have only
+ * room for 64 bit-wise methods available, so do not squander them (more of
+ * the above methods should probably move here)
+ */
+/* #define M_BREW                  nn */     /** RFC 2324: HTCPCP/1.0 */
+/* #define M_WHEN                  nn */     /** RFC 2324: HTCPCP/1.0 */
+#define M_INVALID               26      /** invalid method value terminates the
+                                         *  listed ap_method_registry_init()
+                                         */
 
 /**
  * METHODS needs to be equal to the number of bits

Modified: httpd/httpd/trunk/modules/http/http_protocol.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_protocol.c?rev=1753263&r1=1753262&r2=1753263&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http/http_protocol.c (original)
+++ httpd/httpd/trunk/modules/http/http_protocol.c Mon Jul 18 16:20:27 2016
@@ -718,8 +718,6 @@ AP_DECLARE(void) ap_method_registry_init
     register_one_method(p, "MKACTIVITY", M_MKACTIVITY);
     register_one_method(p, "BASELINE-CONTROL", M_BASELINE_CONTROL);
     register_one_method(p, "MERGE", M_MERGE);
-    register_one_method(p, "BREW", M_BREW);
-    register_one_method(p, "WHEN", M_WHEN);
 }
 
 AP_DECLARE(int) ap_method_register(apr_pool_t *p, const char *methname)



Re: svn commit: r1753263 - in /httpd/httpd/trunk: include/httpd.h modules/http/http_protocol.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
+1. Plan for the future.

> On Jul 18, 2016, at 12:32 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> 
> Worse, in http 2.4, the first two registered methods collide with BREW and WHEN. That said, the 'fix', if we wanted to resolve it, is to use M_INVALID +3 as the first value.
> 
> I suggest on trunk we use a value outside the bitmask range of 0-63 as INVALID and consider turning this into an array of 128 bits. mod_ftp, for examples, implents many more non-http methods and relies on the method auth logic.
> 
> 
> On Jul 18, 2016 11:25 AM, "Jim Jagielski" <ji...@jagunet.com> wrote:
> Doesn't this make this unsuitable for backport for 2.4?
> 
> > On Jul 18, 2016, at 12:20 PM, wrowe@apache.org wrote:
> >
> > Author: wrowe
> > Date: Mon Jul 18 16:20:27 2016
> > New Revision: 1753263
> >
> > URL: http://svn.apache.org/viewvc?rev=1753263&view=rev
> > Log:
> > A whole lotta nope, if you implement HTCPCP then register your methods in init
> >
> > Modified:
> >    httpd/httpd/trunk/include/httpd.h
> >    httpd/httpd/trunk/modules/http/http_protocol.c
> >
> > Modified: httpd/httpd/trunk/include/httpd.h
> > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/httpd.h?rev=1753263&r1=1753262&r2=1753263&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/include/httpd.h (original)
> > +++ httpd/httpd/trunk/include/httpd.h Mon Jul 18 16:20:27 2016
> > @@ -612,9 +612,15 @@ AP_DECLARE(const char *) ap_get_server_b
> > #define M_MKACTIVITY            23
> > #define M_BASELINE_CONTROL      24
> > #define M_MERGE                 25
> > -#define M_INVALID               26      /** no valid method */
> > -#define M_BREW                  27      /** RFC 2324: HTCPCP/1.0 */
> > -#define M_WHEN                  28      /** RFC 2324: HTCPCP/1.0 */
> > +/* Additional methods must be registered by the implementor, we have only
> > + * room for 64 bit-wise methods available, so do not squander them (more of
> > + * the above methods should probably move here)
> > + */
> > +/* #define M_BREW                  nn */     /** RFC 2324: HTCPCP/1.0 */
> > +/* #define M_WHEN                  nn */     /** RFC 2324: HTCPCP/1.0 */
> > +#define M_INVALID               26      /** invalid method value terminates the
> > +                                         *  listed ap_method_registry_init()
> > +                                         */
> >
> > /**
> >  * METHODS needs to be equal to the number of bits
> >
> > Modified: httpd/httpd/trunk/modules/http/http_protocol.c
> > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_protocol.c?rev=1753263&r1=1753262&r2=1753263&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/modules/http/http_protocol.c (original)
> > +++ httpd/httpd/trunk/modules/http/http_protocol.c Mon Jul 18 16:20:27 2016
> > @@ -718,8 +718,6 @@ AP_DECLARE(void) ap_method_registry_init
> >     register_one_method(p, "MKACTIVITY", M_MKACTIVITY);
> >     register_one_method(p, "BASELINE-CONTROL", M_BASELINE_CONTROL);
> >     register_one_method(p, "MERGE", M_MERGE);
> > -    register_one_method(p, "BREW", M_BREW);
> > -    register_one_method(p, "WHEN", M_WHEN);
> > }
> >
> > AP_DECLARE(int) ap_method_register(apr_pool_t *p, const char *methname)
> >
> >
> 


Re: svn commit: r1753263 - in /httpd/httpd/trunk: include/httpd.h modules/http/http_protocol.c

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Wed, Aug 3, 2016 at 12:19 PM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> On Wed, Aug 3, 2016 at 8:13 AM, Graham Leggett <mi...@sharp.fm> wrote:
>
>> On 18 Jul 2016, at 6:32 PM, William A Rowe Jr <wr...@rowe-clan.net>
>> wrote:
>>
>> > Worse, in http 2.4, the first two registered methods collide with BREW
>> and WHEN. That said, the 'fix', if we wanted to resolve it, is to use
>> M_INVALID +3 as the first value.
>>
>> I’m not seeing the problem here, there is no expectation of binary
>> compatibility between httpd v2.4 and trunk. Can you confirm what you’re
>> trying to achieve by removing this?
>>
>
> The first problem in dropping BREW and WHEN is that any provider against
> 2.6/3.0 can register these methods itself. The ftp module, for example,
> registers some 50 commands. These exceed the number of AllowMethod
> limitable methods. The right answer for future-proofing would be to either
> have providers register only extension methods they want to use, and/or
> increase that from a 64-bit word to 128-bit index of allow methods (say
> 16 chars for simplicity's sake).
>
> The problem here is that BREW would collide with the first registered
> method,
> and WHEN would collide with the second registered method, wherein two
> method
> identifiers would share the same ID and same AllowMethod rule. That's bad
> (if these two were implemented by a provider).
>
> httpd 2.4 requires binary compatibility, if we don't want these to
> collide, we must
> assign the first registered method to 1+WHEN. The IDs can't change on
> 2.4.x,
> but the first number allocated for a registered method certainly can.
>
> In all, this was a cleanup-[de?]optimization of convoluted trunk logic, I
> don't see
> the point in backporting such noise to a maintenance branch. Only trunk,
> moving forwards, aught to be 'optimal' in terms of legibility and
> performance.
>
> So it wasn't my intent to backport. If we want to avoid BREW/WHEN
> collisions
> we should simply fix that.
>

Never mind, BREW and WHEN were simply not registered at all in 2.4/2.2.

I am going to suggest we register the hash lookup value for HEAD though as
a one-line patch to 2.2/2.4, aliased to M_GET just as on trunk.

Re: svn commit: r1753263 - in /httpd/httpd/trunk: include/httpd.h modules/http/http_protocol.c

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Wed, Aug 3, 2016 at 8:13 AM, Graham Leggett <mi...@sharp.fm> wrote:

> On 18 Jul 2016, at 6:32 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
>
> > Worse, in http 2.4, the first two registered methods collide with BREW
> and WHEN. That said, the 'fix', if we wanted to resolve it, is to use
> M_INVALID +3 as the first value.
>
> I’m not seeing the problem here, there is no expectation of binary
> compatibility between httpd v2.4 and trunk. Can you confirm what you’re
> trying to achieve by removing this?
>

The first problem in dropping BREW and WHEN is that any provider against
2.6/3.0 can register these methods itself. The ftp module, for example,
registers some 50 commands. These exceed the number of AllowMethod
limitable methods. The right answer for future-proofing would be to either
have providers register only extension methods they want to use, and/or
increase that from a 64-bit word to 128-bit index of allow methods (say
16 chars for simplicity's sake).

The problem here is that BREW would collide with the first registered
method,
and WHEN would collide with the second registered method, wherein two method
identifiers would share the same ID and same AllowMethod rule. That's bad
(if these two were implemented by a provider).

httpd 2.4 requires binary compatibility, if we don't want these to collide,
we must
assign the first registered method to 1+WHEN. The IDs can't change on 2.4.x,
but the first number allocated for a registered method certainly can.

In all, this was a cleanup-[de?]optimization of convoluted trunk logic, I
don't see
the point in backporting such noise to a maintenance branch. Only trunk,
moving forwards, aught to be 'optimal' in terms of legibility and
performance.

So it wasn't my intent to backport. If we want to avoid BREW/WHEN collisions
we should simply fix that.

Re: svn commit: r1753263 - in /httpd/httpd/trunk: include/httpd.h modules/http/http_protocol.c

Posted by Graham Leggett <mi...@sharp.fm>.
On 18 Jul 2016, at 6:32 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:

> Worse, in http 2.4, the first two registered methods collide with BREW and WHEN. That said, the 'fix', if we wanted to resolve it, is to use M_INVALID +3 as the first value.

I’m not seeing the problem here, there is no expectation of binary compatibility between httpd v2.4 and trunk. Can you confirm what you’re trying to achieve by removing this?

Regards,
Graham
—


Re: svn commit: r1753263 - in /httpd/httpd/trunk: include/httpd.h modules/http/http_protocol.c

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
Worse, in http 2.4, the first two registered methods collide with BREW and
WHEN. That said, the 'fix', if we wanted to resolve it, is to use M_INVALID
+3 as the first value.

I suggest on trunk we use a value outside the bitmask range of 0-63 as
INVALID and consider turning this into an array of 128 bits. mod_ftp, for
examples, implents many more non-http methods and relies on the method auth
logic.

On Jul 18, 2016 11:25 AM, "Jim Jagielski" <ji...@jagunet.com> wrote:

> Doesn't this make this unsuitable for backport for 2.4?
>
> > On Jul 18, 2016, at 12:20 PM, wrowe@apache.org wrote:
> >
> > Author: wrowe
> > Date: Mon Jul 18 16:20:27 2016
> > New Revision: 1753263
> >
> > URL: http://svn.apache.org/viewvc?rev=1753263&view=rev
> > Log:
> > A whole lotta nope, if you implement HTCPCP then register your methods
> in init
> >
> > Modified:
> >    httpd/httpd/trunk/include/httpd.h
> >    httpd/httpd/trunk/modules/http/http_protocol.c
> >
> > Modified: httpd/httpd/trunk/include/httpd.h
> > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/include/httpd.h?rev=1753263&r1=1753262&r2=1753263&view=diff
> >
> ==============================================================================
> > --- httpd/httpd/trunk/include/httpd.h (original)
> > +++ httpd/httpd/trunk/include/httpd.h Mon Jul 18 16:20:27 2016
> > @@ -612,9 +612,15 @@ AP_DECLARE(const char *) ap_get_server_b
> > #define M_MKACTIVITY            23
> > #define M_BASELINE_CONTROL      24
> > #define M_MERGE                 25
> > -#define M_INVALID               26      /** no valid method */
> > -#define M_BREW                  27      /** RFC 2324: HTCPCP/1.0 */
> > -#define M_WHEN                  28      /** RFC 2324: HTCPCP/1.0 */
> > +/* Additional methods must be registered by the implementor, we have
> only
> > + * room for 64 bit-wise methods available, so do not squander them
> (more of
> > + * the above methods should probably move here)
> > + */
> > +/* #define M_BREW                  nn */     /** RFC 2324: HTCPCP/1.0 */
> > +/* #define M_WHEN                  nn */     /** RFC 2324: HTCPCP/1.0 */
> > +#define M_INVALID               26      /** invalid method value
> terminates the
> > +                                         *  listed
> ap_method_registry_init()
> > +                                         */
> >
> > /**
> >  * METHODS needs to be equal to the number of bits
> >
> > Modified: httpd/httpd/trunk/modules/http/http_protocol.c
> > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_protocol.c?rev=1753263&r1=1753262&r2=1753263&view=diff
> >
> ==============================================================================
> > --- httpd/httpd/trunk/modules/http/http_protocol.c (original)
> > +++ httpd/httpd/trunk/modules/http/http_protocol.c Mon Jul 18 16:20:27
> 2016
> > @@ -718,8 +718,6 @@ AP_DECLARE(void) ap_method_registry_init
> >     register_one_method(p, "MKACTIVITY", M_MKACTIVITY);
> >     register_one_method(p, "BASELINE-CONTROL", M_BASELINE_CONTROL);
> >     register_one_method(p, "MERGE", M_MERGE);
> > -    register_one_method(p, "BREW", M_BREW);
> > -    register_one_method(p, "WHEN", M_WHEN);
> > }
> >
> > AP_DECLARE(int) ap_method_register(apr_pool_t *p, const char *methname)
> >
> >
>
>

Re: svn commit: r1753263 - in /httpd/httpd/trunk: include/httpd.h modules/http/http_protocol.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
Doesn't this make this unsuitable for backport for 2.4?

> On Jul 18, 2016, at 12:20 PM, wrowe@apache.org wrote:
> 
> Author: wrowe
> Date: Mon Jul 18 16:20:27 2016
> New Revision: 1753263
> 
> URL: http://svn.apache.org/viewvc?rev=1753263&view=rev
> Log:
> A whole lotta nope, if you implement HTCPCP then register your methods in init
> 
> Modified:
>    httpd/httpd/trunk/include/httpd.h
>    httpd/httpd/trunk/modules/http/http_protocol.c
> 
> Modified: httpd/httpd/trunk/include/httpd.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/httpd.h?rev=1753263&r1=1753262&r2=1753263&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/include/httpd.h (original)
> +++ httpd/httpd/trunk/include/httpd.h Mon Jul 18 16:20:27 2016
> @@ -612,9 +612,15 @@ AP_DECLARE(const char *) ap_get_server_b
> #define M_MKACTIVITY            23
> #define M_BASELINE_CONTROL      24
> #define M_MERGE                 25
> -#define M_INVALID               26      /** no valid method */
> -#define M_BREW                  27      /** RFC 2324: HTCPCP/1.0 */
> -#define M_WHEN                  28      /** RFC 2324: HTCPCP/1.0 */
> +/* Additional methods must be registered by the implementor, we have only
> + * room for 64 bit-wise methods available, so do not squander them (more of
> + * the above methods should probably move here)
> + */
> +/* #define M_BREW                  nn */     /** RFC 2324: HTCPCP/1.0 */
> +/* #define M_WHEN                  nn */     /** RFC 2324: HTCPCP/1.0 */
> +#define M_INVALID               26      /** invalid method value terminates the
> +                                         *  listed ap_method_registry_init()
> +                                         */
> 
> /**
>  * METHODS needs to be equal to the number of bits
> 
> Modified: httpd/httpd/trunk/modules/http/http_protocol.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_protocol.c?rev=1753263&r1=1753262&r2=1753263&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/http/http_protocol.c (original)
> +++ httpd/httpd/trunk/modules/http/http_protocol.c Mon Jul 18 16:20:27 2016
> @@ -718,8 +718,6 @@ AP_DECLARE(void) ap_method_registry_init
>     register_one_method(p, "MKACTIVITY", M_MKACTIVITY);
>     register_one_method(p, "BASELINE-CONTROL", M_BASELINE_CONTROL);
>     register_one_method(p, "MERGE", M_MERGE);
> -    register_one_method(p, "BREW", M_BREW);
> -    register_one_method(p, "WHEN", M_WHEN);
> }
> 
> AP_DECLARE(int) ap_method_register(apr_pool_t *p, const char *methname)
> 
>