You are viewing a plain text version of this content. The canonical link for it is here.
Posted to test-dev@httpd.apache.org by Sander Temme <sc...@covalent.net> on 2003/07/10 23:11:09 UTC
[PATCH] Re: cvs commit:
httpd-test/perl-framework/Apache-Test/lib/Apache TestRequest.pm
on 7/10/03 12:56, Sander Temme at sctemme@covalent.net wrote:
> parameter to the request invocations in t/apache/acceptpathinfo.t. Neither
> produces any result. Am I looking in the right place?
Breadcrumbing my way through Apache-Test/lib/Apache/TestRequest.pm by
liberally sprinkling print statements, I get to the following around line
97:
>> + if (my $redir = $args->{requests_redirectable}) {
>> + if (ref $redir and (@$redir > 1 or $redir->[0] ne 'POST')) {
>> + $RedirectOK = 1;
>> + } else {
>> + $RedirectOK = 0;
>> + }
>> + } else {
>> + $RedirectOK = $redir;
>> + }
>> +
We arrive at this if statement in all failing acceptpathinfo and
modules/alias tests, and end up in the else clause because $redir is
undefined. The result is that we reset RedirectOK to something undefined,
hence don't follow the redirection. Getting rid of the else clause gets me
my passing tests back, since RedirectOK stays at its default value of '1'.
Like so:
Index: Apache-Test/lib/Apache/TestRequest.pm
===================================================================
RCS file:
/home/cvspublic/httpd-test/perl-framework/Apache-Test/lib/Apache/TestRequest
.pm,v
retrieving revision 1.81
diff -u -r1.81 TestRequest.pm
--- Apache-Test/lib/Apache/TestRequest.pm 8 Jul 2003 07:56:24 -0000
1.81
+++ Apache-Test/lib/Apache/TestRequest.pm 10 Jul 2003 21:05:34 -0000
@@ -94,8 +94,6 @@
} else {
$RedirectOK = 0;
}
- } else {
- $RedirectOK = $redir;
}
$args->{keep_alive} ||= $ENV{APACHE_TEST_HTTP11};
(also attached to avoid breakage by line wrappage). Tested on Darwin 6.6,
Perl 5.6.0, against Apache 2.0.47.
Does that sound sound?
S.
--
Covalent Technologies sctemme@covalent.net
Engineering group Voice: (415) 856 4214
303 Second Street #375 South Fax: (415) 856 4210
San Francisco CA 94107
PGP Fingerprint: 7A8D B189 E871 80CB 9521 9320 C11E 7B47 964F 31D9
=======================================================
This email message is for the sole use of the intended recipient(s) and may
contain confidential and privileged information. Any unauthorized review,
use, disclosure or distribution is prohibited. If you are not the intended
recipient, please contact the sender by reply email and destroy all copies
of the original message
=======================================================
Re: [PATCH] Re: cvs commit: httpd-test/perl-framework/Apache-Test/lib/Apache
TestRequest.pm
Posted by Stas Bekman <st...@stason.org>.
David Wheeler wrote:
> On Friday, July 11, 2003, at 09:27 AM, Sander Temme wrote:
>
>>> The above patch doesn't work. But this does:
>>
>>
>> Ehm... works for me. I think you're working in the mod_perl space and I'm
>> just concentrating on the Apache core. Maybe there are side effects
>> that I'm
>> not seeing?
>
>
> Yes, the redirect_ok subroutine, which is relevant to using lwp to send
> requests to the server.. It might work to have it return 0 instead of
> undef, though.
>
>> I don't think it will... I need redirect to be 1 under the circumstances
>> that I described: during those apache/acceptpathinfo and modules/alias
>> tests
>> that need it. So, unless there is a compelling reason (like
>> $args->{requests_redirectable} existing and containing pertinent
>> information), we should leave $RedirectOK alone here.
>
>
> Oops, of course. The problem was actually my stupid use of local in a
> block where it actually wouldn't do anything. So try this, instead:
>
> --- TestRequest.pm.~1.81.~ Fri Jul 11 09:02:32 2003
> +++ TestRequest.pm Fri Jul 11 10:43:36 2003
> @@ -88,14 +88,13 @@
> $UA = undef;
> }
>
> - if (my $redir = $args->{requests_redirectable}) {
> + if (exists $args->{requests_redirectable}) {
> + my $redir = $args->{requests_redirectable};
> if (ref $redir and (@$redir > 1 or $redir->[0] ne 'POST')) {
> $RedirectOK = 1;
> } else {
> $RedirectOK = 0;
> }
> - } else {
> - $RedirectOK = $redir;
> }
>
> $args->{keep_alive} ||= $ENV{APACHE_TEST_HTTP11};
> @@ -298,9 +297,9 @@
> sub UPLOAD {
> my($url, $pass, $keep) = prepare(@_);
>
> - if (exists $keep->{redirect_ok}) {
> - local $RedirectOK = $keep->{redirect_ok};
> - }
> + local $RedirectOK = exists $keep->{redirect_ok} ?
> + $keep->{redirect_ok} : $RedirectOK;
> +
> if ($keep->{filename}) {
> return upload_file($url, $keep->{filename}, $pass);
> }
> @@ -461,9 +460,8 @@
>
> *$name = sub {
> my($url, $pass, $keep) = prepare(@_);
> - if (exists $keep->{redirect_ok}) {
> - local $RedirectOK = $keep->{redirect_ok};
> - }
> + local $RedirectOK = exists $keep->{redirect_ok} ?
> + $keep->{redirect_ok} : $RedirectOK;
> return lwp_call($method, undef, $url, @$pass);
> };
That looks good. I've committed it. Thanks David!
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: [PATCH] Re: cvs commit: httpd-test/perl-framework/Apache-Test/lib/Apache TestRequest.pm
Posted by David Wheeler <da...@kineticode.com>.
On Friday, July 11, 2003, at 09:27 AM, Sander Temme wrote:
>> The above patch doesn't work. But this does:
>
> Ehm... works for me. I think you're working in the mod_perl space and
> I'm
> just concentrating on the Apache core. Maybe there are side effects
> that I'm
> not seeing?
Yes, the redirect_ok subroutine, which is relevant to using lwp to send
requests to the server.. It might work to have it return 0 instead of
undef, though.
> I don't think it will... I need redirect to be 1 under the
> circumstances
> that I described: during those apache/acceptpathinfo and modules/alias
> tests
> that need it. So, unless there is a compelling reason (like
> $args->{requests_redirectable} existing and containing pertinent
> information), we should leave $RedirectOK alone here.
Oops, of course. The problem was actually my stupid use of local in a
block where it actually wouldn't do anything. So try this, instead:
--- TestRequest.pm.~1.81.~ Fri Jul 11 09:02:32 2003
+++ TestRequest.pm Fri Jul 11 10:43:36 2003
@@ -88,14 +88,13 @@
$UA = undef;
}
- if (my $redir = $args->{requests_redirectable}) {
+ if (exists $args->{requests_redirectable}) {
+ my $redir = $args->{requests_redirectable};
if (ref $redir and (@$redir > 1 or $redir->[0] ne 'POST')) {
$RedirectOK = 1;
} else {
$RedirectOK = 0;
}
- } else {
- $RedirectOK = $redir;
}
$args->{keep_alive} ||= $ENV{APACHE_TEST_HTTP11};
@@ -298,9 +297,9 @@
sub UPLOAD {
my($url, $pass, $keep) = prepare(@_);
- if (exists $keep->{redirect_ok}) {
- local $RedirectOK = $keep->{redirect_ok};
- }
+ local $RedirectOK = exists $keep->{redirect_ok} ?
+ $keep->{redirect_ok} : $RedirectOK;
+
if ($keep->{filename}) {
return upload_file($url, $keep->{filename}, $pass);
}
@@ -461,9 +460,8 @@
*$name = sub {
my($url, $pass, $keep) = prepare(@_);
- if (exists $keep->{redirect_ok}) {
- local $RedirectOK = $keep->{redirect_ok};
- }
+ local $RedirectOK = exists $keep->{redirect_ok} ?
+ $keep->{redirect_ok} : $RedirectOK;
return lwp_call($method, undef, $url, @$pass);
};
Regards,
David
--
David Wheeler AIM: dwTheory
david@kineticode.com ICQ: 15726394
http://kineticode.com/ Yahoo!: dew7e
Jabber: Theory@jabber.org
Kineticode. Setting knowledge in motion.[sm]
Re: [PATCH] Re: cvs commit:
httpd-test/perl-framework/Apache-Test/lib/Apache TestRequest.pm
Posted by Sander Temme <sc...@covalent.net>.
on 7/11/03 9:06, David Wheeler at david@kineticode.com wrote:
> The above patch doesn't work. But this does:
Ehm... works for me. I think you're working in the mod_perl space and I'm
just concentrating on the Apache core. Maybe there are side effects that I'm
not seeing?
> --- TestRequest.pm.~1.81.~ Fri Jul 11 09:02:32 2003
> +++ TestRequest.pm Fri Jul 11 09:03:59 2003
> @@ -95,7 +95,7 @@
> $RedirectOK = 0;
> }
> } else {
> - $RedirectOK = $redir;
> + $RedirectOK = 0;
> }
>
> $args->{keep_alive} ||= $ENV{APACHE_TEST_HTTP11};
>
> IOW, $RedirectOK needs a value. Sender, does this work for you?
I don't think it will... I need redirect to be 1 under the circumstances
that I described: during those apache/acceptpathinfo and modules/alias tests
that need it. So, unless there is a compelling reason (like
$args->{requests_redirectable} existing and containing pertinent
information), we should leave $RedirectOK alone here.
S.
--
Covalent Technologies sctemme@covalent.net
Engineering group Voice: (415) 856 4214
303 Second Street #375 South Fax: (415) 856 4210
San Francisco CA 94107
PGP Fingerprint: 7A8D B189 E871 80CB 9521 9320 C11E 7B47 964F 31D9
=======================================================
This email message is for the sole use of the intended recipient(s) and may
contain confidential and privileged information. Any unauthorized review,
use, disclosure or distribution is prohibited. If you are not the intended
recipient, please contact the sender by reply email and destroy all copies
of the original message
=======================================================
Re: [PATCH] Re: cvs commit: httpd-test/perl-framework/Apache-Test/lib/Apache TestRequest.pm
Posted by David Wheeler <da...@kineticode.com>.
On Friday, July 11, 2003, at 02:22 AM, Stas Bekman wrote:
>> diff -u -r1.81 TestRequest.pm
>> --- Apache-Test/lib/Apache/TestRequest.pm 8 Jul 2003 07:56:24
>> -0000
>> 1.81
>> +++ Apache-Test/lib/Apache/TestRequest.pm 11 Jul 2003 04:28:45
>> -0000
>> @@ -88,14 +88,12 @@
>> $UA = undef;
>> }
>> - if (my $redir = $args->{requests_redirectable}) {
>> + if (defined (my $redir = $args->{requests_redirectable})) {
>> if (ref $redir and (@$redir > 1 or $redir->[0] ne 'POST')) {
>> $RedirectOK = 1;
>> } else {
>> $RedirectOK = 0;
>> }
>> - } else {
>> - $RedirectOK = $redir;
>> }
>> $args->{keep_alive} ||= $ENV{APACHE_TEST_HTTP11};
>> In other words, if $redir is defined, do something with it to the
>> effect of
>> $RedirectOK. If it isn't defined, go with the default value which is
>> 1.
>
> Looks right to me. I was just looking at the diff that you've posted
> earlier hence the awkward fixup suggestion. You probably don't even
> need defined. I'll let David decide how he wants it.
Gee, thanks!
> David Wheeler was trying to make $RedirectOK to be a non-global flag,
> so you can change it locally to your specific test. He will soonish
> reply confirming whether this still works for him or not. I don't have
> the test suite that he has, so I can't test.
The above patch doesn't work. But this does:
--- TestRequest.pm.~1.81.~ Fri Jul 11 09:02:32 2003
+++ TestRequest.pm Fri Jul 11 09:03:59 2003
@@ -95,7 +95,7 @@
$RedirectOK = 0;
}
} else {
- $RedirectOK = $redir;
+ $RedirectOK = 0;
}
$args->{keep_alive} ||= $ENV{APACHE_TEST_HTTP11};
IOW, $RedirectOK needs a value. Sender, does this work for you?
Regards,
David
--
David Wheeler AIM: dwTheory
david@kineticode.com ICQ: 15726394
http://kineticode.com/ Yahoo!: dew7e
Jabber: Theory@jabber.org
Kineticode. Setting knowledge in motion.[sm]
Re: [PATCH] Re: cvs commit: httpd-test/perl-framework/Apache-Test/lib/Apache
TestRequest.pm
Posted by Stas Bekman <st...@stason.org>.
Sander Temme wrote:
> on 7/10/03 18:00, Stas Bekman at stas@stason.org wrote:
>
>
>>Does this work?
>>
>>else if ($redir) {
>> $RedirectOK = $redir;
>>}
>
>
> It does. However, isn't this the same condition as in the top if clause?
> Wouldn't you want to:
>
> Index: Apache-Test/lib/Apache/TestRequest.pm
> ===================================================================
> RCS file:
> /home/cvspublic/httpd-test/perl-framework/Apache-Test/lib/Apache/TestRequest
> .pm,v
> retrieving revision 1.81
> diff -u -r1.81 TestRequest.pm
> --- Apache-Test/lib/Apache/TestRequest.pm 8 Jul 2003 07:56:24 -0000
> 1.81
> +++ Apache-Test/lib/Apache/TestRequest.pm 11 Jul 2003 04:28:45 -0000
> @@ -88,14 +88,12 @@
> $UA = undef;
> }
>
> - if (my $redir = $args->{requests_redirectable}) {
> + if (defined (my $redir = $args->{requests_redirectable})) {
> if (ref $redir and (@$redir > 1 or $redir->[0] ne 'POST')) {
> $RedirectOK = 1;
> } else {
> $RedirectOK = 0;
> }
> - } else {
> - $RedirectOK = $redir;
> }
>
> $args->{keep_alive} ||= $ENV{APACHE_TEST_HTTP11};
>
> In other words, if $redir is defined, do something with it to the effect of
> $RedirectOK. If it isn't defined, go with the default value which is 1.
Looks right to me. I was just looking at the diff that you've posted earlier
hence the awkward fixup suggestion. You probably don't even need defined. I'll
let David decide how he wants it.
> What
> exactly is this code trying to do?
David Wheeler was trying to make $RedirectOK to be a non-global flag, so you
can change it locally to your specific test. He will soonish reply confirming
whether this still works for him or not. I don't have the test suite that he
has, so I can't test.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: [PATCH] Re: cvs commit:
httpd-test/perl-framework/Apache-Test/lib/Apache TestRequest.pm
Posted by Sander Temme <sc...@covalent.net>.
on 7/10/03 18:00, Stas Bekman at stas@stason.org wrote:
>
> Does this work?
>
> else if ($redir) {
> $RedirectOK = $redir;
> }
It does. However, isn't this the same condition as in the top if clause?
Wouldn't you want to:
Index: Apache-Test/lib/Apache/TestRequest.pm
===================================================================
RCS file:
/home/cvspublic/httpd-test/perl-framework/Apache-Test/lib/Apache/TestRequest
.pm,v
retrieving revision 1.81
diff -u -r1.81 TestRequest.pm
--- Apache-Test/lib/Apache/TestRequest.pm 8 Jul 2003 07:56:24 -0000
1.81
+++ Apache-Test/lib/Apache/TestRequest.pm 11 Jul 2003 04:28:45 -0000
@@ -88,14 +88,12 @@
$UA = undef;
}
- if (my $redir = $args->{requests_redirectable}) {
+ if (defined (my $redir = $args->{requests_redirectable})) {
if (ref $redir and (@$redir > 1 or $redir->[0] ne 'POST')) {
$RedirectOK = 1;
} else {
$RedirectOK = 0;
}
- } else {
- $RedirectOK = $redir;
}
$args->{keep_alive} ||= $ENV{APACHE_TEST_HTTP11};
In other words, if $redir is defined, do something with it to the effect of
$RedirectOK. If it isn't defined, go with the default value which is 1. What
exactly is this code trying to do?
S.
--
Covalent Technologies sctemme@covalent.net
Engineering group Voice: (415) 856 4214
303 Second Street #375 South Fax: (415) 856 4210
San Francisco CA 94107
PGP Fingerprint: 7A8D B189 E871 80CB 9521 9320 C11E 7B47 964F 31D9
=======================================================
This email message is for the sole use of the intended recipient(s) and may
contain confidential and privileged information. Any unauthorized review,
use, disclosure or distribution is prohibited. If you are not the intended
recipient, please contact the sender by reply email and destroy all copies
of the original message
=======================================================
Re: [PATCH] Re: cvs commit: httpd-test/perl-framework/Apache-Test/lib/Apache
TestRequest.pm
Posted by Stas Bekman <st...@stason.org>.
Sander Temme wrote:
> on 7/10/03 12:56, Sander Temme at sctemme@covalent.net wrote:
>
>
>>parameter to the request invocations in t/apache/acceptpathinfo.t. Neither
>>produces any result. Am I looking in the right place?
>
>
> Breadcrumbing my way through Apache-Test/lib/Apache/TestRequest.pm by
> liberally sprinkling print statements, I get to the following around line
> 97:
>
>
>>>+ if (my $redir = $args->{requests_redirectable}) {
>>>+ if (ref $redir and (@$redir > 1 or $redir->[0] ne 'POST')) {
>>>+ $RedirectOK = 1;
>>>+ } else {
>>>+ $RedirectOK = 0;
>>>+ }
>>>+ } else {
>>>+ $RedirectOK = $redir;
>>>+ }
>>>+
>
>
> We arrive at this if statement in all failing acceptpathinfo and
> modules/alias tests, and end up in the else clause because $redir is
> undefined. The result is that we reset RedirectOK to something undefined,
> hence don't follow the redirection. Getting rid of the else clause gets me
> my passing tests back, since RedirectOK stays at its default value of '1'.
> Like so:
>
> Index: Apache-Test/lib/Apache/TestRequest.pm
> ===================================================================
> RCS file:
> /home/cvspublic/httpd-test/perl-framework/Apache-Test/lib/Apache/TestRequest
> .pm,v
> retrieving revision 1.81
> diff -u -r1.81 TestRequest.pm
> --- Apache-Test/lib/Apache/TestRequest.pm 8 Jul 2003 07:56:24 -0000
> 1.81
> +++ Apache-Test/lib/Apache/TestRequest.pm 10 Jul 2003 21:05:34 -0000
> @@ -94,8 +94,6 @@
> } else {
> $RedirectOK = 0;
> }
> - } else {
> - $RedirectOK = $redir;
> }
>
> $args->{keep_alive} ||= $ENV{APACHE_TEST_HTTP11};
>
> (also attached to avoid breakage by line wrappage). Tested on Darwin 6.6,
> Perl 5.6.0, against Apache 2.0.47.
>
> Does that sound sound?
Does this work?
else if ($redir) {
$RedirectOK = $redir;
}
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com