Re: svn commit: r1894982 - /httpd/apreq/trunk/library/util.c

2022-08-17 Thread Yann Ylavic
On Wed, Aug 17, 2022 at 3:40 PM Joe Orton  wrote:
>
> On Wed, Aug 17, 2022 at 02:05:09PM +0100, Joe Orton wrote:
> > On Fri, Nov 12, 2021 at 06:12:58PM -, yla...@apache.org wrote:
> > > Author: ylavic
> > > Date: Fri Nov 12 18:12:58 2021
> > > New Revision: 1894982
> > >
> > > URL: http://svn.apache.org/viewvc?rev=1894982=rev
> > > Log:
> > > apreq_header_attribute: Search for the exact attribute name.
> > >
> > > Improve the parsing of the header attributes such that we don't match any
> > > special character before that attribute name (e.g. "(boundary=") or let
> > > forbidden characters unnoticed.
>
> Yann, it appears this change is also breaking the "params" test case in
> the apreq test suite. A test is trying to parse a content-type like
> header:
>
> https://svn.apache.org/viewvc/httpd/apreq/trunk/library/t/params.c?revision=1903492=markup#l100
>
> it fails when reaching the '/' in "text/plain" which is a non-token
> character:
>
> default:
> /* The name is a token */
> if (!IS_TOKEN_CHAR(*hde))
> return APREQ_ERROR_BADCHAR;
>
> Unless this is an invalid use case (the test case implies otherwise)
> this seems like a regression as well?

I fixed it in r1903496 by requiring that the name in a name=value pair
only be a token, with no equal sign the attribute is a value only.


Regards;
Yann.


Re: svn commit: r1894982 - /httpd/apreq/trunk/library/util.c

2022-08-17 Thread Yann Ylavic
On Wed, Aug 17, 2022 at 3:05 PM Joe Orton  wrote:
>
> This is an infinite loop. The libapreq test suite is spinning here,
> "make test" from apreq trunk.

Indeed, should be fixed in r1903495.

Thanks;
Yann.


Re: svn commit: r1894982 - /httpd/apreq/trunk/library/util.c

2022-08-17 Thread Joe Orton
On Wed, Aug 17, 2022 at 02:05:09PM +0100, Joe Orton wrote:
> On Fri, Nov 12, 2021 at 06:12:58PM -, yla...@apache.org wrote:
> > Author: ylavic
> > Date: Fri Nov 12 18:12:58 2021
> > New Revision: 1894982
> > 
> > URL: http://svn.apache.org/viewvc?rev=1894982=rev
> > Log:
> > apreq_header_attribute: Search for the exact attribute name.
> > 
> > Improve the parsing of the header attributes such that we don't match any
> > special character before that attribute name (e.g. "(boundary=") or let
> > forbidden characters unnoticed.

Yann, it appears this change is also breaking the "params" test case in 
the apreq test suite. A test is trying to parse a content-type like 
header:

https://svn.apache.org/viewvc/httpd/apreq/trunk/library/t/params.c?revision=1903492=markup#l100

it fails when reaching the '/' in "text/plain" which is a non-token 
character:

default:
/* The name is a token */
if (!IS_TOKEN_CHAR(*hde))
return APREQ_ERROR_BADCHAR;

Unless this is an invalid use case (the test case implies otherwise) 
this seems like a regression as well?

Regrads, Joe



Re: svn commit: r1894982 - /httpd/apreq/trunk/library/util.c

2022-08-17 Thread Joe Orton
On Fri, Nov 12, 2021 at 06:12:58PM -, yla...@apache.org wrote:
> Author: ylavic
> Date: Fri Nov 12 18:12:58 2021
> New Revision: 1894982
> 
> URL: http://svn.apache.org/viewvc?rev=1894982=rev
> Log:
> apreq_header_attribute: Search for the exact attribute name.
> 
> Improve the parsing of the header attributes such that we don't match any
> special character before that attribute name (e.g. "(boundary=") or let
> forbidden characters unnoticed.

...
> +look_for_after_quote:
> +switch (*v) {
> +case 0:
> +case '\r':
> +case '\n':
> +done = 1;
> +case ';':
> +case ',':
> +break;
> +case ' ':
> +case '\t':
> +goto look_for_after_quote;

This is an infinite loop. The libapreq test suite is spinning here, 
"make test" from apreq trunk.




Re: svn commit: r1854963 - in /httpd/httpd/trunk: ./ modules/http2/

2022-08-17 Thread Ruediger Pluem



On 8/17/22 9:57 AM, Stefan Eissing via dev wrote:
> 
> 
>> Am 17.08.2022 um 09:26 schrieb Ruediger Pluem :
>>
>>
>>
>> On 3/7/19 10:41 AM, ic...@apache.org wrote:
>>> Author: icing
>>> Date: Thu Mar  7 09:41:15 2019
>>> New Revision: 1854963
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1854963=rev
>>> Log:
>>>  *) mod_http2: new configuration directive: ```H2Padding numbits``` to 
>>> control 
>>> padding of HTTP/2 payload frames. 'numbits' is a number from 0-8,
>>> controlling the range of padding bytes added to a frame. The actual 
>>> number
>>> added is chosen randomly per frame. This applies to HEADERS, DATA and 
>>> PUSH_PROMISE
>>> frames equally. The default continues to be 0, e.g. no padding. [Stefan 
>>> Eissing] 
>>>
>>>  *) mod_http2: ripping out all the h2_req_engine internal features now that 
>>> mod_proxy_http2
>>> has no more need for it. Optional functions are still declared but no 
>>> longer implemented.
>>> While previous mod_proxy_http2 will work with this, it is recommeneded 
>>> to run the matching
>>> versions of both modules. [Stefan Eissing]
>>>
>>>  *) mod_proxy_http2: changed mod_proxy_http2 implementation and fixed 
>>> several bugs which
>>> resolve PR63170. The proxy module does now a single h2 request on the 
>>> (reused)
>>> connection and returns. [Stefan Eissing]
>>>
>>>
>>> Removed:
>>>httpd/httpd/trunk/modules/http2/h2_ngn_shed.c
>>>httpd/httpd/trunk/modules/http2/h2_ngn_shed.h
>>> Modified:
>>>httpd/httpd/trunk/CHANGES
>>>httpd/httpd/trunk/modules/http2/config2.m4
>>>httpd/httpd/trunk/modules/http2/h2.h
>>>httpd/httpd/trunk/modules/http2/h2_config.c
>>>httpd/httpd/trunk/modules/http2/h2_config.h
>>>httpd/httpd/trunk/modules/http2/h2_conn_io.c
>>>httpd/httpd/trunk/modules/http2/h2_mplx.c
>>>httpd/httpd/trunk/modules/http2/h2_mplx.h
>>>httpd/httpd/trunk/modules/http2/h2_proxy_session.c
>>>httpd/httpd/trunk/modules/http2/h2_proxy_session.h
>>>httpd/httpd/trunk/modules/http2/h2_request.c
>>>httpd/httpd/trunk/modules/http2/h2_session.c
>>>httpd/httpd/trunk/modules/http2/h2_session.h
>>>httpd/httpd/trunk/modules/http2/h2_stream.c
>>>httpd/httpd/trunk/modules/http2/h2_task.c
>>>httpd/httpd/trunk/modules/http2/h2_task.h
>>>httpd/httpd/trunk/modules/http2/h2_version.h
>>>httpd/httpd/trunk/modules/http2/mod_http2.c
>>>httpd/httpd/trunk/modules/http2/mod_http2.h
>>>httpd/httpd/trunk/modules/http2/mod_proxy_http2.c
>>>
>>
>>> Modified: httpd/httpd/trunk/modules/http2/mod_http2.h
>>> URL: 
>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/mod_http2.h?rev=1854963=1854962=1854963=diff
>>> ==
>>> --- httpd/httpd/trunk/modules/http2/mod_http2.h (original)
>>> +++ httpd/httpd/trunk/modules/http2/mod_http2.h Thu Mar  7 09:41:15 2019
>>> @@ -30,22 +30,20 @@ APR_DECLARE_OPTIONAL_FN(int,
>>>
>>>
>>> /***
>>> - * HTTP/2 request engines
>>> + * START HTTP/2 request engines (DEPRECATED)
>>>  
>>> **/
>>> +
>>> +/* The following functions were introduced for the experimental 
>>> mod_proxy_http2
>>> + * support, but have been abandoned since.
>>> + * They are still declared here for backward compatibiliy, in case someone
>>> + * tries to build an old mod_proxy_http2 against it, but will disappear
>>> + * completely sometime in the future.
>>> + */ 
>>>
>>> struct apr_thread_cond_t;
>>> -
>>> typedef struct h2_req_engine h2_req_engine;
>>> -
>>> typedef void http2_output_consumed(void *ctx, conn_rec *c, apr_off_t 
>>> consumed);
>>>
>>> -/**
>>> - * Initialize a h2_req_engine. The structure will be passed in but
>>> - * only the name and master are set. The function should initialize
>>> - * all fields.
>>> - * @param engine the allocated, partially filled structure
>>> - * @param r  the first request to process, or NULL
>>> - */
>>> typedef apr_status_t http2_req_engine_init(h2_req_engine *engine, 
>>>const char *id, 
>>>const char *type,
>>> @@ -55,35 +53,11 @@ typedef apr_status_t http2_req_engine_in
>>>http2_output_consumed 
>>> **pconsumed,
>>>void **pbaton);
>>>
>>> -/**
>>> - * Push a request to an engine with the specified name for further 
>>> processing.
>>> - * If no such engine is available, einit is not NULL, einit is called 
>>> - * with a new engine record and the caller is responsible for running the
>>> - * new engine instance.
>>> - * @param engine_type the type of the engine to add the request to
>>> - * @param r   the request to push to an engine for processing
>>> - * @param einit   an optional initialization callback 

Re: svn commit: r1854963 - in /httpd/httpd/trunk: ./ modules/http2/

2022-08-17 Thread Stefan Eissing via dev



> Am 17.08.2022 um 09:26 schrieb Ruediger Pluem :
> 
> 
> 
> On 3/7/19 10:41 AM, ic...@apache.org wrote:
>> Author: icing
>> Date: Thu Mar  7 09:41:15 2019
>> New Revision: 1854963
>> 
>> URL: http://svn.apache.org/viewvc?rev=1854963=rev
>> Log:
>>  *) mod_http2: new configuration directive: ```H2Padding numbits``` to 
>> control 
>> padding of HTTP/2 payload frames. 'numbits' is a number from 0-8,
>> controlling the range of padding bytes added to a frame. The actual 
>> number
>> added is chosen randomly per frame. This applies to HEADERS, DATA and 
>> PUSH_PROMISE
>> frames equally. The default continues to be 0, e.g. no padding. [Stefan 
>> Eissing] 
>> 
>>  *) mod_http2: ripping out all the h2_req_engine internal features now that 
>> mod_proxy_http2
>> has no more need for it. Optional functions are still declared but no 
>> longer implemented.
>> While previous mod_proxy_http2 will work with this, it is recommeneded 
>> to run the matching
>> versions of both modules. [Stefan Eissing]
>> 
>>  *) mod_proxy_http2: changed mod_proxy_http2 implementation and fixed 
>> several bugs which
>> resolve PR63170. The proxy module does now a single h2 request on the 
>> (reused)
>> connection and returns. [Stefan Eissing]
>> 
>> 
>> Removed:
>>httpd/httpd/trunk/modules/http2/h2_ngn_shed.c
>>httpd/httpd/trunk/modules/http2/h2_ngn_shed.h
>> Modified:
>>httpd/httpd/trunk/CHANGES
>>httpd/httpd/trunk/modules/http2/config2.m4
>>httpd/httpd/trunk/modules/http2/h2.h
>>httpd/httpd/trunk/modules/http2/h2_config.c
>>httpd/httpd/trunk/modules/http2/h2_config.h
>>httpd/httpd/trunk/modules/http2/h2_conn_io.c
>>httpd/httpd/trunk/modules/http2/h2_mplx.c
>>httpd/httpd/trunk/modules/http2/h2_mplx.h
>>httpd/httpd/trunk/modules/http2/h2_proxy_session.c
>>httpd/httpd/trunk/modules/http2/h2_proxy_session.h
>>httpd/httpd/trunk/modules/http2/h2_request.c
>>httpd/httpd/trunk/modules/http2/h2_session.c
>>httpd/httpd/trunk/modules/http2/h2_session.h
>>httpd/httpd/trunk/modules/http2/h2_stream.c
>>httpd/httpd/trunk/modules/http2/h2_task.c
>>httpd/httpd/trunk/modules/http2/h2_task.h
>>httpd/httpd/trunk/modules/http2/h2_version.h
>>httpd/httpd/trunk/modules/http2/mod_http2.c
>>httpd/httpd/trunk/modules/http2/mod_http2.h
>>httpd/httpd/trunk/modules/http2/mod_proxy_http2.c
>> 
> 
>> Modified: httpd/httpd/trunk/modules/http2/mod_http2.h
>> URL: 
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/mod_http2.h?rev=1854963=1854962=1854963=diff
>> ==
>> --- httpd/httpd/trunk/modules/http2/mod_http2.h (original)
>> +++ httpd/httpd/trunk/modules/http2/mod_http2.h Thu Mar  7 09:41:15 2019
>> @@ -30,22 +30,20 @@ APR_DECLARE_OPTIONAL_FN(int,
>> 
>> 
>> /***
>> - * HTTP/2 request engines
>> + * START HTTP/2 request engines (DEPRECATED)
>>  
>> **/
>> +
>> +/* The following functions were introduced for the experimental 
>> mod_proxy_http2
>> + * support, but have been abandoned since.
>> + * They are still declared here for backward compatibiliy, in case someone
>> + * tries to build an old mod_proxy_http2 against it, but will disappear
>> + * completely sometime in the future.
>> + */ 
>> 
>> struct apr_thread_cond_t;
>> -
>> typedef struct h2_req_engine h2_req_engine;
>> -
>> typedef void http2_output_consumed(void *ctx, conn_rec *c, apr_off_t 
>> consumed);
>> 
>> -/**
>> - * Initialize a h2_req_engine. The structure will be passed in but
>> - * only the name and master are set. The function should initialize
>> - * all fields.
>> - * @param engine the allocated, partially filled structure
>> - * @param r  the first request to process, or NULL
>> - */
>> typedef apr_status_t http2_req_engine_init(h2_req_engine *engine, 
>>const char *id, 
>>const char *type,
>> @@ -55,35 +53,11 @@ typedef apr_status_t http2_req_engine_in
>>http2_output_consumed **pconsumed,
>>void **pbaton);
>> 
>> -/**
>> - * Push a request to an engine with the specified name for further 
>> processing.
>> - * If no such engine is available, einit is not NULL, einit is called 
>> - * with a new engine record and the caller is responsible for running the
>> - * new engine instance.
>> - * @param engine_type the type of the engine to add the request to
>> - * @param r   the request to push to an engine for processing
>> - * @param einit   an optional initialization callback for a new engine 
>> - *of the requested type, should no instance be 
>> available.
>> - *By passing a non-NULL callback, 

Re: svn commit: r1854963 - in /httpd/httpd/trunk: ./ modules/http2/

2022-08-17 Thread Ruediger Pluem



On 3/7/19 10:41 AM, ic...@apache.org wrote:
> Author: icing
> Date: Thu Mar  7 09:41:15 2019
> New Revision: 1854963
> 
> URL: http://svn.apache.org/viewvc?rev=1854963=rev
> Log:
>   *) mod_http2: new configuration directive: ```H2Padding numbits``` to 
> control 
>  padding of HTTP/2 payload frames. 'numbits' is a number from 0-8,
>  controlling the range of padding bytes added to a frame. The actual 
> number
>  added is chosen randomly per frame. This applies to HEADERS, DATA and 
> PUSH_PROMISE
>  frames equally. The default continues to be 0, e.g. no padding. [Stefan 
> Eissing] 
>   
>   *) mod_http2: ripping out all the h2_req_engine internal features now that 
> mod_proxy_http2
>  has no more need for it. Optional functions are still declared but no 
> longer implemented.
>  While previous mod_proxy_http2 will work with this, it is recommeneded 
> to run the matching
>  versions of both modules. [Stefan Eissing]
>   
>   *) mod_proxy_http2: changed mod_proxy_http2 implementation and fixed 
> several bugs which
>  resolve PR63170. The proxy module does now a single h2 request on the 
> (reused)
>  connection and returns. [Stefan Eissing]
> 
> 
> Removed:
> httpd/httpd/trunk/modules/http2/h2_ngn_shed.c
> httpd/httpd/trunk/modules/http2/h2_ngn_shed.h
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/http2/config2.m4
> httpd/httpd/trunk/modules/http2/h2.h
> httpd/httpd/trunk/modules/http2/h2_config.c
> httpd/httpd/trunk/modules/http2/h2_config.h
> httpd/httpd/trunk/modules/http2/h2_conn_io.c
> httpd/httpd/trunk/modules/http2/h2_mplx.c
> httpd/httpd/trunk/modules/http2/h2_mplx.h
> httpd/httpd/trunk/modules/http2/h2_proxy_session.c
> httpd/httpd/trunk/modules/http2/h2_proxy_session.h
> httpd/httpd/trunk/modules/http2/h2_request.c
> httpd/httpd/trunk/modules/http2/h2_session.c
> httpd/httpd/trunk/modules/http2/h2_session.h
> httpd/httpd/trunk/modules/http2/h2_stream.c
> httpd/httpd/trunk/modules/http2/h2_task.c
> httpd/httpd/trunk/modules/http2/h2_task.h
> httpd/httpd/trunk/modules/http2/h2_version.h
> httpd/httpd/trunk/modules/http2/mod_http2.c
> httpd/httpd/trunk/modules/http2/mod_http2.h
> httpd/httpd/trunk/modules/http2/mod_proxy_http2.c
> 

> Modified: httpd/httpd/trunk/modules/http2/mod_http2.h
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/mod_http2.h?rev=1854963=1854962=1854963=diff
> ==
> --- httpd/httpd/trunk/modules/http2/mod_http2.h (original)
> +++ httpd/httpd/trunk/modules/http2/mod_http2.h Thu Mar  7 09:41:15 2019
> @@ -30,22 +30,20 @@ APR_DECLARE_OPTIONAL_FN(int,
>  
>  
>  
> /***
> - * HTTP/2 request engines
> + * START HTTP/2 request engines (DEPRECATED)
>   
> **/
> +
> +/* The following functions were introduced for the experimental 
> mod_proxy_http2
> + * support, but have been abandoned since.
> + * They are still declared here for backward compatibiliy, in case someone
> + * tries to build an old mod_proxy_http2 against it, but will disappear
> + * completely sometime in the future.
> + */ 
>   
>  struct apr_thread_cond_t;
> -
>  typedef struct h2_req_engine h2_req_engine;
> -
>  typedef void http2_output_consumed(void *ctx, conn_rec *c, apr_off_t 
> consumed);
>  
> -/**
> - * Initialize a h2_req_engine. The structure will be passed in but
> - * only the name and master are set. The function should initialize
> - * all fields.
> - * @param engine the allocated, partially filled structure
> - * @param r  the first request to process, or NULL
> - */
>  typedef apr_status_t http2_req_engine_init(h2_req_engine *engine, 
> const char *id, 
> const char *type,
> @@ -55,35 +53,11 @@ typedef apr_status_t http2_req_engine_in
> http2_output_consumed **pconsumed,
> void **pbaton);
>  
> -/**
> - * Push a request to an engine with the specified name for further 
> processing.
> - * If no such engine is available, einit is not NULL, einit is called 
> - * with a new engine record and the caller is responsible for running the
> - * new engine instance.
> - * @param engine_type the type of the engine to add the request to
> - * @param r   the request to push to an engine for processing
> - * @param einit   an optional initialization callback for a new engine 
> - *of the requested type, should no instance be available.
> - *By passing a non-NULL callback, the caller is willing
> - *to init and run a new engine itself.
> - * @return APR_SUCCESS iff slave