You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Luca Toscano <to...@gmail.com> on 2017/03/09 21:47:00 UTC
Re: [users@httpd] Problem when using nested if statements in apache 2.4
2017-02-24 23:27 GMT+01:00 Luca Toscano <to...@gmail.com>:
> Hi everybody,
>
> the following users@ question is interesting in my opinion:
>
> 2017-02-20 18:17 GMT+01:00 Mike Schlottman <ms...@spe.org>:
>
>>
>> The problem comes when I combine these 2 so that all users except those
>> coming from 127.*.*.* or 192.168.*.* see the nice error page.
>>
>> <If "! %{HTTP:X-Real-IP} -ipmatch '127.0.0.0/8' ">
>>
>> <If "! %{HTTP:X-Real-IP} -ipmatch '192.168.0.0/16' ">
>>
>> ErrorDocument 404 /errors/404
>>
>> </If>
>>
>> </If>
>>
>> The user from 172.28.1.84 does not get the nice 404 page, but the default
>> 404 page. The IP does not match either of the ranges as observed when
>> using the ranges individually, but when combined in this way it does not
>> work as expected.
>>
>>
>>
>> Any ideas why this is?
>>
>>
>>
>
> He ended up using a single if with an expression composed by the two
> conditions in && to solve the problem, but I started to wonder why this
> configuration does not work. I tested the "nested ifs" config with trace8
> logging and it seem that only the outermost <If> expression gets evaluated.
>
> Is there a specific reason why this happens? I'd expect two possible
> outcomes from this configuration, namely either a syntax error while
> parsing the config (preferred imho) or a context merge, but not a no-op.
>
> Any pointers/suggestions about where to look?
>
>
I tried to investigate a bit the problem to write a good explanation in the
docs (and also to better understand the httpd internals for fun) but I am
still far from a satisfactory result.
I tried the following config (inside other containers like Location too):
<If "true">
ErrorDocument 503 "This is a custom 503 inside the outer IF!"
<If "true">
ErrorDocument 503 "This is a custom 503 inside the inner IF!"
</If>
</If>
Each time that 503 is generated, I get the "outer IF" 503 response, never
the inner one.
With gdb it is easy to see that when ap_if_walk is executed,
dconf->sec_if->nelts is 1, so only one of the Ifs is evaluated.
So I checked how the if sections are parsed and built
with ap_process_config_tree->ap_walk_config->ap_walk_config_sub->invoke_cmd->ifsection
and eventually ap_add_if, but I only got that the parent/child relationship
is set up correctly.
My only goal is to figure out if nested Ifs are not evaluated on purpose
and if so why they are allowed by the syntax checker. Eventually it would
be really great to update the docs (regular user docs and also
https://httpd.apache.org/docs/trunk/developer/request.html).
Any hints would really be appreciated :)
Thanks in advance!
Luca
Re: [users@httpd] Problem when using nested if statements in apache 2.4
Posted by Luca Toscano <to...@gmail.com>.
2017-04-19 10:05 GMT+02:00 Luca Toscano <to...@gmail.com>:
>
>
> 2017-03-27 19:09 GMT+02:00 Luca Toscano <to...@gmail.com>:
>
>> Hi Yann,
>>
>> 2017-03-27 8:56 GMT+02:00 Yann Ylavic <yl...@gmail.com>:
>>
>>> Hi Luca,
>>>
>>> On Mon, Mar 20, 2017 at 1:25 PM, Luca Toscano <to...@gmail.com>
>>> wrote:
>>> >
>>> > Documentation updated with the current status, plus the following patch
>>> > seems to allow nested if blocks (probably not the best one but it is a
>>> pof):
>>> >
>>> > http://home.apache.org/~elukey/httpd-trunk-core-nested_if_blocks.patch
>>>
>>> LGTM (nit: ap_if_walk_sub() needs not be AP_DECLARE()d since it's a
>>> local helper only).
>>>
>>
>> Thanks a lot for the feedback, I modified the patch to include your
>> suggestion and another one from Jim (checking whether or not the
>> ap_if_walk_sub calls return something different than OK, hope that the
>> implementation is what was expected). Ran also the test suite in trunk, no
>> failure registered (Jacob: make check is really awesome).
>>
>> I'd like to perform some performance tests before proceeding, this code
>> adds a (probably negligible in 90% of the use case) overhead to each
>> ap_if_walk call (running twice for each request afaics).
>>
>> Any other comment/review will be really appreciated :)
>>
>>
> Updates:
>
> - the original patch evolved up to http://home.apache.org/~elukey
> /httpd-trunk-core-nested_if_blocks.patch but it was a bit too hacky.
>
> - Jacob reviewed the code and suggested to investigate the use of
> now_merged to clean up the code, and a new patch came to life:
> http://home.apache.org/~elukey/httpd-trunk-core-nested_if_blocks_v2.patch
>
> - Added some tests to the suite in http://home.apache.org/~elukey
> /httpd-framework-nested-ifs.patch, everything seems to work fine. My
> understanding is that the tests are leveraging the "Header merge" feature
> to simulate the order of merging with different <If> condition evaluations.
> I added the same block of nested <If>/ElseIf/Else to
> Directory/Location/Files and the result looks consistent.
>
> Comments welcome, I think the work is almost done but I might have missed
> something..
>
For the records the trunk patch has been committed in r1792589 and the
related tests in r1793187.
Luca
Re: [users@httpd] Problem when using nested if statements in apache 2.4
Posted by Luca Toscano <to...@gmail.com>.
2017-03-27 19:09 GMT+02:00 Luca Toscano <to...@gmail.com>:
> Hi Yann,
>
> 2017-03-27 8:56 GMT+02:00 Yann Ylavic <yl...@gmail.com>:
>
>> Hi Luca,
>>
>> On Mon, Mar 20, 2017 at 1:25 PM, Luca Toscano <to...@gmail.com>
>> wrote:
>> >
>> > Documentation updated with the current status, plus the following patch
>> > seems to allow nested if blocks (probably not the best one but it is a
>> pof):
>> >
>> > http://home.apache.org/~elukey/httpd-trunk-core-nested_if_blocks.patch
>>
>> LGTM (nit: ap_if_walk_sub() needs not be AP_DECLARE()d since it's a
>> local helper only).
>>
>
> Thanks a lot for the feedback, I modified the patch to include your
> suggestion and another one from Jim (checking whether or not the
> ap_if_walk_sub calls return something different than OK, hope that the
> implementation is what was expected). Ran also the test suite in trunk, no
> failure registered (Jacob: make check is really awesome).
>
> I'd like to perform some performance tests before proceeding, this code
> adds a (probably negligible in 90% of the use case) overhead to each
> ap_if_walk call (running twice for each request afaics).
>
> Any other comment/review will be really appreciated :)
>
>
Updates:
- the original patch evolved up to http://home.apache.org/~
elukey/httpd-trunk-core-nested_if_blocks.patch but it was a bit too hacky.
- Jacob reviewed the code and suggested to investigate the use of
now_merged to clean up the code, and a new patch came to life:
http://home.apache.org/~elukey/httpd-trunk-core-nested_if_blocks_v2.patch
- Added some tests to the suite in http://home.apache.org/~
elukey/httpd-framework-nested-ifs.patch, everything seems to work fine. My
understanding is that the tests are leveraging the "Header merge" feature
to simulate the order of merging with different <If> condition evaluations.
I added the same block of nested <If>/ElseIf/Else to
Directory/Location/Files and the result looks consistent.
Comments welcome, I think the work is almost done but I might have missed
something..
Luca
Re: [users@httpd] Problem when using nested if statements in apache 2.4
Posted by Luca Toscano <to...@gmail.com>.
Hi Yann,
2017-03-27 8:56 GMT+02:00 Yann Ylavic <yl...@gmail.com>:
> Hi Luca,
>
> On Mon, Mar 20, 2017 at 1:25 PM, Luca Toscano <to...@gmail.com>
> wrote:
> >
> > Documentation updated with the current status, plus the following patch
> > seems to allow nested if blocks (probably not the best one but it is a
> pof):
> >
> > http://home.apache.org/~elukey/httpd-trunk-core-nested_if_blocks.patch
>
> LGTM (nit: ap_if_walk_sub() needs not be AP_DECLARE()d since it's a
> local helper only).
>
Thanks a lot for the feedback, I modified the patch to include your
suggestion and another one from Jim (checking whether or not the
ap_if_walk_sub calls return something different than OK, hope that the
implementation is what was expected). Ran also the test suite in trunk, no
failure registered (Jacob: make check is really awesome).
I'd like to perform some performance tests before proceeding, this code
adds a (probably negligible in 90% of the use case) overhead to each
ap_if_walk call (running twice for each request afaics).
Any other comment/review will be really appreciated :)
Luca
Re: [users@httpd] Problem when using nested if statements in apache 2.4
Posted by Yann Ylavic <yl...@gmail.com>.
Hi Luca,
On Mon, Mar 20, 2017 at 1:25 PM, Luca Toscano <to...@gmail.com> wrote:
>
> Documentation updated with the current status, plus the following patch
> seems to allow nested if blocks (probably not the best one but it is a pof):
>
> http://home.apache.org/~elukey/httpd-trunk-core-nested_if_blocks.patch
LGTM (nit: ap_if_walk_sub() needs not be AP_DECLARE()d since it's a
local helper only).
Thanks for this work.
Regards,
Yann.
Re: [users@httpd] Problem when using nested if statements in apache 2.4
Posted by Luca Toscano <to...@gmail.com>.
2017-03-17 22:33 GMT+01:00 Luca Toscano <to...@gmail.com>:
>
>
> 2017-03-09 22:47 GMT+01:00 Luca Toscano <to...@gmail.com>:
>
>>
>>
>> 2017-02-24 23:27 GMT+01:00 Luca Toscano <to...@gmail.com>:
>>
>>> Hi everybody,
>>>
>>> the following users@ question is interesting in my opinion:
>>>
>>> 2017-02-20 18:17 GMT+01:00 Mike Schlottman <ms...@spe.org>:
>>>
>>>>
>>>> The problem comes when I combine these 2 so that all users except those
>>>> coming from 127.*.*.* or 192.168.*.* see the nice error page.
>>>>
>>>> <If "! %{HTTP:X-Real-IP} -ipmatch '127.0.0.0/8' ">
>>>>
>>>> <If "! %{HTTP:X-Real-IP} -ipmatch '192.168.0.0/16' ">
>>>>
>>>> ErrorDocument 404 /errors/404
>>>>
>>>> </If>
>>>>
>>>> </If>
>>>>
>>>> The user from 172.28.1.84 does not get the nice 404 page, but the
>>>> default 404 page. The IP does not match either of the ranges as observed
>>>> when using the ranges individually, but when combined in this way it does
>>>> not work as expected.
>>>>
>>>>
>>>>
>>>> Any ideas why this is?
>>>>
>>>>
>>>>
>>>
>>> He ended up using a single if with an expression composed by the two
>>> conditions in && to solve the problem, but I started to wonder why this
>>> configuration does not work. I tested the "nested ifs" config with trace8
>>> logging and it seem that only the outermost <If> expression gets evaluated.
>>>
>>> Is there a specific reason why this happens? I'd expect two possible
>>> outcomes from this configuration, namely either a syntax error while
>>> parsing the config (preferred imho) or a context merge, but not a no-op.
>>>
>>> Any pointers/suggestions about where to look?
>>>
>>>
>> I tried to investigate a bit the problem to write a good explanation in
>> the docs (and also to better understand the httpd internals for fun) but I
>> am still far from a satisfactory result.
>>
>> I tried the following config (inside other containers like Location too):
>>
>> <If "true">
>> ErrorDocument 503 "This is a custom 503 inside the outer IF!"
>> <If "true">
>> ErrorDocument 503 "This is a custom 503 inside the inner IF!"
>> </If>
>> </If>
>>
>> Each time that 503 is generated, I get the "outer IF" 503 response, never
>> the inner one.
>>
>> With gdb it is easy to see that when ap_if_walk is executed,
>> dconf->sec_if->nelts is 1, so only one of the Ifs is evaluated.
>>
>> So I checked how the if sections are parsed and built
>> with ap_process_config_tree->ap_walk_config->ap_walk_config_sub->invoke_cmd->ifsection
>> and eventually ap_add_if, but I only got that the parent/child relationship
>> is set up correctly.
>>
>> My only goal is to figure out if nested Ifs are not evaluated on purpose
>> and if so why they are allowed by the syntax checker. Eventually it would
>> be really great to update the docs (regular user docs and also
>> https://httpd.apache.org/docs/trunk/developer/request.html).
>>
>>
> This time (hopefully) I think I found what I was looking for, namely why
> nested <If> blocks are definitely not going to work.
>
> The ap_if_walk (request.c) function gets the core_dir_config settings
> via ap_get_core_module_config(r->per_dir_config), retrieving all the <If>
> containers using the sec_if field, and then iterating through them to
> evaluate their (apr_expr) conditions.
>
> In order to be able to use nested <If> blocks ap_if_walk would need to
> check, for each sec_if->elts element of the core's r->per_dir_config, if it
> contains a sec_if blocks too (and restart the process of checking its
> condition etc..). Something like:
>
> AP_DECLARE(int) ap_if_walk(request_rec *r) {
> [..]
> /* Go through the if entries, and check for matches */
> for (sec_idx = 0; sec_idx < num_sec; ++sec_idx) {
> core_dir_config *entry_core;
> entry_core = ap_get_core_module_config(sec_ent[sec_idx]);
> if(entry_core->sec_if->nelts > 0) { .. /* handling nested <If>s */
> .. }
> [..]
>
> In this way we would allow arbitrary nesting of <If> blocks in the httpd
> config. I am not advocating for this solution but we should either allow
> nested <If> blocks or explicitly warn the users when httpd checks its
> config that a nested configuration will be silently ignored (emitting an
> explicit configuration error while parsing the config would be even better
> but probably too invasive for 2.4.x releases).
>
Documentation updated with the current status, plus the following patch
seems to allow nested if blocks (probably not the best one but it is a pof):
http://home.apache.org/~elukey/httpd-trunk-core-nested_if_blocks.patch
Luca
Re: [users@httpd] Problem when using nested if statements in apache 2.4
Posted by Luca Toscano <to...@gmail.com>.
2017-03-09 22:47 GMT+01:00 Luca Toscano <to...@gmail.com>:
>
>
> 2017-02-24 23:27 GMT+01:00 Luca Toscano <to...@gmail.com>:
>
>> Hi everybody,
>>
>> the following users@ question is interesting in my opinion:
>>
>> 2017-02-20 18:17 GMT+01:00 Mike Schlottman <ms...@spe.org>:
>>
>>>
>>> The problem comes when I combine these 2 so that all users except those
>>> coming from 127.*.*.* or 192.168.*.* see the nice error page.
>>>
>>> <If "! %{HTTP:X-Real-IP} -ipmatch '127.0.0.0/8' ">
>>>
>>> <If "! %{HTTP:X-Real-IP} -ipmatch '192.168.0.0/16' ">
>>>
>>> ErrorDocument 404 /errors/404
>>>
>>> </If>
>>>
>>> </If>
>>>
>>> The user from 172.28.1.84 does not get the nice 404 page, but the
>>> default 404 page. The IP does not match either of the ranges as observed
>>> when using the ranges individually, but when combined in this way it does
>>> not work as expected.
>>>
>>>
>>>
>>> Any ideas why this is?
>>>
>>>
>>>
>>
>> He ended up using a single if with an expression composed by the two
>> conditions in && to solve the problem, but I started to wonder why this
>> configuration does not work. I tested the "nested ifs" config with trace8
>> logging and it seem that only the outermost <If> expression gets evaluated.
>>
>> Is there a specific reason why this happens? I'd expect two possible
>> outcomes from this configuration, namely either a syntax error while
>> parsing the config (preferred imho) or a context merge, but not a no-op.
>>
>> Any pointers/suggestions about where to look?
>>
>>
> I tried to investigate a bit the problem to write a good explanation in
> the docs (and also to better understand the httpd internals for fun) but I
> am still far from a satisfactory result.
>
> I tried the following config (inside other containers like Location too):
>
> <If "true">
> ErrorDocument 503 "This is a custom 503 inside the outer IF!"
> <If "true">
> ErrorDocument 503 "This is a custom 503 inside the inner IF!"
> </If>
> </If>
>
> Each time that 503 is generated, I get the "outer IF" 503 response, never
> the inner one.
>
> With gdb it is easy to see that when ap_if_walk is executed,
> dconf->sec_if->nelts is 1, so only one of the Ifs is evaluated.
>
> So I checked how the if sections are parsed and built
> with ap_process_config_tree->ap_walk_config->ap_walk_config_sub->invoke_cmd->ifsection
> and eventually ap_add_if, but I only got that the parent/child relationship
> is set up correctly.
>
> My only goal is to figure out if nested Ifs are not evaluated on purpose
> and if so why they are allowed by the syntax checker. Eventually it would
> be really great to update the docs (regular user docs and also
> https://httpd.apache.org/docs/trunk/developer/request.html).
>
>
This time (hopefully) I think I found what I was looking for, namely why
nested <If> blocks are definitely not going to work.
The ap_if_walk (request.c) function gets the core_dir_config settings
via ap_get_core_module_config(r->per_dir_config), retrieving all the <If>
containers using the sec_if field, and then iterating through them to
evaluate their (apr_expr) conditions.
In order to be able to use nested <If> blocks ap_if_walk would need to
check, for each sec_if->elts element of the core's r->per_dir_config, if it
contains a sec_if blocks too (and restart the process of checking its
condition etc..). Something like:
AP_DECLARE(int) ap_if_walk(request_rec *r) {
[..]
/* Go through the if entries, and check for matches */
for (sec_idx = 0; sec_idx < num_sec; ++sec_idx) {
core_dir_config *entry_core;
entry_core = ap_get_core_module_config(sec_ent[sec_idx]);
if(entry_core->sec_if->nelts > 0) { .. /* handling nested <If>s */
.. }
[..]
In this way we would allow arbitrary nesting of <If> blocks in the httpd
config. I am not advocating for this solution but we should either allow
nested <If> blocks or explicitly warn the users when httpd checks its
config that a nested configuration will be silently ignored (emitting an
explicit configuration error while parsing the config would be even better
but probably too invasive for 2.4.x releases).
Thoughts?
Luca