On Wed, Aug 28, 2013 at 1:11 PM, Mike Rumph <[email protected]> wrote:

> Hello all,
>
> Since the URL validation in apr_uri_parse() has been tightened in the
> handling of the scheme portion of a URL,
> I submitted a patch to httpd bug 55315 against the mod_proxy code in httpd
> trunk to handle the special case
> of interpolating a variable in the scheme portion of a URL.
>
> - 
> https://issues.apache.org/**bugzilla/show_bug.cgi?id=55315<https://issues.apache.org/bugzilla/show_bug.cgi?id=55315>
>
>
Do you know if it is practical to have the one magic path down to
ap_proxy_define_worker() munge the URI?  I guess the problem is that
ap_proxy_define_worker() saves the parsed uri, and the caller (add_pass or
whatever it is) doesn't have access to that?


There still remains a fundamental conflict between httpd configuration
> interpolation and mod_proxy interpolation.
>
> Take the example of the directives listed in bug 55315.
>
>
>      ProxyPassInterpolateEnv On
>      RewriteEngine On
>
>      RewriteCond %{HTTPS} =off
>      RewriteRule . - [E=protocol:http]
>      RewriteCond %{HTTPS} =on
>      RewriteRule . - [E=protocol:https]
>
>      ProxyPass /my_app/ ${protocol}://1.2.3.4/my_app/ interpolate
>      ProxyPassReverse /my_app/ ${protocol}://1.2.3.4/my_app/ interpolate
>
> With my mod_proxy patch applied, httpd will now start.
> But the following warnings are received:
>
> [Wed Aug 28 08:58:55.082581 2013] [core:warn] [pid 12201:tid
> 47961371215184] AH00111: Config variable ${protocol} is not defined
> [Wed Aug 28 08:58:55.083249 2013] [core:warn] [pid 12201:tid
> 47961371215184] AH00111: Config variable ${protocol} is not defined
>
> These warnings are issued by ap_resolve_env() in server/core.c.
> The syntax "${...}" is used by both httpd configuration interpolation and
> mod_proxy interpolation.
> So this syntax has to get past the httpd configuration interpolation
> before it can be processed by mod_proxy interpolation.
>
> It is interesting that my research seems to indicate that mod_proxy
> interpolation was actually the first to be introduced into the code.
>

I guess the order is this:

1. support for environment variables in the config
2. mod_proxy interpolation
3. core server starts complaining if you have something that looks like an
envvar reference that isn't resolved

Is that what you mean?

The double use of ${} is nasty.  In the fullness of time, I think that
mod_proxy interpolation should support an additional syntax that doesn't
collide with the config-time processing.



>
> Here are some of my findings:
>
> - Trace the history of the mod_proxy interpolation code.
>     - 
> http://svn.apache.org/viewvc?**view=revision&revision=421686<http://svn.apache.org/viewvc?view=revision&revision=421686>
>         - Support environment variable interpolation in reverse proxy
> configuration
>         - Committed by Nick Kew in Apache httpd 2.3.0, 7/13/2006
>         - ProxyPassInterpolateEnv directive
>         - http://mail-archives.apache.**org/mod_mbox/httpd-dev/200607.**
> mbox/browser<http://mail-archives.apache.org/mod_mbox/httpd-dev/200607.mbox/browser>
>     - 
> http://svn.apache.org/viewvc?**view=revision&revision=589371<http://svn.apache.org/viewvc?view=revision&revision=589371>
>         - Committed by Nick Kew, 10/28/2007
>         - interpolate keyword
>     - 
> http://svn.apache.org/viewvc?**view=revision&revision=1065748<http://svn.apache.org/viewvc?view=revision&revision=1065748>
>         - Committed by Jim, 1/31/2011
>         - Added parm to ap_proxy_define_worker() and
> ap_proxy_define_balancer().
>     - 
> http://svn.apache.org/viewvc?**view=revision&revision=1207926<http://svn.apache.org/viewvc?view=revision&revision=1207926>
>         - Committed by Jim, 11/29/2011
>         - Added parm to ap_proxy_get_balancer().
> - Trace the history of ap_resolve_env() in server/core.c.
>     - 
> http://svn.apache.org/viewvc?**view=revision&revision=1061444<http://svn.apache.org/viewvc?view=revision&revision=1061444>
>         - Committed by Stefan Fritsch, 1/20/2011.
>         - Moves ap_resolve_env() from server/util.c to server/core.c.
>     - 
> http://svn.apache.org/viewvc?**view=revision&revision=1061465<http://svn.apache.org/viewvc?view=revision&revision=1061465>
>         - Committed by Stefan Fritsch, 1/20/2011.
>         - Changes variable translation table usage.
>     - 
> http://svn.apache.org/viewvc?**view=revision&revision=546651<http://svn.apache.org/viewvc?view=revision&revision=546651>
>         - Committed by Paul Querna, 6/12/2007.
>         - Add the 'Define' command to the core.
>         - This does exactly the same thing as adding a -D FOO to your
> command line.
>
> Thanks,
>
> Mike Rumph
>
>
> On 8/14/2013 7:40 AM, Mike Rumph wrote:
>
>> Thanks for your reply Stefan.
>>
>> First of all, your suggestion of passing __proxy_interpolate_start__ to
>> apr_uri_parse will also fail,
>> since '_' is not accepted by the code in the "find the scheme" section as
>> a valid character within the scheme.
>> Plus, the only first character that will be accepted is one that is
>> marked as T_ALPHA.
>>
>> Secondly, is it really a bug for apr_uri_parse() to be flexible enough to
>> allow variables within the URL to pass through?
>> This would depend on the exact API definition.
>> If this is not precisely documented somewhere,
>> then it is the code of apr_uri_parse() that is dictating the API.
>> If that is the case, then the fix for bug 52479 has changed the API.
>> Is this an acceptable thing to do between httpd 2.2.22 and 2.2.25 or
>> between APR-util 1.5.1 and 1.5.2?
>>
>> If apr_uri_parse() is not flexible enough,
>> then the callers of this function will need to be aware of this
>> and go out of their way to avoid problems with it.
>>
>> The suggestion of having mod_proxy replace variables in the scheme
>> portion of the URL
>> before calling apr_uri_parse() could be a possible way for this to be
>> handled in this case,
>> but it would mean that mod_proxy (or any other module that would want to
>> use variables)
>> would need to do part of the parsing itself.
>>
>> So I think it comes down to the question of whether or not
>> apr_uri_parse() was designed
>> to handle embedded variables.  If it isn't, then mod_proxy has misused
>> the API.
>> It does have its own interpolation code, but this is only called later
>> through the fixups and translate_name hooks.
>> It could be that the values to the variables might not be reliably
>> available at the time apr_uri_parse is called.
>> So mod_proxy would need to do some special processing just to allow the
>> variables to continue to exist
>> until they can be interpolated later.
>>
>> Perhaps someone that knows more about the mod_proxy code could offer some
>> suggestions here.
>>
>> Here are the relevant bugs for easy reference:
>>
>> - 55315:  error in ProxyPass URL parsing with interpolation
>> - 
>> https://issues.apache.org/**bugzilla/show_bug.cgi?id=55315<https://issues.apache.org/bugzilla/show_bug.cgi?id=55315>
>>
>> - 52479: apr_uri_parse("@localhost::**8080") reports "@localhost" for
>> the scheme
>> - 
>> https://issues.apache.org/**bugzilla/show_bug.cgi?id=52479<https://issues.apache.org/bugzilla/show_bug.cgi?id=52479>
>>
>> Take care,
>>
>> Mike Rumph
>>
>>
>> On 8/13/2013 2:17 PM, Stefan Fritsch wrote:
>>
>>> Am Dienstag, 13. August 2013, 13:34:49 schrieb Mike Rumph:
>>>
>>>> The ProxyPass directive is processed by the add_pass_noregex
>>>> function in mod_proxy.c. This calls add_pass which calls
>>>> ap_proxy_add_worker in proxy_util.c. ap_proxy_add_worker passes an
>>>> uninterpolated URL to apr_uri_parse() in apr-util/uri/apr_uri.c.
>>>> This is true for both httpd 2.2.22 and httpd 2.2.25.
>>>>
>>>> As a result of the fix for bug 52479,
>>>> apr_uri_parse no longer allows the interpolation characters (${}) to
>>>> pass through cleanly.
>>>>
>>>> The patch I submitted will allow the characters to pass through.
>>>>
>>>> But perhaps it is not correct for mod_proxy to be passing
>>>> uninterpolated URLs to apr_uri to begin with. Perhaps the mod_proxy
>>>> interpolation code should be structured differently.
>>>>
>>>
>>> This means httpd depends on a bug in apr_uri_parse(). I don't think we
>>> should re-introduce the bug, but fix httpd instead. If it needs to
>>> call apr_uri_parse() before interpolation, it could convert the
>>> special characters to strings that are acceptable in any URI part,
>>> call apr_uri_parse(), and then convert the strings back. Maybe replace
>>> '${' with __proxy_interpolate_start__ and '}' with
>>> __proxy_interpolate_end__? Or create a random string and check that it
>>> does not appear in the uri?
>>>
>>> It would be nicer if httpd could call apr_uri_parse() only after
>>> interpolation, but I don't know the relevant code and can't say if
>>> that's feasible.
>>>
>>>
>>>
>>>
>>
>>
>>
>


-- 
Born in Roswell... married an alien...
http://emptyhammock.com/

Reply via email to