You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Torsten Foertsch <to...@gmx.net> on 2006/01/06 22:09:14 UTC

[PATCH] Apache2::RequestUtil->add_config

Hi,

the patch below solves the following situation.

The ProxyPassReverse directive is outside any <Location> block given as

  ProxyPassReverse /path http://...

but inside a Location block without the /path specification:

  <Location /path>
  ProxyPassReverse http://...
  </Location>

In the latter case the path is taken from $parms->path, that means the path of 
the Location block.

Since $r->add_config([@lines]) is literally a

  <Location />
  @lines
  </Location>

ProxyPassReverse cannot be applied with $r->add_config for a path other than 
"/".

The patch adds an optional 3rd parameter to $r->add_config that lets you pass 
in the path.

I don't think ProxyPassReverse is the only directive that behaves this way.

Torsten

Re: [PATCH] Apache2::RequestUtil->add_config

Posted by Torsten Foertsch <to...@gmx.net>.
On Saturday 07 January 2006 10:17, Philippe M. Chiasson wrote:
> Well, you could do add_config(
> <Location /path>
>   ProxyPassReverse http://...
> </Location>);
>
> No ?

I tried that. But <Location> is not allowed inside a <Location> block:

$r->add_config() has failed: <Location> cannot occur within 
<Directory/Location/Files> section

Torsten

Re: [PATCH] Apache2::RequestUtil->add_config

Posted by Torsten Foertsch <to...@gmx.net>.
On Wednesday 18 January 2006 02:41, Philippe M. Chiasson wrote:
> I've finally got around looking at this patch, and it looks good. I tweaked
> it some (whitespace and formatting) and checked it in as r370001 & r370002.

Thanks,

Torsten

Re: [PATCH] Apache2::RequestUtil->add_config

Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Philippe M. Chiasson wrote:
> Torsten Foertsch wrote:
> 
>>Hi,
>>
>>the patch below solves the following situation.
>>
>>The ProxyPassReverse directive is outside any <Location> block given as
>>
>>  ProxyPassReverse /path http://...
>>
>>but inside a Location block without the /path specification:
>>
>>  <Location /path>
>>  ProxyPassReverse http://...
>>  </Location>
>>
>>In the latter case the path is taken from $parms->path, that means the path of 
>>the Location block.
>>
> [...]
>
>>The patch adds an optional 3rd parameter to $r->add_config that lets you pass 
>>in the path.
>>
>>I don't think ProxyPassReverse is the only directive that behaves this way.
> 
> As a more generic solution, this might make sense. On first glance, your patch
> looks sane, but I won't have the time to really look at it until Jan 15th

I've finally got around looking at this patch, and it looks good. I tweaked it
some (whitespace and formatting) and checked it in as r370001 & r370002.

--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/     F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5

Re: [PATCH] Apache2::RequestUtil->add_config

Posted by Torsten Foertsch <to...@gmx.net>.
On Saturday 07 January 2006 18:39, Philip M. Gollucci wrote:
> > As a more generic solution, this might make sense. On first glance, your
> > patch looks sane, but I won't have the time to really look at it until
> > Jan 15th, so if anybody else feels like writing a few tests for this and
> > beat me to it...
>
> Wouldn't that be an API change ?

I would say an API extension. add_config is prototyped to expect up to 3 
parameters $r, $lines and $override. If called with more it throws an 
exception. Hence, if a new 4th optional parameter is added it cannot break 
any existing code.

Torsten

Re: [PATCH] Apache2::RequestUtil->add_config

Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
> Philip M. Gollucci wrote:
> 
>>>As a more generic solution, this might make sense. On first glance,
>>>your patch
>>>looks sane, but I won't have the time to really look at it until Jan
>>>15th, so
>>>if anybody else feels like writing a few tests for this and beat me to
>>>it...
>>
>>Wouldn't that be an API change ?
> 
> 
> Yes, I guess so, yet at the same time, I am not sure if there is anything
> wrong with changing a function from
> 
> ([foo]) to ([foo],bar);
> 
> Since that can't possibly break old code.

API changes within the same generation of the product are fine as long as 
they are backward-compatible (which seems to be the case).

-- 
_____________________________________________________________
Stas Bekman mailto:stas@stason.org  http://stason.org/
MailChannels: Assured Messaging(TM) http://mailchannels.com/
The "Practical mod_perl" book       http://modperlbook.org/
http://perl.apache.org/ http://perl.org/ http://logilune.com/


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: [PATCH] Apache2::RequestUtil->add_config

Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Philip M. Gollucci wrote:
>> As a more generic solution, this might make sense. On first glance,
>> your patch
>> looks sane, but I won't have the time to really look at it until Jan
>> 15th, so
>> if anybody else feels like writing a few tests for this and beat me to
>> it...
> 
> Wouldn't that be an API change ?

Yes, I guess so, yet at the same time, I am not sure if there is anything
wrong with changing a function from

([foo]) to ([foo],bar);

Since that can't possibly break old code.

> Agree with the patch and it looks good, but no time to free tuits right
> now.

I'll get to it when I get back home on Jan 15th then, no biggie.

-- 
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/     F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5

Re: [PATCH] Apache2::RequestUtil->add_config

Posted by "Philip M. Gollucci" <pg...@p6m7g8.com>.
> As a more generic solution, this might make sense. On first glance, your patch
> looks sane, but I won't have the time to really look at it until Jan 15th, so
> if anybody else feels like writing a few tests for this and beat me to it...
Wouldn't that be an API change ?

Agree with the patch and it looks good, but no time to free tuits right now.

-- 
------------------------------------------------------------------------
"Love is not the one you can picture yourself marrying,
but the one you can't picture the rest of your life without."

"It takes a minute to have a crush on someone, an hour to like someone,
and a day to love someone, but it takes a lifetime to forget someone..."

"I wanna hold ya till I die ... I wanna hold ya till the fear in me 
subsides."

Philip M. Gollucci (pgollucci@p6m7g8.com) 301.254.5198
Consultant / http://p6m7g8.net/Resume/resume.shtml
Senior Software Engineer - TicketMaster - http://ticketmaster.com
1024D/A79997FA F357 0FDD 2301 6296 690F  6A47 D55A 7172 A799 97F

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: [PATCH] Apache2::RequestUtil->add_config

Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Torsten Foertsch wrote:
> Hi,
> 
> the patch below solves the following situation.
> 
> The ProxyPassReverse directive is outside any <Location> block given as
> 
>   ProxyPassReverse /path http://...
> 
> but inside a Location block without the /path specification:
> 
>   <Location /path>
>   ProxyPassReverse http://...
>   </Location>
> 
> In the latter case the path is taken from $parms->path, that means the path of 
> the Location block.
> 
> Since $r->add_config([@lines]) is literally a
> 
>   <Location />
>   @lines
>   </Location>

Well, it's not exactly that, literally, but, yup, it's close enough.

> ProxyPassReverse cannot be applied with $r->add_config for a path other than 
> "/".

Well, you could do add_config(
<Location /path>
  ProxyPassReverse http://...
</Location>);

No ?

> The patch adds an optional 3rd parameter to $r->add_config that lets you pass 
> in the path.
> 
> I don't think ProxyPassReverse is the only directive that behaves this way.

As a more generic solution, this might make sense. On first glance, your patch
looks sane, but I won't have the time to really look at it until Jan 15th, so
if anybody else feels like writing a few tests for this and beat me to it...

-- 
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/     F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5