You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jim Jagielski <ji...@jaguNET.com> on 2016/08/14 19:53:06 UTC

Re: svn commit: r1756186 - in /httpd/httpd/trunk: include/ modules/http/ modules/proxy/

> On Aug 12, 2016, at 9:58 AM, ylavic@apache.org wrote:
> 
> Modified: httpd/httpd/trunk/include/ap_mmn.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=1756186&r1=1756185&r2=1756186&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/include/ap_mmn.h (original)
> +++ httpd/httpd/trunk/include/ap_mmn.h Fri Aug 12 13:58:10 2016
> @@ -532,10 +532,11 @@
>  *                         dav_success_proppatch.
>  * 20160608.4 (2.5.0-dev)  Add dav_acl_provider, dav_acl_provider_register
>  *                         dav_get_acl_providers.
> - * 20160608.5 (2.5.0-dev)  Add ap_proxy_check_backend(), and tmp_bb to struct
> - *                         proxy_conn_rec.
> + * 20160608.5 (2.5.0-dev)  Add ap_proxy_check_connection(), and tmp_bb to
> + *                         struct proxy_conn_rec.
>  * 20160608.6 (2.5.0-dev)  Add ap_scan_http_field_content, ap_scan_http_token
>  *                         and ap_get_http_token
> + * 20160608.7 (2.5.0-dev)  Add ap_check_pipeline().
>  */
> 
> #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
> @@ -543,7 +544,7 @@
> #ifndef MODULE_MAGIC_NUMBER_MAJOR
> #define MODULE_MAGIC_NUMBER_MAJOR 20160608
> #endif
> -#define MODULE_MAGIC_NUMBER_MINOR 6                 /* 0...n */
> +#define MODULE_MAGIC_NUMBER_MINOR 7                 /* 0...n */
> 

I know that there are no real ABI guarantees on trunk, but it
still seems to me that this should have been 2 bumps, one of which
said "replaced ap_proxy_check_backend with ap_proxy_check_connection"
for some history.

>         else {
> -            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(03408)
> +            ap_log_error(APLOG_MARK, APLOG_WARNING, 0, server, APLOGNO(03408)
>                          "%s: reusable backend connection is not empty: "
> -                         "forcibly closed", proxy_function);
> +                         "forcibly closed", scheme);
>         }
> 

Why the change in log level?


Re: svn commit: r1756186 - in /httpd/httpd/trunk: include/ modules/http/ modules/proxy/

Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Aug 15, 2016 at 2:46 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> On Aug 14, 2016 3:35 PM, "Yann Ylavic" <yl...@gmail.com> wrote:
>>
>> > Why the change in log level?
>>
>> Reading a response before any request was sent looks quite severe to
>> me, a warning might better alert the admin about some (backend)
>> misbehaviour/response splitting, while a debug could stay unnoticed.
>
> If this is their own back end (and not in a forward proxy scenario) info
> level should be sufficient for daily operation.

Agreed, changed in r1756531, thanks for the suggestion.

Re: svn commit: r1756186 - in /httpd/httpd/trunk: include/ modules/http/ modules/proxy/

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Aug 14, 2016 3:35 PM, "Yann Ylavic" <yl...@gmail.com> wrote:
>
> > Why the change in log level?
>
> Reading a response before any request was sent looks quite severe to
> me, a warning might better alert the admin about some (backend)
> misbehaviour/response splitting, while a debug could stay unnoticed.

If this is their own back end (and not in a forward proxy scenario) info
level should be sufficient for daily operation.

Re: svn commit: r1756186 - in /httpd/httpd/trunk: include/ modules/http/ modules/proxy/

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
It is an abusive attempt, yes. But noise for every abuse is an issue for
admins attempting to trim their log file details.

Warning exists only to alert the admin of some action they should take.
This abuse attempt failed, so no action needed IMO.

On Aug 14, 2016 3:35 PM, "Yann Ylavic" <yl...@gmail.com> wrote:

On Sun, Aug 14, 2016 at 9:53 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>
>> On Aug 12, 2016, at 9:58 AM, ylavic@apache.org wrote:
>>
>> Modified: httpd/httpd/trunk/include/ap_mmn.h
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_
mmn.h?rev=1756186&r1=1756185&r2=1756186&view=diff
>> ============================================================
==================
>> --- httpd/httpd/trunk/include/ap_mmn.h (original)
>> +++ httpd/httpd/trunk/include/ap_mmn.h Fri Aug 12 13:58:10 2016
>> @@ -532,10 +532,11 @@
>>  *                         dav_success_proppatch.
>>  * 20160608.4 (2.5.0-dev)  Add dav_acl_provider,
dav_acl_provider_register
>>  *                         dav_get_acl_providers.
>> - * 20160608.5 (2.5.0-dev)  Add ap_proxy_check_backend(), and tmp_bb to
struct
>> - *                         proxy_conn_rec.
>> + * 20160608.5 (2.5.0-dev)  Add ap_proxy_check_connection(), and tmp_bb
to
>> + *                         struct proxy_conn_rec.
>>  * 20160608.6 (2.5.0-dev)  Add ap_scan_http_field_content,
ap_scan_http_token
>>  *                         and ap_get_http_token
>> + * 20160608.7 (2.5.0-dev)  Add ap_check_pipeline().
>>  */
>>
>> #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
>> @@ -543,7 +544,7 @@
>> #ifndef MODULE_MAGIC_NUMBER_MAJOR
>> #define MODULE_MAGIC_NUMBER_MAJOR 20160608
>> #endif
>> -#define MODULE_MAGIC_NUMBER_MINOR 6                 /* 0...n */
>> +#define MODULE_MAGIC_NUMBER_MINOR 7                 /* 0...n */
>>
>
> I know that there are no real ABI guarantees on trunk, but it
> still seems to me that this should have been 2 bumps, one of which
> said "replaced ap_proxy_check_backend with ap_proxy_check_connection"
> for some history.

I rather handled it as a follow up (2 months later..), but the second
bump makes sense, will do.

>
>>         else {
>> -            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(03408)
>> +            ap_log_error(APLOG_MARK, APLOG_WARNING, 0, server,
APLOGNO(03408)
>>                          "%s: reusable backend connection is not empty: "
>> -                         "forcibly closed", proxy_function);
>> +                         "forcibly closed", scheme);
>>         }
>>
>
> Why the change in log level?

Reading a response before any request was sent looks quite severe to
me, a warning might better alert the admin about some (backend)
misbehaviour/response splitting, while a debug could stay unnoticed.

Re: svn commit: r1756186 - in /httpd/httpd/trunk: include/ modules/http/ modules/proxy/

Posted by Yann Ylavic <yl...@gmail.com>.
On Sun, Aug 14, 2016 at 9:53 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>
>> On Aug 12, 2016, at 9:58 AM, ylavic@apache.org wrote:
>>
>> Modified: httpd/httpd/trunk/include/ap_mmn.h
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=1756186&r1=1756185&r2=1756186&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/include/ap_mmn.h (original)
>> +++ httpd/httpd/trunk/include/ap_mmn.h Fri Aug 12 13:58:10 2016
>> @@ -532,10 +532,11 @@
>>  *                         dav_success_proppatch.
>>  * 20160608.4 (2.5.0-dev)  Add dav_acl_provider, dav_acl_provider_register
>>  *                         dav_get_acl_providers.
>> - * 20160608.5 (2.5.0-dev)  Add ap_proxy_check_backend(), and tmp_bb to struct
>> - *                         proxy_conn_rec.
>> + * 20160608.5 (2.5.0-dev)  Add ap_proxy_check_connection(), and tmp_bb to
>> + *                         struct proxy_conn_rec.
>>  * 20160608.6 (2.5.0-dev)  Add ap_scan_http_field_content, ap_scan_http_token
>>  *                         and ap_get_http_token
>> + * 20160608.7 (2.5.0-dev)  Add ap_check_pipeline().
>>  */
>>
>> #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
>> @@ -543,7 +544,7 @@
>> #ifndef MODULE_MAGIC_NUMBER_MAJOR
>> #define MODULE_MAGIC_NUMBER_MAJOR 20160608
>> #endif
>> -#define MODULE_MAGIC_NUMBER_MINOR 6                 /* 0...n */
>> +#define MODULE_MAGIC_NUMBER_MINOR 7                 /* 0...n */
>>
>
> I know that there are no real ABI guarantees on trunk, but it
> still seems to me that this should have been 2 bumps, one of which
> said "replaced ap_proxy_check_backend with ap_proxy_check_connection"
> for some history.

I rather handled it as a follow up (2 months later..), but the second
bump makes sense, will do.

>
>>         else {
>> -            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(03408)
>> +            ap_log_error(APLOG_MARK, APLOG_WARNING, 0, server, APLOGNO(03408)
>>                          "%s: reusable backend connection is not empty: "
>> -                         "forcibly closed", proxy_function);
>> +                         "forcibly closed", scheme);
>>         }
>>
>
> Why the change in log level?

Reading a response before any request was sent looks quite severe to
me, a warning might better alert the admin about some (backend)
misbehaviour/response splitting, while a debug could stay unnoticed.