You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jacob Champion <ch...@gmail.com> on 2017/06/20 16:07:44 UTC
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
On 02/08/2017 07:56 PM, Eric Covener wrote:
> Assuming there's some alternate path that actually does change
> SCRIPT_NAME by default, we a) don't have any complaint about
> SCRIPT_NAME and b) have the SetEnv thing. If we want more options,
> maybe we can stash this older SCRIPT_NAME into a new variable and show
> how to copy it over SCRIPT_NAME?
...and restarting this conversation, since the new behavior seems to
have a bug:
https://bz.apache.org/bugzilla/show_bug.cgi?id=61202
Can we do a quick fixup and reroll before it's too late?
--Jacob
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Champion <ch...@gmail.com>.
On 06/20/2017 06:39 PM, Jan Ehrhardt wrote:
> BTW: the developers at Directadmin are aware of this bug.
> https://forum.directadmin.com/showthread.php?t=54952&page=2&p=281618#post281618
> I ran into it while updating my Centos 6 servers.
Thanks for the heads up. CC's are coming in on the related bugs in
Bugzilla as well.
Anyone opposed to a users@ announcement on the issue?
--Jacob
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jan Ehrhardt <ph...@ehrhardt.nl>.
Jacob Champion in gmane.comp.apache.devel (Tue, 20 Jun 2017 09:07:44
-0700):
>On 02/08/2017 07:56 PM, Eric Covener wrote:
>> Assuming there's some alternate path that actually does change
>> SCRIPT_NAME by default, we a) don't have any complaint about
>> SCRIPT_NAME and b) have the SetEnv thing. If we want more options,
>> maybe we can stash this older SCRIPT_NAME into a new variable and show
>> how to copy it over SCRIPT_NAME?
>
>...and restarting this conversation, since the new behavior seems to
>have a bug:
>
>https://bz.apache.org/bugzilla/show_bug.cgi?id=61202
BTW: the developers at Directadmin are aware of this bug.
https://forum.directadmin.com/showthread.php?t=54952&page=2&p=281618#post281618
I ran into it while updating my Centos 6 servers.
--
Jan
RE: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by "Moradhassel, Kavian" <km...@ciena.com>.
Thanks! That’s exactly the kind of ballpark I was hoping to hear. ☺
From: William A Rowe Jr [mailto:wrowe@rowe-clan.net]
Sent: Tuesday, June 27, 2017 1:15 PM
To: httpd <de...@httpd.apache.org>
Subject: RE: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
On Jun 27, 2017 12:08 PM, "Moradhassel, Kavian" <km...@ciena.com>> wrote:
Did this discussion result in a decision to provide a fix for the bug in 2.4.26 and plan for a 2.4.27 soon? I'm wondering if I should be waiting for a 2.4.27 in the next handful of weeks, or if I should just accept that 2.4.26 has a bug that we need to work around...
We don't generally try to predict our release schedule, when it's baked we publish a release.
But Jim and Jacob are furiously updating the test framework as you were asking your question, so I'm guessing the group will be working to release this fix just as soon as it is agreed upon, within a week or few.
RE: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Jun 27, 2017 12:08 PM, "Moradhassel, Kavian" <km...@ciena.com> wrote:
Did this discussion result in a decision to provide a fix for the bug in
2.4.26 and plan for a 2.4.27 soon? I'm wondering if I should be waiting
for a 2.4.27 in the next handful of weeks, or if I should just accept that
2.4.26 has a bug that we need to work around...
We don't generally try to predict our release schedule, when it's baked we
publish a release.
But Jim and Jacob are furiously updating the test framework as you were
asking your question, so I'm guessing the group will be working to release
this fix just as soon as it is agreed upon, within a week or few.
RE: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by "Moradhassel, Kavian" <km...@ciena.com>.
Did this discussion result in a decision to provide a fix for the bug in 2.4.26 and plan for a 2.4.27 soon? I'm wondering if I should be waiting for a 2.4.27 in the next handful of weeks, or if I should just accept that 2.4.26 has a bug that we need to work around...
Thanks!
-----Original Message-----
From: William A Rowe Jr [mailto:wrowe@rowe-clan.net]
Sent: Tuesday, June 20, 2017 1:00 PM
To: httpd <de...@httpd.apache.org>
Subject: Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
On Tue, Jun 20, 2017 at 11:17 AM, Jacob Champion <ch...@gmail.com> wrote:
> On 06/20/2017 09:16 AM, William A Rowe Jr wrote:
>>
>> It's released into the wild, what is done is done.
>
>
> Of course. But having it in the wild for three days is different than having
> it in the wild for six months.
Unless you are classifying this a security fix, it may prove to be
a less popular update given what we announced as fixed in 2.4.26.
The same isn't expected for 2.4.27, since some users prioritize
security updates and let others sit for a while, or undergo added
scrutiny for non-critical updates.
In any case, it will be in the wild for >3 days. It's at five days and
counting, if you tag and roll today. Vote is 72 hrs long, and we
always give our mirrors a day to catch up with us.
You must presume it is in the wild, and shortening the exposure
by a matter of days isn't significant. It just needs the appropriate
clear note in CHANGES with 2.4.27 so that users are prepared
for the change (or reversion).
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Champion <ch...@gmail.com>.
On 06/20/2017 10:55 AM, Jim Jagielski wrote:
> It's already in the wild.
We do not guarantee bug compatibility. That's our judgment call based on
adoption, expected user disruption, time in the wild, etc.
The purpose of my Showstopper was to revert to known-good behavior. That
didn't happen, not least because I didn't catch this bug in the
backport, I wanted to keep things moving, and I +1'd it despite the
presence of the rider patch that I tried unsuccessfully to keep out [1].
So that's on me (and that's why I'm grinding this particular axe). I
want to fix the mistake I started last year, and not punish our users
with yet another unusable default for the third release in a row.
--Jacob
[1]
https://lists.apache.org/thread.html/539645ba41021f85a87ad397861cb8d09fb057f7fb7e9ed768781e85@%3Cdev.httpd.apache.org%3E
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jim Jagielski <ji...@jaguNET.com>.
> On Jun 20, 2017, at 1:18 PM, Jacob Champion <ch...@gmail.com> wrote:
>
> On 06/20/2017 10:12 AM, Jim Jagielski wrote:
>> All this is, IMO, moot until we have a *patch*.
>
> Agreed. See my other fork of this thread for my questions on that.
>
>> Right now there
>> is a work-around, which, again IMO, reduces the "need" to release
>> something "now"... the only question is whether the patch changes
>> the behavior of the 2 current settings or whether it adds a 3rd
>> option (the latter is my recommendation).
>
> The faster we can patch, the less there is a need to introduce yet another "ProxyFcgiBackendType FPM-But-Better-And-Without-Bugs-This-Time" option.
>
It's already in the wild.
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Champion <ch...@gmail.com>.
On 06/20/2017 10:12 AM, Jim Jagielski wrote:
> All this is, IMO, moot until we have a *patch*.
Agreed. See my other fork of this thread for my questions on that.
> Right now there
> is a work-around, which, again IMO, reduces the "need" to release
> something "now"... the only question is whether the patch changes
> the behavior of the 2 current settings or whether it adds a 3rd
> option (the latter is my recommendation).
The faster we can patch, the less there is a need to introduce yet
another "ProxyFcgiBackendType FPM-But-Better-And-Without-Bugs-This-Time"
option.
--Jacob
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Tue, Jun 20, 2017 at 1:32 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> On Tue, Jun 20, 2017 at 12:12 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>
>>> On Jun 20, 2017, at 1:03 PM, Jacob Champion <ch...@gmail.com> wrote:
>>>
>>> On 06/20/2017 10:00 AM, William A Rowe Jr wrote:
>>>> You must presume it is in the wild, and shortening the exposure
>>>> by a matter of days isn't significant.
>>>
>>> My point is that we should fix it ASAP. Days vs. more days may not be significant, but days vs. months is definitely significant when it comes to bug-compatibility and user expectations.
>>
>> All this is, IMO, moot until we have a *patch*. Right now there
>> is a work-around, which, again IMO, reduces the "need" to release
>> something "now"... the only question is whether the patch changes
>> the behavior of the 2 current settings or whether it adds a 3rd
>> option (the latter is my recommendation).
>
> To your point and Jacob's concern, whatever we settle on, we can
> surely add that upon adoption to
... the http://www.apache.org/dist/httpd/patches/apply_to_2.4.26/ tree
and provide the appropriate pointer on users@ and mention on the
various bugzilla tickets that arise.
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Tue, Jun 20, 2017 at 12:12 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>
>> On Jun 20, 2017, at 1:03 PM, Jacob Champion <ch...@gmail.com> wrote:
>>
>> On 06/20/2017 10:00 AM, William A Rowe Jr wrote:
>>> You must presume it is in the wild, and shortening the exposure
>>> by a matter of days isn't significant.
>>
>> My point is that we should fix it ASAP. Days vs. more days may not be significant, but days vs. months is definitely significant when it comes to bug-compatibility and user expectations.
>
> All this is, IMO, moot until we have a *patch*. Right now there
> is a work-around, which, again IMO, reduces the "need" to release
> something "now"... the only question is whether the patch changes
> the behavior of the 2 current settings or whether it adds a 3rd
> option (the latter is my recommendation).
To your point and Jacob's concern, whatever we settle on, we can
surely add that upon adoption to
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Perkins <ja...@cpanel.net>.
As a general FYI: cPanel can’t release 2.4.26 until this is patched / fixed. We were bitten *hard* by the SCRIPT_URI breakage a while back, and this is going to put a blocker on our release.
—
Jacob Perkins
Product Owner
cPanel Inc.
jacob.perkins@cpanel.net <ma...@cpanel.net>
Office: 713-529-0800 x 4046
Cell: 713-560-8655
> On Jun 20, 2017, at 12:12 PM, Jim Jagielski <ji...@jaguNET.com> wrote:
>
>
>> On Jun 20, 2017, at 1:03 PM, Jacob Champion <ch...@gmail.com> wrote:
>>
>> On 06/20/2017 10:00 AM, William A Rowe Jr wrote:
>>> You must presume it is in the wild, and shortening the exposure
>>> by a matter of days isn't significant.
>>
>> My point is that we should fix it ASAP. Days vs. more days may not be significant, but days vs. months is definitely significant when it comes to bug-compatibility and user expectations.
>
> All this is, IMO, moot until we have a *patch*. Right now there
> is a work-around, which, again IMO, reduces the "need" to release
> something "now"... the only question is whether the patch changes
> the behavior of the 2 current settings or whether it adds a 3rd
> option (the latter is my recommendation).
>
> And we should update ./t/modules/proxy_fcgi.t and maybe create
> a test which runs if PHP is available.
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jim Jagielski <ji...@jaguNET.com>.
> On Jun 20, 2017, at 1:03 PM, Jacob Champion <ch...@gmail.com> wrote:
>
> On 06/20/2017 10:00 AM, William A Rowe Jr wrote:
>> You must presume it is in the wild, and shortening the exposure
>> by a matter of days isn't significant.
>
> My point is that we should fix it ASAP. Days vs. more days may not be significant, but days vs. months is definitely significant when it comes to bug-compatibility and user expectations.
All this is, IMO, moot until we have a *patch*. Right now there
is a work-around, which, again IMO, reduces the "need" to release
something "now"... the only question is whether the patch changes
the behavior of the 2 current settings or whether it adds a 3rd
option (the latter is my recommendation).
And we should update ./t/modules/proxy_fcgi.t and maybe create
a test which runs if PHP is available.
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Champion <ch...@gmail.com>.
On 06/20/2017 10:00 AM, William A Rowe Jr wrote:
> You must presume it is in the wild, and shortening the exposure
> by a matter of days isn't significant.
My point is that we should fix it ASAP. Days vs. more days may not be
significant, but days vs. months is definitely significant when it comes
to bug-compatibility and user expectations.
--Jacob
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Tue, Jun 20, 2017 at 11:17 AM, Jacob Champion <ch...@gmail.com> wrote:
> On 06/20/2017 09:16 AM, William A Rowe Jr wrote:
>>
>> It's released into the wild, what is done is done.
>
>
> Of course. But having it in the wild for three days is different than having
> it in the wild for six months.
Unless you are classifying this a security fix, it may prove to be
a less popular update given what we announced as fixed in 2.4.26.
The same isn't expected for 2.4.27, since some users prioritize
security updates and let others sit for a while, or undergo added
scrutiny for non-critical updates.
In any case, it will be in the wild for >3 days. It's at five days and
counting, if you tag and roll today. Vote is 72 hrs long, and we
always give our mirrors a day to catch up with us.
You must presume it is in the wild, and shortening the exposure
by a matter of days isn't significant. It just needs the appropriate
clear note in CHANGES with 2.4.27 so that users are prepared
for the change (or reversion).
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Champion <ch...@gmail.com>.
On 06/20/2017 09:16 AM, William A Rowe Jr wrote:
> It's released into the wild, what is done is done.
Of course. But having it in the wild for three days is different than
having it in the wild for six months.
--Jacob
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Tue, Jun 20, 2017 at 11:15 AM, Jacob Champion <ch...@gmail.com> wrote:
> On 06/20/2017 09:14 AM, William A Rowe Jr wrote:
>>
>> Would encourage us to wait at least a couple more days for
>> other, unrelated regression reports to filter in while fixing this
>> defect. But there is nothing stopping a 2.4.27 in rapid
>> succession, we simply don't retroactively "retract" releases.
>
>
> Right, re-issuing 2.4.26 isn't what I meant. I just want to patch this up as
> quickly as possible so people don't standardize on the new behavior.
It's released into the wild, what is done is done.
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Champion <ch...@gmail.com>.
On 06/20/2017 09:14 AM, William A Rowe Jr wrote:
> Would encourage us to wait at least a couple more days for
> other, unrelated regression reports to filter in while fixing this
> defect. But there is nothing stopping a 2.4.27 in rapid
> succession, we simply don't retroactively "retract" releases.
Right, re-issuing 2.4.26 isn't what I meant. I just want to patch this
up as quickly as possible so people don't standardize on the new behavior.
--Jaccob
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Tue, Jun 20, 2017 at 11:07 AM, Jacob Champion <ch...@gmail.com> wrote:
> On 02/08/2017 07:56 PM, Eric Covener wrote:
>>
>> Assuming there's some alternate path that actually does change
>> SCRIPT_NAME by default, we a) don't have any complaint about
>> SCRIPT_NAME and b) have the SetEnv thing. If we want more options,
>> maybe we can stash this older SCRIPT_NAME into a new variable and show
>> how to copy it over SCRIPT_NAME?
>
>
> ...and restarting this conversation, since the new behavior seems to have a
> bug:
>
> https://bz.apache.org/bugzilla/show_bug.cgi?id=61202
>
> Can we do a quick fixup and reroll before it's too late?
Define reroll? too late?
I know what you mean by fixup, hopefully not so quick as to
overlook other tangential issues. But we never 're-roll', we
discard the candidate and roll the next version (this assures
that whomever has httpd-2.4.26.tar.gz in their hands has the
one and only one package of that name that was ever created.)
And 2.4.26 is released, announcement is away.
Would encourage us to wait at least a couple more days for
other, unrelated regression reports to filter in while fixing this
defect. But there is nothing stopping a 2.4.27 in rapid
succession, we simply don't retroactively "retract" releases.
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jim Jagielski <ji...@jaguNET.com>.
> On Jun 28, 2017, at 6:46 AM, Eric Covener <co...@gmail.com> wrote:
>
> On Wed, Jun 28, 2017 at 5:50 AM, Jim Jagielski <ji...@jagunet.com> wrote:
>> Is there someplace a set of examples on This Is What PHP and PHP-FPM
>> Expect These Values To Be?
>>
>> Like a whole slew of:
>>
>> For request: /blag/futo/gtyj.php?qur=kjr
>> SCRIPT_NAME: Should be /gtyj.php
>> PATH_INFO Should be /blag/futo
>> PATH_TRANSLATED Should be /Home/Charlie/public_html/site/blag/futo/
>>
>> or whatever?
>>
>> I think until we have something like this, we will be constantly
>> playing fetch-me-a-rock
>
> The problem is that FPM has very ugly code to second-guess most of
> these values based on other values and its own configuration. I just
> can't follow it. But we know it worked for quite a long time.
>
> We had not been fixing FPM compatibility for a very long time. We got
> into trouble because we tried to clean up some of the variables that
> FPM was already using as markers to ID mod_proxy_fcgi and cleaning up
> itself.
>
> The first of the modern problems was an attempt to make the variables
> consumable by non-FPM fcgi servers. That's the intent of having the
> ProxyFCGIBackendType (or whatever) so we can leave FPM input alone but
> do a better job on variables where they can be more directly verified
> without some overly complex layer second-guessing them.
FWIW, I'm looking thru
https://mail.python.org/pipermail/web-sig/2007-January/002475.html
Anyway, at least now we have in the test framework the ability to
test fcgi and php-fpm, if available, so we can at least ensure no
regressions or, if we like, finally "fix" things so that Apache and PHP
have similar expectations :)
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Eric Covener <co...@gmail.com>.
On Wed, Jun 28, 2017 at 5:50 AM, Jim Jagielski <ji...@jagunet.com> wrote:
> Is there someplace a set of examples on This Is What PHP and PHP-FPM
> Expect These Values To Be?
>
> Like a whole slew of:
>
> For request: /blag/futo/gtyj.php?qur=kjr
> SCRIPT_NAME: Should be /gtyj.php
> PATH_INFO Should be /blag/futo
> PATH_TRANSLATED Should be /Home/Charlie/public_html/site/blag/futo/
>
> or whatever?
>
> I think until we have something like this, we will be constantly
> playing fetch-me-a-rock
The problem is that FPM has very ugly code to second-guess most of
these values based on other values and its own configuration. I just
can't follow it. But we know it worked for quite a long time.
We had not been fixing FPM compatibility for a very long time. We got
into trouble because we tried to clean up some of the variables that
FPM was already using as markers to ID mod_proxy_fcgi and cleaning up
itself.
The first of the modern problems was an attempt to make the variables
consumable by non-FPM fcgi servers. That's the intent of having the
ProxyFCGIBackendType (or whatever) so we can leave FPM input alone but
do a better job on variables where they can be more directly verified
without some overly complex layer second-guessing them.
--
Eric Covener
covener@gmail.com
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jim Jagielski <ji...@jaguNET.com>.
Is there someplace a set of examples on This Is What PHP and PHP-FPM
Expect These Values To Be?
Like a whole slew of:
For request: /blag/futo/gtyj.php?qur=kjr
SCRIPT_NAME: Should be /gtyj.php
PATH_INFO Should be /blag/futo
PATH_TRANSLATED Should be /Home/Charlie/public_html/site/blag/futo/
or whatever?
I think until we have something like this, we will be constantly
playing fetch-me-a-rock
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Champion <ch...@gmail.com>.
On 06/27/2017 04:59 PM, Eric Covener wrote:
> I would just as well pull this block out entirely rather than taking
> the "fpm||" half of the test out. It seems like if you go out of your
> way to run a script with PATH_INFO set as some parameter that we
> shouldn't negate that. And like the non path_info case, nobody has
> ever asked us for this.
Checked in as r1800306 and proposed for backport.
--Jacob
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Eric Covener <co...@gmail.com>.
On Wed, Jun 21, 2017 at 2:00 PM, Jacob Champion <ch...@gmail.com> wrote:
> On 06/20/2017 02:19 PM, Jacob Champion wrote:
>>
>> Here's why I'm asking: if I were to propose the attached patch for
>> backport, what is the test case that *should* fail but doesn't?
>> (proxy_fcgi.t passes, no problem.) And once we get that test case, can we
>> show that it actually addresses a valid PHP-FPM use case, or is it a feature
>> without a user?
>
>
> Any thoughts on this?
I still cannot make any sense of what FPM is doing with our input.
I don't think either of the SCRIPT_NAME transformations make sense
just by the virtue of talking to FPM, primarily because it is neither
historically necessary or demonstrated as a problem.
if (fpm || apr_table_get(r->notes, "virtual_script")) {
/*
* Adjust SCRIPT_NAME, PATH_INFO and PATH_TRANSLATED for PHP-FPM
* TODO: Right now, PATH_INFO and PATH_TRANSLATED look OK...
*/
const char *pend;
const char *script_name = apr_table_get(r->subprocess_env,
"SCRIPT_NAME");
pend = script_name + strlen(script_name);
if (r->path_info && *r->path_info) {
pend = script_name + ap_find_path_info(script_name,
r->path_info) - 1;
}
while (pend != script_name && *pend != '/') {
pend--;
}
apr_table_setn(r->subprocess_env, "SCRIPT_NAME", pend);
ap_log_rerror(APLOG_MARK, APLOG_TRACE4, 0, r,
"fpm:virtual_script: Modified SCRIPT_NAME to: %s",
pend);
}
I would just as well pull this block out entirely rather than taking
the "fpm||" half of the test out. It seems like if you go out of your
way to run a script with PATH_INFO set as some parameter that we
shouldn't negate that. And like the non path_info case, nobody has
ever asked us for this.
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Champion <ch...@gmail.com>.
On 06/20/2017 02:19 PM, Jacob Champion wrote:
> Here's why I'm asking: if I were to propose the attached patch for
> backport, what is the test case that *should* fail but doesn't?
> (proxy_fcgi.t passes, no problem.) And once we get that test case, can
> we show that it actually addresses a valid PHP-FPM use case, or is it a
> feature without a user?
Any thoughts on this?
--Jacob
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Perkins <ja...@cpanel.net>.
This patch has fixed the issues for us. Do you feel this is a production worthy patch, or will there be more movement on it?
—
Jacob Perkins
Product Owner
cPanel Inc.
jacob.perkins@cpanel.net <ma...@cpanel.net>
Office: 713-529-0800 x 4046
Cell: 713-560-8655
> On Jun 20, 2017, at 4:19 PM, Jacob Champion <ch...@gmail.com> wrote:
>
> On 06/20/2017 01:35 PM, Jacob Champion wrote:
>> What requests and server config did you use with this test setup?
>
> Here's why I'm asking: if I were to propose the attached patch for backport, what is the test case that *should* fail but doesn't? (proxy_fcgi.t passes, no problem.) And once we get that test case, can we show that it actually addresses a valid PHP-FPM use case, or is it a feature without a user?
>
> --Jacob
> <remove-fpm.patch>
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Champion <ch...@gmail.com>.
On 06/20/2017 01:35 PM, Jacob Champion wrote:
> What requests and server config did you use with this test setup?
Here's why I'm asking: if I were to propose the attached patch for
backport, what is the test case that *should* fail but doesn't?
(proxy_fcgi.t passes, no problem.) And once we get that test case, can
we show that it actually addresses a valid PHP-FPM use case, or is it a
feature without a user?
--Jacob
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Champion <ch...@gmail.com>.
On 06/23/2017 09:42 AM, Jim Jagielski wrote:
> Over this weekend I may try to extend the current fcgi testing
> to include php-fpm... we should not, imo, fold in any patches
> until we can test each applicable use case and avoid regressions.
I've also added one of the known 2.4.26 regressions to the existing
tests. I'll be going through today and adding cases for the GENERIC
backend as well.
--Jacob
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jim Jagielski <ji...@jaguNET.com>.
Over this weekend I may try to extend the current fcgi testing
to include php-fpm... we should not, imo, fold in any patches
until we can test each applicable use case and avoid regressions.
Hacking on the test will keep my mind off of things...
> On Jun 23, 2017, at 7:52 AM, Jim Jagielski <ji...@jaguNET.com> wrote:
>
> AddType application/x-php7-fpm .php
> Action application/x-php7-fpm /fpm virtual
> <Location /fpm>
> SetHandler proxy:fcgi://localhost:9001
> </Location>
>
> <FilesMatch \.php$>
> SetHandler "proxy:fcgi://localhost:9001
> </FilesMatch>
>
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jim Jagielski <ji...@jaguNET.com>.
AddType application/x-php7-fpm .php
Action application/x-php7-fpm /fpm virtual
<Location /fpm>
SetHandler proxy:fcgi://localhost:9001
</Location>
<FilesMatch \.php$>
SetHandler "proxy:fcgi://localhost:9001
</FilesMatch>
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Champion <ch...@gmail.com>.
On 06/20/2017 12:23 PM, Jim Jagielski wrote:
> % cat fcgi.pl
> #!/usr/bin/env perl
> <snip>
What requests and server config did you use with this test setup?
--Jacob
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jim Jagielski <ji...@jaguNET.com>.
% cat fcgi.pl
#!/usr/bin/env perl
use FCGI;
use Socket;
use FCGI::ProcManager;
use Data::Dumper;
$num_args = $#ARGV + 1;
if ($num_args != 1) {
print "\nUsage: fcgi.pl <socket>\n";
exit 1;
}
$proc_manager = FCGI::ProcManager->new( {n_processes => 1} );
$socket = FCGI::OpenSocket( $ARGV[0], 10 );
$request = FCGI::Request( \*STDIN, \*STDOUT, \*STDERR, \%req_params,
$socket, &FCGI::FAIL_ACCEPT_ON_INTR );
$proc_manager->pm_manage();
if ($request) {
while ( $request->Accept() >= 0 ) {
$proc_manager->pm_pre_dispatch();
print("Content-type: text/plain\r\n\r\n");
print Dumper(\%req_params);
}
}
FCGI::CloseSocket($socket);
% cat fpm.sh
php-fpm70 -p /usr/local2/php-fpm
% ls /usr/local2/php-fpm
docs/ etc/ fcgi.pl* fpm/ fpm.sh* log/ pools/ run/ var/ www/
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Champion <ch...@gmail.com>.
On 06/20/2017 11:48 AM, Jim Jagielski wrote:
> Ahh... the tests from the orig bug report
There have been several reports. I'm hoping to get the test case you
were using to test the new `if (fpm ...` logic in mod_proxy_fcgi. Even
if it was just a manual test; I just want to get it recorded in the suite.
--Jacob
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jim Jagielski <ji...@jaguNET.com>.
Ahh... the tests from the orig bug report
> On Jun 20, 2017, at 2:39 PM, Jacob Champion <ch...@gmail.com> wrote:
>
> SCRIPT_NAME
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Champion <ch...@gmail.com>.
On 06/20/2017 10:55 AM, Jim Jagielski wrote:
> Last I checked, it's in the test framework...
Quick grep for "SCRIPT_NAME" in the tests doesn't turn up anything; can
you point me to it?
--Jacob
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jim Jagielski <ji...@jaguNET.com>.
Last I checked, it's in the test framework...
> On Jun 20, 2017, at 1:42 PM, Jacob Champion <ch...@gmail.com> wrote:
>
> On 06/20/2017 09:47 AM, Jacob Champion wrote:
>> I think. Still trying to context switch back three months.
>
> Jim, do you have a good test case for the current FPM logic so we don't break that with a fix?
>
> --Jacob
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Champion <ch...@gmail.com>.
On 06/20/2017 09:47 AM, Jacob Champion wrote:
> I think. Still trying to context switch back three months.
Jim, do you have a good test case for the current FPM logic so we don't
break that with a fix?
--Jacob
Re: svn commit: r1782209 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Jacob Champion <ch...@gmail.com>.
On 06/20/2017 09:07 AM, Jacob Champion wrote:
> Can we do a quick fixup and reroll before it's too late?
And... what *is* the fixup?
It looks like the logic here -- where we start at the end of SCRIPT_NAME
(or the beginning of PATH_INFO) and work backwards -- is designed to
stop at the first directory separator no matter what, which I completely
missed in my review. It needs to stop wherever the "URL" part of the
path ends and the "filesystem" part of the path begins.
I think. Still trying to context switch back three months.
--Jacob