You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2016/07/18 15:22:02 UTC

Re: svn commit: r1753223 - /httpd/httpd/trunk/modules/http/http_protocol.c


On 07/18/2016 03:41 PM, wrowe@apache.org wrote:
> Author: wrowe
> Date: Mon Jul 18 13:41:26 2016
> New Revision: 1753223
> 
> URL: http://svn.apache.org/viewvc?rev=1753223&view=rev
> Log:
> Simplify; this code is executed one per request processed, saving 
> an immeasurably small quantum of CPU of a server under load.
> 
> Modified:
>     httpd/httpd/trunk/modules/http/http_protocol.c
> 
> Modified: httpd/httpd/trunk/modules/http/http_protocol.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_protocol.c?rev=1753223&r1=1753222&r2=1753223&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/http/http_protocol.c (original)
> +++ httpd/httpd/trunk/modules/http/http_protocol.c Mon Jul 18 13:41:26 2016
> @@ -754,193 +754,6 @@ AP_DECLARE(int) ap_method_register(apr_p
>      return cur_method_number++;
>  }
>  
> -#define UNKNOWN_METHOD (-1)
> -
> -static int lookup_builtin_method(const char *method, apr_size_t len)
> -{
> -    /* Note: the following code was generated by the "shilka" tool from
> -       the "cocom" parsing/compilation toolkit. It is an optimized lookup
> -       based on analysis of the input keywords. Postprocessing was done
> -       on the shilka output, but the basic structure and analysis is
> -       from there. Should new HTTP methods be added, then manual insertion
> -       into this code is fine, or simply re-running the shilka tool on
> -       the appropriate input. */
> -
> -    /* Note: it is also quite reasonable to just use our method_registry,
> -       but I'm assuming (probably incorrectly) we want more speed here
> -       (based on the optimizations the previous code was doing). */
> -
> -    switch (len)
> -    {
> -    case 3:
> -        switch (method[0])
> -        {
> -        case 'P':
> -            return (method[1] == 'U'
> -                    && method[2] == 'T'
> -                    ? M_PUT : UNKNOWN_METHOD);
> -        case 'G':
> -            return (method[1] == 'E'
> -                    && method[2] == 'T'
> -                    ? M_GET : UNKNOWN_METHOD);
> -        default:
> -            return UNKNOWN_METHOD;
> -        }
> -
> -    case 4:
> -        switch (method[0])
> -        {
> -        case 'H':
> -            return (method[1] == 'E'
> -                    && method[2] == 'A'
> -                    && method[3] == 'D'
> -                    ? M_GET : UNKNOWN_METHOD);
> -        case 'P':
> -            return (method[1] == 'O'
> -                    && method[2] == 'S'
> -                    && method[3] == 'T'
> -                    ? M_POST : UNKNOWN_METHOD);
> -        case 'M':
> -            return (method[1] == 'O'
> -                    && method[2] == 'V'
> -                    && method[3] == 'E'
> -                    ? M_MOVE : UNKNOWN_METHOD);
> -        case 'L':
> -            return (method[1] == 'O'
> -                    && method[2] == 'C'
> -                    && method[3] == 'K'
> -                    ? M_LOCK : UNKNOWN_METHOD);
> -        case 'C':
> -            return (method[1] == 'O'
> -                    && method[2] == 'P'
> -                    && method[3] == 'Y'
> -                    ? M_COPY : UNKNOWN_METHOD);
> -        case 'B':
> -            return (method[1] == 'R'
> -                    && method[2] == 'E'
> -                    && method[3] == 'W'
> -                    ? M_BREW : UNKNOWN_METHOD);
> -        case 'W':
> -            return (method[1] == 'H'
> -                    && method[2] == 'E'
> -                    && method[3] == 'N'
> -                    ? M_WHEN : UNKNOWN_METHOD);
> -        default:
> -            return UNKNOWN_METHOD;
> -        }
> -
> -    case 5:
> -        switch (method[2])
> -        {
> -        case 'T':
> -            return (memcmp(method, "PATCH", 5) == 0
> -                    ? M_PATCH : UNKNOWN_METHOD);
> -        case 'R':
> -            return (memcmp(method, "MERGE", 5) == 0
> -                    ? M_MERGE : UNKNOWN_METHOD);
> -        case 'C':
> -            return (memcmp(method, "MKCOL", 5) == 0
> -                    ? M_MKCOL : UNKNOWN_METHOD);
> -        case 'B':
> -            return (memcmp(method, "LABEL", 5) == 0
> -                    ? M_LABEL : UNKNOWN_METHOD);
> -        case 'A':
> -            return (memcmp(method, "TRACE", 5) == 0
> -                    ? M_TRACE : UNKNOWN_METHOD);
> -        default:
> -            return UNKNOWN_METHOD;
> -        }
> -
> -    case 6:
> -        switch (method[0])
> -        {
> -        case 'U':
> -            switch (method[5])
> -            {
> -            case 'K':
> -                return (memcmp(method, "UNLOCK", 6) == 0
> -                        ? M_UNLOCK : UNKNOWN_METHOD);
> -            case 'E':
> -                return (memcmp(method, "UPDATE", 6) == 0
> -                        ? M_UPDATE : UNKNOWN_METHOD);
> -            default:
> -                return UNKNOWN_METHOD;
> -            }
> -        case 'R':
> -            return (memcmp(method, "REPORT", 6) == 0
> -                    ? M_REPORT : UNKNOWN_METHOD);
> -        case 'D':
> -            return (memcmp(method, "DELETE", 6) == 0
> -                    ? M_DELETE : UNKNOWN_METHOD);
> -        default:
> -            return UNKNOWN_METHOD;
> -        }
> -
> -    case 7:
> -        switch (method[1])
> -        {
> -        case 'P':
> -            return (memcmp(method, "OPTIONS", 7) == 0
> -                    ? M_OPTIONS : UNKNOWN_METHOD);
> -        case 'O':
> -            return (memcmp(method, "CONNECT", 7) == 0
> -                    ? M_CONNECT : UNKNOWN_METHOD);
> -        case 'H':
> -            return (memcmp(method, "CHECKIN", 7) == 0
> -                    ? M_CHECKIN : UNKNOWN_METHOD);
> -        default:
> -            return UNKNOWN_METHOD;
> -        }
> -
> -    case 8:
> -        switch (method[0])
> -        {
> -        case 'P':
> -            return (memcmp(method, "PROPFIND", 8) == 0
> -                    ? M_PROPFIND : UNKNOWN_METHOD);
> -        case 'C':
> -            return (memcmp(method, "CHECKOUT", 8) == 0
> -                    ? M_CHECKOUT : UNKNOWN_METHOD);
> -        default:
> -            return UNKNOWN_METHOD;
> -        }
> -
> -    case 9:
> -        return (memcmp(method, "PROPPATCH", 9) == 0
> -                ? M_PROPPATCH : UNKNOWN_METHOD);
> -
> -    case 10:
> -        switch (method[0])
> -        {
> -        case 'U':
> -            return (memcmp(method, "UNCHECKOUT", 10) == 0
> -                    ? M_UNCHECKOUT : UNKNOWN_METHOD);
> -        case 'M':
> -            return (memcmp(method, "MKACTIVITY", 10) == 0
> -                    ? M_MKACTIVITY : UNKNOWN_METHOD);
> -        default:
> -            return UNKNOWN_METHOD;
> -        }
> -
> -    case 11:
> -        return (memcmp(method, "MKWORKSPACE", 11) == 0
> -                ? M_MKWORKSPACE : UNKNOWN_METHOD);
> -
> -    case 15:
> -        return (memcmp(method, "VERSION-CONTROL", 15) == 0
> -                ? M_VERSION_CONTROL : UNKNOWN_METHOD);
> -
> -    case 16:
> -        return (memcmp(method, "BASELINE-CONTROL", 16) == 0
> -                ? M_BASELINE_CONTROL : UNKNOWN_METHOD);
> -
> -    default:
> -        return UNKNOWN_METHOD;
> -    }
> -
> -    /* NOTREACHED */
> -}
> -
>  /* Get the method number associated with the given string, assumed to
>   * contain an HTTP method.  Returns M_INVALID if not recognized.
>   *
> @@ -951,18 +764,12 @@ static int lookup_builtin_method(const c
>  AP_DECLARE(int) ap_method_number_of(const char *method)
>  {
>      int len = strlen(method);
> -    int which = lookup_builtin_method(method, len);
> -
> -    if (which != UNKNOWN_METHOD)
> -        return which;
>  
>      /* check if the method has been dynamically registered */
> -    if (methods_registry != NULL) {
> -        int *methnum = apr_hash_get(methods_registry, method, len);
> +    int *methnum = apr_hash_get(methods_registry, method, len);

How do we ensure that methods_registry is not NULL or better that
ap_method_registry_init was called before?

Regards

R�diger


Re: svn commit: r1753223 - /httpd/httpd/trunk/modules/http/http_protocol.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
[Mon Jul 18 15:57:00.513202 2016] [core:error] [pid 6239:tid 123145317863424] [client 127.0.0.1:60655] AH00135: Invalid method in request HEAD /apache/etags/none/plus-mis/minus-ms/test.txt HTTP/1.1

> On Jul 18, 2016, at 11:57 AM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> 
> Investigating, should have it clearer w/in 1/2 an hr
> 
> 
> On Jul 18, 2016 10:54 AM, "Jim Jagielski" <ji...@jagunet.com> wrote:
> I am getting NUMEROUS errors on trunk... HEAD is returning 501. The
> only test where this does NOT occur is:
> 
>     "HEAD /modules/cgi/perl.pl HTTP/1.1" 200 8
> 
> Looks suspicious to me...
> 
> 
> > On Jul 18, 2016, at 11:44 AM, Rüdiger Plüm <r....@gmx.de> wrote:
> >
> >
> >
> > On 07/18/2016 05:28 PM, William A Rowe Jr wrote:
> >> On Mon, Jul 18, 2016 at 10:22 AM, Ruediger Pluem <rpluem@apache.org <ma...@apache.org>> wrote:
> >>
> >>
> >>    On 07/18/2016 03:41 PM, wrowe@apache.org <ma...@apache.org> wrote:
> >>> Author: wrowe
> >>> Date: Mon Jul 18 13:41:26 2016
> >>> New Revision: 1753223
> >>>
> >>> URL: http://svn.apache.org/viewvc?rev=1753223&view=rev
> >>> Log:
> >>> Simplify; this code is executed one per request processed, saving
> >>> an immeasurably small quantum of CPU of a server under load.
> >>>
> >>> +    int *methnum = apr_hash_get(methods_registry, method, len);
> >>
> >>    How do we ensure that methods_registry is not NULL or better that
> >>    ap_method_registry_init was called before?
> >>
> >>
> >> Is the ap_method_registry_init in mod_http register_hooks() insufficient?
> >>
> >>
> >>
> >
> > Doh. I missed that. Sorry for the noise.
> >
> > Regards
> >
> > Rüdiger
> 


Re: svn commit: r1753223 - /httpd/httpd/trunk/modules/http/http_protocol.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
Beat me to the patch... :)

> On Jul 18, 2016, at 12:02 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> 
> On Mon, Jul 18, 2016 at 11:00 AM, Jim Jagielski <ji...@jagunet.com> wrote:
> Hrm. ap_method_registry_init lacks HEAD
> 
> And has no M_HEAD, it's M_GET. Resolved, reviewing the zany bytewise
> logic for any other missing identifiers.
> 
> Thanks for the catch. 
> 


Re: svn commit: r1753223 - /httpd/httpd/trunk/modules/http/http_protocol.c

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Jul 18, 2016 at 11:00 AM, Jim Jagielski <ji...@jagunet.com> wrote:

> Hrm. ap_method_registry_init lacks HEAD


And has no M_HEAD, it's M_GET. Resolved, reviewing the zany bytewise
logic for any other missing identifiers.

Thanks for the catch.

Re: svn commit: r1753223 - /httpd/httpd/trunk/modules/http/http_protocol.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
Hrm. ap_method_registry_init lacks HEAD.

> On Jul 18, 2016, at 11:57 AM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> 
> Investigating, should have it clearer w/in 1/2 an hr
> 
> 
> On Jul 18, 2016 10:54 AM, "Jim Jagielski" <ji...@jagunet.com> wrote:
> I am getting NUMEROUS errors on trunk... HEAD is returning 501. The
> only test where this does NOT occur is:
> 
>     "HEAD /modules/cgi/perl.pl HTTP/1.1" 200 8
> 
> Looks suspicious to me...
> 
> 
> > On Jul 18, 2016, at 11:44 AM, Rüdiger Plüm <r....@gmx.de> wrote:
> >
> >
> >
> > On 07/18/2016 05:28 PM, William A Rowe Jr wrote:
> >> On Mon, Jul 18, 2016 at 10:22 AM, Ruediger Pluem <rpluem@apache.org <ma...@apache.org>> wrote:
> >>
> >>
> >>    On 07/18/2016 03:41 PM, wrowe@apache.org <ma...@apache.org> wrote:
> >>> Author: wrowe
> >>> Date: Mon Jul 18 13:41:26 2016
> >>> New Revision: 1753223
> >>>
> >>> URL: http://svn.apache.org/viewvc?rev=1753223&view=rev
> >>> Log:
> >>> Simplify; this code is executed one per request processed, saving
> >>> an immeasurably small quantum of CPU of a server under load.
> >>>
> >>> +    int *methnum = apr_hash_get(methods_registry, method, len);
> >>
> >>    How do we ensure that methods_registry is not NULL or better that
> >>    ap_method_registry_init was called before?
> >>
> >>
> >> Is the ap_method_registry_init in mod_http register_hooks() insufficient?
> >>
> >>
> >>
> >
> > Doh. I missed that. Sorry for the noise.
> >
> > Regards
> >
> > Rüdiger
> 


Re: svn commit: r1753223 - /httpd/httpd/trunk/modules/http/http_protocol.c

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
Investigating, should have it clearer w/in 1/2 an hr

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

> I am getting NUMEROUS errors on trunk... HEAD is returning 501. The
> only test where this does NOT occur is:
>
>     "HEAD /modules/cgi/perl.pl HTTP/1.1" 200 8
>
> Looks suspicious to me...
>
>
> > On Jul 18, 2016, at 11:44 AM, Rüdiger Plüm <r....@gmx.de> wrote:
> >
> >
> >
> > On 07/18/2016 05:28 PM, William A Rowe Jr wrote:
> >> On Mon, Jul 18, 2016 at 10:22 AM, Ruediger Pluem <rpluem@apache.org
> <ma...@apache.org>> wrote:
> >>
> >>
> >>    On 07/18/2016 03:41 PM, wrowe@apache.org <ma...@apache.org>
> wrote:
> >>> Author: wrowe
> >>> Date: Mon Jul 18 13:41:26 2016
> >>> New Revision: 1753223
> >>>
> >>> URL: http://svn.apache.org/viewvc?rev=1753223&view=rev
> >>> Log:
> >>> Simplify; this code is executed one per request processed, saving
> >>> an immeasurably small quantum of CPU of a server under load.
> >>>
> >>> +    int *methnum = apr_hash_get(methods_registry, method, len);
> >>
> >>    How do we ensure that methods_registry is not NULL or better that
> >>    ap_method_registry_init was called before?
> >>
> >>
> >> Is the ap_method_registry_init in mod_http register_hooks()
> insufficient?
> >>
> >>
> >>
> >
> > Doh. I missed that. Sorry for the noise.
> >
> > Regards
> >
> > Rüdiger
>
>

Re: svn commit: r1753223 - /httpd/httpd/trunk/modules/http/http_protocol.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
I am getting NUMEROUS errors on trunk... HEAD is returning 501. The
only test where this does NOT occur is:

    "HEAD /modules/cgi/perl.pl HTTP/1.1" 200 8

Looks suspicious to me...


> On Jul 18, 2016, at 11:44 AM, Rüdiger Plüm <r....@gmx.de> wrote:
> 
> 
> 
> On 07/18/2016 05:28 PM, William A Rowe Jr wrote:
>> On Mon, Jul 18, 2016 at 10:22 AM, Ruediger Pluem <rpluem@apache.org <ma...@apache.org>> wrote:
>> 
>> 
>>    On 07/18/2016 03:41 PM, wrowe@apache.org <ma...@apache.org> wrote:
>>> Author: wrowe
>>> Date: Mon Jul 18 13:41:26 2016
>>> New Revision: 1753223
>>> 
>>> URL: http://svn.apache.org/viewvc?rev=1753223&view=rev
>>> Log:
>>> Simplify; this code is executed one per request processed, saving
>>> an immeasurably small quantum of CPU of a server under load.
>>> 
>>> +    int *methnum = apr_hash_get(methods_registry, method, len);
>> 
>>    How do we ensure that methods_registry is not NULL or better that
>>    ap_method_registry_init was called before?
>> 
>> 
>> Is the ap_method_registry_init in mod_http register_hooks() insufficient?
>> 
>> 
>> 
> 
> Doh. I missed that. Sorry for the noise.
> 
> Regards
> 
> Rüdiger


Re: svn commit: r1753223 - /httpd/httpd/trunk/modules/http/http_protocol.c

Posted by Rüdiger Plüm <r....@gmx.de>.

On 07/18/2016 05:28 PM, William A Rowe Jr wrote:
> On Mon, Jul 18, 2016 at 10:22 AM, Ruediger Pluem <rpluem@apache.org <ma...@apache.org>> wrote:
> 
> 
>     On 07/18/2016 03:41 PM, wrowe@apache.org <ma...@apache.org> wrote:
>     > Author: wrowe
>     > Date: Mon Jul 18 13:41:26 2016
>     > New Revision: 1753223
>     >
>     > URL: http://svn.apache.org/viewvc?rev=1753223&view=rev
>     > Log:
>     > Simplify; this code is executed one per request processed, saving
>     > an immeasurably small quantum of CPU of a server under load.
>     >
>     > +    int *methnum = apr_hash_get(methods_registry, method, len);
> 
>     How do we ensure that methods_registry is not NULL or better that
>     ap_method_registry_init was called before?
> 
>  
> Is the ap_method_registry_init in mod_http register_hooks() insufficient?
> 
> 
> 

Doh. I missed that. Sorry for the noise.

Regards

Rüdiger

Re: svn commit: r1753223 - /httpd/httpd/trunk/modules/http/http_protocol.c

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Jul 18, 2016 at 10:22 AM, Ruediger Pluem <rp...@apache.org> wrote:

>
> On 07/18/2016 03:41 PM, wrowe@apache.org wrote:
> > Author: wrowe
> > Date: Mon Jul 18 13:41:26 2016
> > New Revision: 1753223
> >
> > URL: http://svn.apache.org/viewvc?rev=1753223&view=rev
> > Log:
> > Simplify; this code is executed one per request processed, saving
> > an immeasurably small quantum of CPU of a server under load.
> >
> > +    int *methnum = apr_hash_get(methods_registry, method, len);
>
> How do we ensure that methods_registry is not NULL or better that
> ap_method_registry_init was called before?
>

Is the ap_method_registry_init in mod_http register_hooks() insufficient?