Re: load order dependency between mod_ldap and mod_authnz_ldap

2011-07-07 Thread Graham Leggett

On 04 Jul 2011, at 6:48 PM, Joe Orton wrote:


It's incumbent on you to provide specific technical objections if
vetoing code, not this hand-waving objections must exist because of  
X.


I have already done so. If you disagree with the objection, or do not  
understand the objection, engage the objection directly so it can be  
resolved.



The removal from APR was because the API does not match design of the
other APIs in APR, because it is an incomplete abstraction -  
mod_ldap

must use the underlying LDAP API directly as well as the wrapper
functions.  This is not something which translates directly across to
httpd; the exported API from mod_ldap was already an incomplete
abstraction, adding more exports does not change that.  Indeed, it
explains the motivation for consolidating this stuff in one place.


We are not discussing the removal of apr_ldap from APR, we are  
discussing the addition of ap_ldap to httpd.


Wrowe's reasons for removing apr_ldap from APR are very valid and  
real, the incomplete abstraction is wrong, and sf has already  
explained the problems that are caused when libraries and/or modules  
compiled with different LDAP library are linked to the same server.  
These problems are not in any shape or form solved by moving apr_ldap  
to httpd, the move serves no purpose whatsoever. Over and above this  
zero nett gain, we now have two nett losses:


- We now suffer an abstraction API in httpd, when all other  
abstraction APIs are in APR. I personally don't care whether the  
abstraction libraries are in APR proper, in apr-util, in apr-ldap or  
httpd-helper-libraries, what I do care is that all abstraction APIs  
should be treated the same way. I have not seen any discussion to  
suggest that there are any plans to move DBD, DBM, crypto and XML, nor  
have I seen anyone justify why we are using an inconsistent approach  
in our architecture.


- We currently build against 7 different LDAP APIs on a multitude of  
different platforms. Without the courtesy of any warning or  
discussion, maintainers of various platforms are now expected to spend  
their time and money making this code build again in the httpd  
codebase. What is the payoff for them? Nothing.


Wrowe has attempted to justify his addition of ap_ldap to httpd by  
saying because httpd would not have been happy if it had abandoned  
these helpers and
they were not present for httpd's consumption, however this statement  
is not true - LDAP is present in apr-util, and will always be present  
as per the versioning rules of APR.


In addition, the versioning rules of apr-util allow us to add to the  
API and in so doing complete the abstraction, solving the problem in  
apr-util v1.4 or v1.5. That makes this entire move unnecessary and a  
huge waste of time.


I am confident that the httpd people will address the APR v2.0 LDAP  
issue, but only after httpd v2.4 is baked, people have time again, and  
APR v2.0 starts becoming something less than vapourware.



If you have specific concerns with exposing all the ap[r]_ldap_* API
from httpd, then that is something we can maybe resolve; it looks like
mod_authnz_ldap and mod_vhost_ldap only require the url_parse hook, so
maybe we could cut the additional exported API back down to that?


A cut down LDAP API is useless to anybody wishing to bind to an LDAP  
API and use its complete functionality, unless the API is complete, we  
are wasting the end user's time.


Long ago we moved away from this portability nonsense, delegating the  
job to APR. Moving this all back to httpd is ugly in the extreme.


You allude to build issues without giving details, what's still  
broken?

(Thanks Stefan for fixing my own screwup)


See at least the following threads:

- Thread ending with [VOTE REVOKED] Re: [VOTE] Release Apache  
httpd-2.3.13 as beta

- httpd LDAP mess

In the case of MacOSX, it breaks for me as follows:

checking whether to enable mod_authnz_ldap... checking dependencies
checking whether to enable mod_authnz_ldap... configure: error:  
mod_authnz_ldap has been requested but can not be built due to  
prerequisite failures


Regards,
Graham
--



Re: [vote] mod_ldap

2011-07-07 Thread Graham Leggett

On 07 Jul 2011, at 10:51 PM, Jim Jagielski wrote:


On Jul 7, 2011, at 2:44 PM, Nick Kew wrote:



On 7 Jul 2011, at 17:55, William A. Rowe Jr. wrote:

[ ]  Retain ap_ldap API's in httpd 2.3 mod_ldap, as currently in  
trunk

   (binding mod_ldap to ldap libs)


+1.   But get it right: not a botch job just because there's
  pressure to release.  Should really have been alpha rather
  than beta when this blew up, but too late to change that.



+1


+1.

Regards,
Graham
--



Re: httpd ldap mess

2011-07-06 Thread Graham Leggett

On 06 Jul 2011, at 1:18 AM, Guenter Knauf wrote:

can you please tell me how it is intended to make mod_authnz_ldap.c  
aware of AP_HAS_LDAP ?
I see you added ap_ldap.h[w|nw] which should define it, but for  
NetWare mod_authnz_ldap.c still bails out:

CC   mod_authnz_ldap.c
### mwccnlm Compiler:
#File: mod_authnz_ldap.c
# --
#  38:  #error mod_authnz_ldap requires LDAP support. To fix add  
--with-ldap to ./configure.

#   Error:   ^
#   preprocessor #error directive

can you please clarify?


I have vetoed the mess you've just referred to, and I expect wrowe to  
revert this change as per this project's rules.


If this is still not done by the end of the week I will make plans to  
do so myself, however I am currently arranging a wedding, and that  
comes first right now.


Regards,
Graham
--



Re: load order dependency between mod_ldap and mod_authnz_ldap

2011-07-06 Thread Graham Leggett

On 07 Jul 2011, at 12:44 AM, Igor Galić wrote:


I have already stated the basis for the veto: every single apparent
flaw in the apr_ldap code that caused wrowe to remove it from APR is
still present in the code that wrowe dumped into httpd. If it's not


It is, fortunately, not in httpd's core. It's in mod_ldap.


It makes no difference in this case whether it's in the core or  
mod_ldap, it is still an incomplete API, and suffers all the flaws we  
suffer right now. What this means practically is that if a module  
wants to limit itself to the caching functionality exposed by mod_ldap  
that module is fine, but if not, that module has to suffer a load  
order problem, and the need to link directly to the LDAP library to  
fill in the blanks (mod_authnz_ldap is forced to do this right now).  
If the end user installs a module binary linked to a different LDAP  
API (mozilla instead of openldap, for example) you get the segfaults  
pointed out by sf.


This change does nothing whatsoever to fix the current problems, and  
performs no purpose.


There is nothing stopping us addressing these problems in apr-util,  
completely within the rules of the APR project, which will fix all our  
problems.



good enough for APR, it is not good enough for httpd, period.


As far as we've figured out so far, mod_ldap is the only consumer.
It might not be good enough for APR or httpd, but it's good enough
for mod_ldap.


mod_authnz_ldap is a consumer of both mod_ldap and the direct LDAP  
API, as is mod_vhost_ldap, and these are just two modules I know about.


People have been writing modules for httpd and it's derivatives for a  
decade and a half, to make a statement like mod_ldap is the only  
consumer is meaningless. People use our code because they trust our  
APIs to remain sensible and stable. This kind of random and badly  
thought out change thrust upon the project at the last minute before a  
GA release is exactly the kind of the change we don't do at the ASF,  
or at httpd.


(Must sleep, more responses over the weekend).

Regards,
Graham
--



Re: httpdl.conf: Load mimal list of modules only

2011-07-06 Thread Graham Leggett

On 07 Jul 2011, at 1:17 AM, Igor Galić wrote:


I agree with Jeff: It would be really good to have only the minimum
list of modules enabled that enable us to support the config we ship


A big +1.

One of the biggest performance enhancing things we've done across our  
estate is switch almost all modules off, and then switch modules on  
only when necessary, like this:


IfModule !mod_cache.c
LoadModule cache_module modules/mod_cache.so
/IfModule

...cache directives...

Regards,
Graham
--



Re: load order dependency between mod_ldap and mod_authnz_ldap

2011-07-04 Thread Graham Leggett

On 04 Jul 2011, at 11:11 AM, Joe Orton wrote:


mod_ldap - An LDAP shared memory cache
mod_authnz_ldap - A user of the LDAP shared memory cache

The LDAP API exposes way more functionality than mod_ldap exposes,
so while you may have fixed the problem for the special case that is
mod_authnz_ldap, you won't have fixed the problem for any other
module that makes LDAP calls directly.


I don't see how this can be the basis of a veto.  You are stating that
there exists a problem which this change does not fix; but that  
problem
existed in the status quo ante, and it *was not the problem this  
change

was intended to fix*.


I have already stated the basis for the veto: every single apparent  
flaw in the apr_ldap code that caused wrowe to remove it from APR is  
still present in the code that wrowe dumped into httpd. If it's not  
good enough for APR, it is not good enough for httpd, period. APR-util  
contains abstraction libraries for LDAP, SQL, dbm, XML, and crypto,  
and now you want to move one abstraction library to an httpd module?  
What were you thinking?


The httpd build is broken on virtually every platform, and is still  
broken more than a month later. That alone is grounds for a veto.


The httpd project will not be held to ransom by a small group of  
people who break the build in an effort to get others to fix it. This  
code is vetoed, remove the code immediately.


Regards,
Graham
--



Re: load order dependency between mod_ldap and mod_authnz_ldap

2011-07-03 Thread Graham Leggett

On 27 Jun 2011, at 4:04 PM, William A. Rowe Jr. wrote:


And I'd nix your definition of mod_ldap... if it an ldap shared cache
provider then it should have been suitably named.  One can omit  
such a
feature and still use mod_authnz_ldap.  Of course, that was not  
possible

because mod_authnz_ldap required mod_ldap required the ldap helpers
required an ldap client library.


mod_ldap is an ldap shared memory cache, arguing about what it should  
have been named is pointless, it doesn't make it any less so.


I expect anybody hacking on the LDAP code to know what it is and  
understand how it works. If you don't have even a basic understanding  
of how the existing code works or is structured, it is highly  
irresponsible to be attempting wholesale changes to it. You can add  
this as yet another reason why I veto your commit.


Please revert your code as soon as possible, to ensure that no more of  
this project's time is wasted.


No.  We didn't agree upon a solution.  We agreed upon a design  
pattern.


Precisely, and despite this agreement, you have taken it upon yourself  
to ignore this design pattern without any justification or  
explanation, repeatedly suggesting and calling for votes out of the  
blue on all sorts of badly thought out alternatives for no good reason.


In the amount of time we have wasted on pointless deadend design  
discussions, I could have fixed this issue ten times over, and so  
could you. I could have also finished outstanding work on apr_crypto,  
apr_dbd, apr_dbm and mod_cache.



I have no expectation that it will be fixed,


The only way this statement could possibly be true is if you had no  
intention of fixing the problem yourself, in the way that was agreed.  
There is no two tier system at the ASF where one group gives orders  
and another group follows them, we all collaborate on this project,  
together. Nothing stopped you picking up the draft RFC, learning how  
the LDAP API worked, and filling in the missing part of the API, and  
yet you chose not to. svn move does not constitute a meaningful  
contribution towards fixing the issues with the LDAP API.



and the majority voted at
APR to evict apr_ldap,


You proposed a vote out of the blue on the APR list to move code to  
httpd, without any prior discussion with either the APR or httpd list.  
You'd obviously discussed this offline, because two people almost  
immediately responded by voting for your move, again without any  
discussion or explanation. Everyone else ignored your thread. The  
httpd project were never notified at all that the vote had taken  
place, never mind their opinions consulted.


As far as procedure is concerned, this vote did not happen.


therefore it is gone (not be veto or fiat, but
by the preference of the majority).  I have reacted accordingly  
because

httpd would not have been happy if it had abandoned these helpers and
they were not present for httpd's consumption.


Did it cross your mind to check with the httpd project first whether  
this assumption was true? I'm sure that the httpd project is overjoyed  
at the fact that the latest beta release failed for every person who  
tried it after you had dumped the code into the tree without any  
warning, any discussion, or any testing. I'm sure we are all  
overwhelmed too at having our builds broken out of the blue on pretty  
much every single platform that httpd runs on because you left the  
build scripts unmodified and untested after the dump.


The httpd project currently depends on and works with the perfectly  
functional apr_ldap interface in the current release of apr-util,  
which supports 7 different LDAP libraries on a host of different  
platforms, and LDAP is not going anywhere in apr-util. There was no  
need to make any panicked rush job import of code into the httpd tree,  
it was entirely unnecessary.


We have an opportunity to complete the missing parts of the apr_ldap  
interface and release apr-util v1.4, and then change mod_ldap and  
friends to use those new interface functions before httpd v2.4 goes out.


This represents the simplest and most efficient solution to the  
original problem, which was people deploying apr linked to one ldap  
library alongside mod_foo_ldap linked to another.


Turning LDAP into a pluggable DBD style API is a desirable addition  
for APR v2.0, but makes no practical difference to httpd v2.4, as APR  
v2.0 is vapourware.


Regards,
Graham
--



Re: load order dependency between mod_proxy and mod_proxy_express

2011-07-03 Thread Graham Leggett

On 02 Jul 2011, at 9:52 PM, Jim Jagielski wrote:

Currently mod_proxy needs to be loaded first because  
mod_proxy_express

references the proxy_module symbol. What is the easiest way to fix
this?



Why is this considered broken??


We have always got away with this in the past because the mod_proxy_*  
modules sort alphabetically after mod_proxy, but this really should be  
fixed.


There should be optional functions for mod_proxy_* modules to access  
the functions of mod_proxy that are needed, I understand this is  
mainly the configuration.


Regards,
Graham
--



Re: load order dependency between mod_ldap and mod_authnz_ldap

2011-06-30 Thread Graham Leggett

On 27 Jun 2011, at 8:29 PM, Stefan Fritsch wrote:


This is fixed by calling the ldap_get_option() function described
in section 9.2 of
http://www-archive.mozilla.org/directory/ietf-docs/draft-ietf-ldap
ext-ldap-c-api-05.txt . There is no need to move the code to
support this, this can be supported today by adding the
appropriate sanity check on init and failing as appropriate.


This does not help us. We need the information encoded in the package
dependencies so that the package manager refuses to install a version
of httpd together with a version of apr that don't work together.



And I don't think you suggestion would work, at least not without a
new API. The problem is this: If apr uses libldap.A and httpd uses
libldap.B, httpd will call apr_ldap_init() which makes apr call
libldap.A's ldap_init(). Apr then passes the resulting LDAP * pointer
to httpd. But httpd will use libldap.B's other ldap_* functions on the
object created by libldap.A. This usually causes segfaults, but it may
also cause more subtle bugs. To detect this issue, both httpd and apr
would need to call ldap_get_option() and httpd would need to compare
the versions returned by both calls.


To fix this, wrap the rest of the LDAP API. This is specified in http://www-archive.mozilla.org/directory/ietf-docs/draft-ietf-ldapext-ldap-c-api-05.txt 
, and should be a straightforward task.


Moving ap(r)_ldap to mod_ldap just means you suffer this problem with  
other packages, like mod_vhost_ldap, you don't solve this problem at  
all.



The problem is that we also need to modify mod_ldap. If we want true
isolation, we would also get #ifdef APR_HAS_SOLARIS_LDAPSDK sections
in mod_ldap, which is what what apr_ldap tried to avoid in the first
place.



This is the wrong way to solve this, here is the full history behind it.

In the early days of LDAP libraries, SSL support was an extremely  
basic affair. Implementers hacked the ldap_init() call in various ugly  
ways to pass a flag that caused ldap_init() to make a simple and  
immediate SSL connection, based on some server wide notion of CA  
certificates and other SSL information. The SSL connection was made  
immediately, and success or failure returned from the ldap_init()  
directly. Call this simple mode.


Over time, LDAP library implementations realised that SSL support is  
configured per connection, do you want SSL or do you want TLS? Do you  
want a specific client or CA certificate for this connection? Instead  
of a single call to a hacked up ldap_init(), now you had a multi-stage  
process, first you ldap_init, then you set any CA and/or client  
certificates with ldap_set_option, and then you set whether the  
connection will be SSL or TLS with ldap_set_option again. At this  
point the connection is completed, and you get success or failure  
after the three steps are complete. Call this multi-stage mode.


Modern LDAP libraries like openldap and mozilla support multi-stage  
mode, while legacy LDAP libraries like the Solaris library only  
support simple mode.


These are two separate and distinct connection patterns that have to  
be allowed for separately.


The apr_ldap API supports both modes. In the apr_ldap API, if you want  
simple mode, you pass the secure parameter to apr_ldap_init(). If  
you want multi-stage mode, you pass the secure parameter via an  
apr_ldap_set_option() call.


Right now, mod_ldap supports multi-stage mode only, it does not yet  
support simple mode, simply because nobody has ever asked mod_ldap  
to support simple mode.


The simplest way to tackle this is to teach mod_ldap to autodetect  
simple mode, in other words if no per connection certificates are  
present, and if secure is SSL (not TLS), use apr_ldap_init() in  
simple mode, otherwise use the existing multi-stage mode.


Any solution that attempts to detect APR_HAS_SOLARIS_LDAPSDK in  
mod_ldap is both unnecessary and wrong, for the reasons you've already  
described, just use the apr_ldap API as it was designed to be used.


Regards,
Graham
--



Re: load order dependency between mod_ldap and mod_authnz_ldap

2011-06-27 Thread Graham Leggett

On 27 Jun 2011, at 12:28 PM, Joe Orton wrote:


This is not so, to fix this, you would need to wrap every single
LDAP API function call[1] in an optional function, and if you did
that, you would solve the problem that caused you to want to remove
apr_ldap from APR in the first place, making the whole exercise
pointless - you may as well just have fixed apr-ldap in place.


mod_authnz_ldap is an existence proof that we don't need to wrap every
LDAP API function call for mod_ldap to be useful in its current form,
surely?


mod_ldap - An LDAP shared memory cache
mod_authnz_ldap - A user of the LDAP shared memory cache

The LDAP API exposes way more functionality than mod_ldap exposes, so  
while you may have fixed the problem for the special case that is  
mod_authnz_ldap, you won't have fixed the problem for any other module  
that makes LDAP calls directly.


The v1.x LDAP API was removed from APR because it was an incomplete  
abstraction, and for this exact same set of reasons it is  
inappropriate to add it to httpd.


Over and above that, given that httpd has no library component common  
to all modules (well, it has, it's called APR), and given that the  
httpd core is inappropriate for an LDAP portability library, the only  
place to put the LDAP bindings is in mod_ldap, which means we either  
have to accept the module ordering problems that result, or we have to  
create optional functions for every single LDAP API call, and if we do  
that, we end up solving the reason apr_ldap was removed from APR in  
the first place.


In addition, we moved all portability code out of httpd in v2.0, it is  
a huge step backwards to go back on that effort. It is really ugly to  
have apr_xml, apr_dbd, apr_crypto and apr_dbm in APR, and then  
ap(r)_ldap somewhere else.


This is the essence of my veto.


The timing cannot be worse


This is probably true, but it doesn't seem like technical  
justification

for a veto.  Release schedules can move at the pace we want them to.


This was an observation, not part of the veto. It's important because  
there are people who are really keen to get v2.4 out the door, and  
this means practically is that a suboptimal API is being rushed  
unnecessarily into GA, and that is really really bad.



There is no need for this move at all


This is strictly true, in the sense that there may be other ways to
solve the problems fixed by moving the code.  If other solutions are
presented, we can have a consensus vote on which to choose.  In the
absence of other solutions being implemented, asserting that better
solutions exist can never be sufficient reason to veto a change.


The solution we agreed on in the past was to rewrite apr_ldap to fit  
the pattern of apr_dbd, but this effort was held up by a whole lot of  
problems in the apr_dbd API that were identified during review of the  
apr_crypto API, which was modeled on the apr_dbd API. This effort was  
also held up by the focus on getting httpd v2.4 out the door.


wrowe got impatient and removed apr_ldap from apr-trunk, which on the  
face of it isn't a problem for httpd because apr-trunk isn't a  
released entity yet, nor is it likely to be until the issues with  
apr_dbd (and as I understand apr_dbm) are fixed (among other issues).  
This move just forces us to solve the apr_ldap issue before apr v2.0  
is released. It doesn't however put httpd under any obligation to take  
any drastic measures for httpd v2.4, because the existing apr_ldap  
library exists and is supported in apr v1.x.


In short, when httpd v2.4 is released, we will be free to focus our  
attention on apr_ldap and get it fixed.


There is no need for any temporary hacks before then (where  
temporary is defined as permanently baked into the httpd v2.4 API).


Regards,
Graham
--



Re: load order dependency between mod_ldap and mod_authnz_ldap

2011-06-26 Thread Graham Leggett

On 26 Jun 2011, at 3:16 PM, Stefan Fritsch wrote:

Nobody said that this would be magically fixed by moving the stuff  
to HTTPD. But it means that the apr libraries are no longer involved  
in the mess, which is already very helpful for distributions like  
Debian. With the apr 1.x situation, an ABI break (soname change) in  
openldap means we have to jump through hoops to prevent crashes when  
httpd uses one version of openldap and apr uses a different version.


This is fixed by calling the ldap_get_option() function described in  
section 9.2 of http://www-archive.mozilla.org/directory/ietf-docs/draft-ietf-ldapext-ldap-c-api-05.txt 
. There is no need to move the code to support this, this can be  
supported today by adding the appropriate sanity check on init and  
failing as appropriate.


I think you misread that comment. The patch attached to the PR makes  
it work with Solaris LDAP but breaks with OpenLDAP, therefore it is  
not acceptable.


Looking at the patch in more detail, the patch makes indiscriminate  
attempts to change the behaviour of all the platforms at the same  
time, instead of targeting the Solaris platform only. OpenLDAP will  
break by definition in this case, as will other platforms.


What the patch needs to do is properly isolate the functions being  
used on Solaris (ldapssl_init()? ldap_sslint()? something else?), and  
modify them only, using #ifdef where appropriate.


But this highlights another problem with how we deal ldap: There is  
no way a developer can test changes on all or even on a significant  
subset of the supported toolkits. This makes it very hard to do any  
changes at all. Ideally we would only support toolkits/plattforms  
that are available at the ASF for testing.


The apr_dbd and apr_crypto APIs already demonstrate how to solve this  
problem, and the plans for apr_ldap in v2.0 are to implement an API  
that follows the same pattern.


The apr_dbd and apr_crypto APIs have had various shortcomings that  
need to be fixed in APR v2.0, and this has held up work on apr_ldap.


Regards,
Graham
--



Re: load order dependency between mod_ldap and mod_authnz_ldap

2011-06-25 Thread Graham Leggett

On 06 Jun 2011, at 11:53 PM, William A. Rowe Jr. wrote:


Since the move from apr-util-ldap to ap_ldap, mod_ldap needs to be
loaded before mod_authnz_ldap. This is somewhat annoying because the
default httpd.conf tries to load mod_authnz_ldap first. Any ideas how
to fix this or do we just change the order in the default httpd.conf?


I believe the entire fix may be an entry point to apr_ldap_parse_uri
(check your own binaries to confirm).  Setting up a single entry point
should be trivial, if its appropriate.


This is not so, to fix this, you would need to wrap every single LDAP  
API function call[1] in an optional function, and if you did that, you  
would solve the problem that caused you to want to remove apr_ldap  
from APR in the first place, making the whole exercise pointless - you  
may as well just have fixed apr-ldap in place.


In it's current form, this change introduces module ordering bugs to  
httpd that we haven't suffered for a decade.


In addition, the autoconf build is currently broken against apr v1.x  
on MacOSX, and this is probably broken on other platforms as well.  
This introduces serious inconvenience for vendors who have to mess  
about trying to make all of this build all over again on all sorts of  
platforms.


The timing cannot be worse - an already suboptimal API plus these new  
bugs are being dumped into httpd in the final stages of trying to bake  
the final version of httpd v2.4.0, which means we will be stuck with  
this brokenness through the life of httpd v2.4.


There is no need for this move at all, as httpd works perfectly  
against APR v1.x (or did until this change). APR v2.x hasn't gone  
through any kind of stabilisation phase, never mind seen an alpha or  
beta release, and so httpd v2.4.x being compatible with apr-trunk at  
this stage is not necessary, especially seeing that when httpd v2.4 is  
released, it's API is set in stone, but APR v2.0's API remains in  
flux. Or to put it another way, the fact that apr_ldap is missing from  
apr-trunk is not a problem for httpd v2.4, and can be solved after  
httpd v2.4.


I am therefore vetoing this move of apr_ldap from APR to httpd.

We need to focus on getting httpd v2.4 out the door before worrying  
about some future version of APR.


[1] 
http://www-archive.mozilla.org/directory/ietf-docs/draft-ietf-ldapext-ldap-c-api-05.txt

Regards,
Graham
--



Re: load order dependency between mod_ldap and mod_authnz_ldap

2011-06-25 Thread Graham Leggett

On 25 Jun 2011, at 11:24 PM, Stefan Fritsch wrote:


This is not so, to fix this, you would need to wrap every single
LDAP API function call[1] in an optional function, and if you did
that, you would solve the problem that caused you to want to
remove apr_ldap from APR in the first place, making the whole
exercise pointless - you may as well just have fixed apr-ldap in
place.


No, that is not correct. You only need to wrap all ap_ldap_* calls and
make the depending modules (mod_authnz_ldap, mod_vhost_ldap, ...) link
to the ldap libraries themselves.


This is one of the key reasons that got apr_ldap evicted from APR in  
the first place.


If the API is not good enough for APR, then the API is not good enough  
for httpd. We must fix this problem, not shift it around.



Moving the ldap_init code from mod_ldap into apr with httpd 2.2.x
broke support for Solaris LDAP (PR 42682), which is a regression from
2.0.x. Fixing that would require changes both in mod_ldap and in apr-
ldap. It says a lot that mod_ldap in 2.2.x does not use
apr_ldap_init() in the way it is recommended in the documentation
(setting secure if ldaps is wanted without client certificates).
Because if it did, it would not work, because the apr_ldap code is
buggy.


Looking at this, the fix involves teaching mod_ldap to pass the  
correct parameter to apr_ldap_ssl_init(), it doesn't require a change  
to the API that I can see.


The long term fix is to hide the LDAP structure within something like  
apr_ldap_t, and then lazy init when necessary. This is why  
apr_ldap_ssl_init() is marked deprecated in v1.x.


To address the comment in the PR about only supporting openssl, we  
support the following distinct LDAP toolkits across a myriad of  
platforms:


- OpenLDAP
- Solaris
- Novell
- Microsoft
- Netscape
- Mozilla
- Tivoli
- z/OS

Do we still work after the API was moved? For MacOSX at least, no, for  
the others, particularly many of the smaller platforms, probably not.



Doing these changes in APR 1.x in a way that does not break an
unmodified 2.2.x mod_ldap with some LDAP library is next to
impossible. So moving the apr_ldap stuff as ap_ldap into httpd and
therefore making it possible to change the API is an advantage.


That's incorrect, it's not possible to change the API - when we  
release httpd v2.4.0, that's the API baked, and you don't get a chance  
fix this.


It is not fair on end users who have waited ages for httpd v2.4.0 to  
suddenly force them to wait even longer while we fix the APIs in APR.  
It is even worse to expect end users to deal with problems caused by  
APIs dumped in at the last minute without a proper stabilisation  
process before we make a major release.


Regards,
Graham
--



Re: svn commit: r916377 - in /httpd/httpd/trunk: CHANGES docs/manual/programs/rotatelogs.xml support/rotatelogs.c

2011-06-20 Thread Graham Leggett

On 20 Jun 2011, at 12:58 PM, Plüm, Rüdiger, VF-Group wrote:


more general
-p mode just added - is it worth keeping?


I think it is worth keeping for those people that only need the link.
Creating a post rotation script that does this seems to be a little
bit of overkill in this case.


+1.

Regards,
Graham
--



Re: 3.0, the 2011 thread.

2011-06-17 Thread Graham Leggett

On 16 Jun 2011, at 10:27 AM, Stefan Fritsch wrote:

I mostly agree with Graham. I propose a hybrid approach. Make the  
MPM and the network/connection filters (this includes ssl) event  
driven and keep the request handling based on threads and workers.


We used openssl to make our non blocking event driven stuff work, and  
it works really well (once you've properly handled SSL_ERROR_WANT_READ  
and SSL_ERROR_WANT_WRITE). There is no reason I can see that would  
stop us using openssl to be async in httpd, we just need to refactor  
the mod_ssl code to actually do it.


The tricky part with event driven code is the really bad support for  
event driven file access. We used libev as our core event loop, which  
does a significant amount of work to make files and sockets work the  
same way in the event loop as best it can in a portable way. Don't  
know of any other event loop that does this. It's difficult trying to  
do the event driven thing if you intersperse event driven socket  
handling with blocking file handling, you end up with many requests  
blocked by an unrelated system call.


One needs to keep a clear separation between we're event driven and  
non blocking code and we're now in worker mode, and are allowed to  
block code.


Regards,
Graham
--



Re: 3.0, the 2011 thread.

2011-06-17 Thread Graham Leggett

On 17 Jun 2011, at 6:14 PM, Paul Querna wrote:

- Existing APIs in unix and windows really really suck at non  
blocking
behaviour. Standard APR file handling couldn't do it, so we  
couldn't use it
properly. DNS libraries are really terrible at it. The vast  
majority of
async DNS libraries are just hidden threads which wrap attempts  
to make
blocking calls, which in turn means unknown resource limits are hit  
when you
least expect it. Database and LDAP calls are blocking. What this  
means

practically is that you can't link to most software out there.



Yes.

Don't use the existing APIs.

Use libuv for IO.

Use c-ares for DNS.

Don't use LDAP and Databases in the Event Loop;  Not all content
generation needs to be in the main event loop, but lots of content
generation and handling of clients should be.


This is where the premise falls down. You can't advertise yourself as  
a generally extensible webserver, and then tell everybody that the  
only libraries they are allowed to use come from a tiny exclusive list.


People who extend httpd will use whatever library is most convenient  
to them, and when their server becomes unstable, they will quite  
rightly blame httpd, they won't blame their own code. There has been  
no shortage of other projects learning this lesson over the last ten  
years.



You are confusing the 'core' network IO model with fault isolation.
The Worker MPM has actually been quite good on most platforms for the
last decade.   There is little reason to use prefork anymore.


In our experience, prefork is still the basis for our dynamic code  
servers. As a media organisation we experience massive thundering  
herds, and so fault isolation for us is a big deal. We certainly don't  
prefork exclusively, just where we need it, but it remains our  
required lowest common denominator.


With load balancers in front of httpd to handle massive concurrent  
connections, having massive concurrent connections in httpd isn't  
necessary for us. We pipe the requests from the load balancers down a  
modest number of parallel keepalive connections, keeping concurrent  
connections to a sane level. Modern multi core hardware is really good  
at ths sort of stuff.


Obviously, one size doesn't fit all, which is why we have mpms.

Yes, an event loop in the core will be an awesome thing to have, but  
we need the option to retain both prefork and worker behaviour, and it  
has to be designed very carefully so that we remain good at being  
reliable.



Should we run PHP inside the core event loop?  Hell no.


Will people who extend our code try to run PHP inside the event loop?  
Hell yes, and this is where the problem lies. We need to design our  
server around what our users will do. It's no use berating users  
afterwards for code they choose to write in good faith.



I think as Stefan aludes to, there is a reasonable middle ground where
network IO is done well in an Event loop, but we can still maintain
easy extendability, with some multi-process and multi-thread systems
for content generators that have their own needs, like file io.

But certain things in the core, like SSL, must be done right, and done
in an evented way.  It'll be hard, but we are programmers after all
aren't we?


I don't understand the problem with SSL - openssl supports  
asynchronous io, we just need to use it.


Regards,
Graham
--



Re: 3.0, the 2011 thread.

2011-06-15 Thread Graham Leggett

On 16 Jun 2011, at 12:01 AM, Paul Querna wrote:

I think we have all joked on and off about 3.0 for... well about 8  
years now.


I think we are nearing the point we might actually need to be  
serious about it.


The web is changed.

SPDY is coming down the pipe pretty quickly.

WebSockets might actually be standardized this year.

Two protocols which HTTPD is unable to be good at. Ever.

The problem is our process model, and our module APIs.


I am not convinced.

Over the last three years, I have developed a low level stream serving  
system that we use to disseminate diagnostic data across datacentres,  
and one of the basic design decisions was that  it was to be lock free  
and event driven, because above all it needed to be fast. The event  
driven stuff was done properly, based on religious application of the  
following rule:


Thou shalt not attempt any single read or write without the event  
loop giving you permission to do that single read or write first. Not  
a single attempt, ever.


From that effort I've learned the following:

- Existing APIs in unix and windows really really suck at non blocking  
behaviour. Standard APR file handling couldn't do it, so we couldn't  
use it properly. DNS libraries are really terrible at it. The vast  
majority of async DNS libraries are just hidden threads which wrap  
attempts to make blocking calls, which in turn means unknown resource  
limits are hit when you least expect it. Database and LDAP calls are  
blocking. What this means practically is that you can't link to most  
software out there.


- You cannot block, ever. Think you can cheat and just make a cheeky  
attempt to load that file quickly while nobody is looking? Your hard  
disk spins down, your network drive is slow for whatever reason, and  
your entire server stops dead in its tracks. We see this choppy  
behaviour in poorly written user interface code, we see the same  
choppy behaviour in cheating event driven webservers.


- You have zero room for error. Not a single mistake can be tolerated.  
One foot wrong, the event loop spins. Step one foot wrong the other  
way, and your task you were doing evaporates. Finding these problems  
is painful, and your server is unstable until you do.


- You have to handle every single possible error condition. Every  
single one. Miss one? You suddenly drop out of an event handler, and  
your event loop spins, or the request becomes abandoned. You have no  
room for error at all.


We have made our event driven code work because it does a number of  
very simple and small things, and it's designed to do these simple and  
small things well, and we want it to be as compact and fast as humanly  
possible, given that datacentre footprint is our primary constraint.


Our system is like a sportscar, it's fast, but it breaks down if we  
break the rules. But for us, we are prepared to abide by the rules to  
achieve the speed we need.


Let's contrast this with a web server.

Webservers are traditionally fluid beasts, that have been and continue  
to be moulded and shaped that way through many many ever changing  
requirements from webmasters. They have been made modular and  
extensible, and those modules and extensions are written by people  
with different programming ability, to different levels of tolerances,  
within very different budget constraints.


Simply put, webservers need to tolerate error. They need to be built  
like tractors.


Unreliable code? We have to work despite that. Unhandled error  
conditions? We have to work despite that. Code that was written in a  
hurry on a budget? We have to work despite that.


Are we going to be sexy? Of course not. But while the sportscar is  
broken down at the side of the road, the tractor just keeps going.


Why does our incredibly unsexy architecture help webmasters? Because  
prefork is bulletproof. Leak, crash, explode, hang, the parent will  
clean up after us. Whatever we do, within reason, doesn't affect the  
process next door. If things get really dire, we're delayed for a  
while, and we recover when the problems pass. Does the server die?  
Pretty much never. What if we trust our code? Well, worker may help  
us. Crashes do affect the request next door, but if they're rare  
enough we can tolerate it. The event mpm? It isn't truly an even mpm,  
it is rather more efficient when it comes to keepalives and waiting  
for connections, where we hand this problem to an event loop that  
doesn't run anyone else's code within it, so we're still reliable  
despite the need for a higher standard of code accuracy.


If you've ever been in a situation where a company demands more speed  
out of a webserver, wait until you sacrifice reliability giving them  
the speed. Suddenly they don't care about the speed, reliability  
becomes top priority again, as it should be.


So, to get round to my point. If we decide to relook at the  
architecture of v3.0, we should be careful to ensure that we don't 

Re: svn commit: r1133582 - in /httpd/httpd/trunk: CHANGES docs/manual/filter.xml docs/manual/mod/mod_data.xml docs/manual/mod/mod_data.xml.meta docs/manual/new_features_2_4.xml modules/filters/config.

2011-06-13 Thread Graham Leggett

On 13 Jun 2011, at 1:11 PM, Paul Querna wrote:


Why is this in the core?

The example in the documentation doesn't make sense, this data encoded
this way is inline, not the whole response.


This was covered in the original thread, which contains links  
explaining what rfc2397 is and what it does:


http://mail-archives.apache.org/mod_mbox/httpd-dev/201105.mbox/%3c53db22b8-5596-4ee8-9ba9-c86dd52c1...@sharp.fm%3e
http://mail-archives.apache.org/mod_mbox/httpd-dev/201106.mbox/%3c201106041955.16737...@sfritsch.de%3E
http://mail-archives.apache.org/mod_mbox/httpd-dev/201106.mbox/%3c751d5e23-32ab-4a1d-aff6-b361d45a4...@sharp.fm%3E

I think an example in the docs using mod_include should make it  
clearer as how to use it out the box, I'll update the docs.



I don't get why this is a filter in httpd core, its a feature of an
app server when its generate its HTML?


In our case we have hundreds and hundreds of separate applications, as  
well as a common skin applied to everything using either php,  
mod_include or varnish ESI. Trying to create an application between a  
file and mod_include et al to create data URLs would have been quite a  
sledgehammer, given that we have a filter stack perfectly suited for  
the task.


Regards,
Graham
--



Re: svn commit: r1133582 - in /httpd/httpd/trunk: CHANGES docs/manual/filter.xml docs/manual/mod/mod_data.xml docs/manual/mod/mod_data.xml.meta docs/manual/new_features_2_4.xml modules/filters/config.

2011-06-09 Thread Graham Leggett

On 09 Jun 2011, at 9:16 AM, Ruediger Pluem wrote:


+/* make sure we don't read more than 6000 bytes at a time */
+apr_brigade_partition(bb, (APR_BUCKET_BUFF_SIZE / 4 * 3),  
e);


Shouldn't we move this below the the checking for the metadata bucket?
Why partitioning again, when we only move a metadata bucket between  
the brigades?


It was an attempt to reuse e, but it's better if it's done later as  
you say.



+/* metadata buckets are preserved as is */
+if (APR_BUCKET_IS_METADATA(e)) {
+/*
+ * Remove meta data bucket from old brigade and insert  
into the

+ * new.
+ */
+APR_BUCKET_REMOVE(e);
+APR_BRIGADE_INSERT_TAIL(ctx-bb, e);
+continue;


Why do we ignore FLUSH buckets here? Shouldn't we handle them  
similar to EOS except for the removal of

the filter?


I understood that flush buckets were metadata buckets, and I checked  
and it doesn't seem to be the case.


We can pass flush buckets, but it needs to be with the understanding  
that we can't flush the overflow value, as that would cause the  
base64 to be terminated, and then continued.


Metadata buckets from the origin brigade that come after an EOS  
bucket are copied to ctx-bb but

silently dropped. Shouldn't we have a

if ((APR_SUCCESS == rv)  (!APR_BRIGADE_EMPTY(ctx-bb))) {
   rv = ap_pass_brigade(f-next, ctx-bb);
}

here for safety reasons?


We do, passing bb rather than ctx-bb, to ensure we've consumed  
everything. When we leave for the last time, we step out of the way,  
so any buckets that follow will bypass us completely.


All done in r1134130.

Regards,
Graham
--



Re: Possible unintiialized variable usage in mod_data.c

2011-06-09 Thread Graham Leggett

On 09 Jun 2011, at 4:06 PM, Chris Wilson wrote:


We recently started using Sentry (static analysis tool) to analyze
apache httpd on a nightly basis. Sentry found a potential unintialized
variable in mod_data.c added in commit 1133582.


Indeed it is - not sure why -Wall didn't catch this.

Fixed in r1134132.

Regards,
Graham
--



Re: An rfc2397 filter for httpd (data URI scheme)

2011-06-06 Thread Graham Leggett

On 04 Jun 2011, at 7:55 PM, Stefan Fritsch wrote:


Would anyone object to adding a simple filter able to encode
rfc2397 to httpd?

http://tools.ietf.org/html/rfc2397
http://en.wikipedia.org/wiki/Data_URI_scheme


I think this would be nice for mod_include. Did you have any other
consumers in mind?


mod_include would be the most obvious consumer for a vanilla httpd  
installation. I was asked to build one of these, and I think it is  
small and simple enough to build it and donate it to httpd. Where we  
want to use it is from php, a call to a restful URL that returns the  
base64 data URL and gets included in the page, and means that our php  
people don't have to reinvent rfc2397 wrapping over and over again.  
You just drop in the filter on a view of your source images and you're  
done.


I'm keen to see us offer more useful filters to our collection, it is  
one of httpd's killer features.


Regards,
Graham
--



Re: Per request DocumentRoot

2011-06-06 Thread Graham Leggett

On 06 Jun 2011, at 10:59 PM, Stefan Fritsch wrote:


These should not be changed for now. The core only translates relative
to the real document root. Everything else is done by modules.

If we ever allow to set the DocumentRoot inside Location blocks,
this may need to be re-evaluated.


That might be nice to have, you could avoid having to Alias everything.

Regards,
Graham
--



Re: mod_filter and adding to the Vary header in 2.2.x

2011-05-31 Thread Graham Leggett

On 31 May 2011, at 2:19 PM, Stefan Fritsch wrote:


if I use mod_filter to configure mod_deflate like this:

BrowserMatch ^Mozilla/4 no-gzip
BrowserMatch \bMSIE [7-9] !no-gzip
Header append Vary User-Agent env=!dont-vary
FilterDeclare compress-response
FilterProvider compress-response DEFLATE Content-Type $text/
FilterProvider compress-response DEFLATE Content-Type
$application/x-javascript
FilterProtocol compress-response change=yes;byteranges=no

How can I make sure in 2.2.x that the vary header is only added if the
DEFLATE filter is actually inserted into the filter chain?


Surely the DEFLATE filter should be responsible for creating a Vary  
header, and adding itself to a Vary header if one already exists?


Or am I misunderstanding you?

Regards,
Graham
--



An rfc2397 filter for httpd (data URI scheme)

2011-05-26 Thread Graham Leggett

Hi all,

Would anyone object to adding a simple filter able to encode rfc2397  
to httpd?


http://tools.ietf.org/html/rfc2397
http://en.wikipedia.org/wiki/Data_URI_scheme

Regards,
Graham
--



Re: [PR #51256] Memory consumption by parent process at sort_hook function

2011-05-24 Thread Graham Leggett

On 24 May 2011, at 2:20 PM, Yehezkel Horowitz wrote:

I have noticed that sort_hook function (in apr_hooks.c) doesn't  
destroy temporary pool.


This leads to a memory consumption of ~500K (=68 hooks * 8K) per  
PROCESS!


Since the sorted hooks are memcpy'ed to another pool anyway, no one  
needs this pool after the function return.


So the resolution is very simple - destroy this pool after usage  
(before returning the sorted hooks array).


I have tested this solution in my environment, without any problems.

What do you think about this?


Definitely sounds sensible. Do you have a patch so we can take a look?

Regards,
Graham
--



Re: svn commit: r1103315 - /httpd/httpd/trunk/modules/filters/mod_deflate.c

2011-05-23 Thread Graham Leggett

On 23 May 2011, at 5:57 PM, Justin Erenkrantz wrote:


mod_dav uses r-output_filters - but, the pointer never gets updated
when it is the first one in the chain.  Hence, we call mod_deflate all
the time even on a request that can't support it - so we have to avoid
repeated memory allocations in that case.  -- justin


This sounds like a general problem though, are other modules  
potentially affected?


Regards,
Graham
--



Re: ap_regexec API for buffers (not NULL terminated strings)

2011-05-19 Thread Graham Leggett

On 19 May 2011, at 1:06 PM, Yehezkel Horowitz wrote:

Can anyone explain why ap_regexec can take only NULL terminated  
string?


I’m working in filter context and want to run regular expression on  
bucket content (so I have the buffer length).


Currently I had to copy the bucket content and add the NULL at the  
end before passing it to ap_regexec (this is a waste of CPU and  
memory).


The underling PCRE engine support getting the length of the input  
string (actually ap_regexec just run strlen on the string and pass  
the result to pcre_exec)


Can you consider adding API for this? (Of course it will get the  
input length as argument)


Similar request (from 2002) could be found at http://www.mail-archive.com/dev@httpd.apache.org/msg12986.html 
 but without any reply.


Sounds like a sensible thing to have.

Is there a patch for this anywhere that we can take a look at, or is  
this just a suggestion at this point?


Regards,
Graham
--



Re: ap_regexec API for buffers (not NULL terminated strings)

2011-05-19 Thread Graham Leggett

On 19 May 2011, at 1:24 PM, Yehezkel Horowitz wrote:


I have a patch (based on 2.2.17) to where should I submit it?


Add it to bugzilla so it doesn't get lost, and then ping here, so  
someone can pick it up.


Ideally, the patch should apply to httpd-trunk first, but having a  
v2.2 patch also (assuming the trunk one doesn't apply as is) makes it  
easier to propose for backport.


Regards,
Graham
--



Re: mod_include and ap_expr

2011-05-15 Thread Graham Leggett

On 15 May 2011, at 12:51 PM, Stefan Fritsch wrote:

The mod_include expression parser tries hard to limit what can be  
done. For example, the subrequest operator -A can be switched of  
with a config option.


If it makes your life easier to remove this config option please do -  
it was only put there to make it possible to backport the -A option to  
v2.2 while guaranteeing no existing configs could break. In v2.4 this  
option doesn't make much sense.


Regards,
Graham
--



Re: mod_include and ap_expr

2011-05-15 Thread Graham Leggett

On 15 May 2011, at 1:22 PM, Stefan Fritsch wrote:

So you implemented it more as a safeguard against confusion with - 
A strings in existing expressions than as a security measure?


Yes.

Do you think that untrusted shmtl files are not a common use case?  
In that case I would tend to the people can always switch back to  
the old restricted expression syntax solution.


I don't follow what you mean by untrusted shtml files?

What the -A option does is say if this particular request for this  
URL would succeed should this particular user attempt to access this  
particular URL directly, then show this data. Or in English, you  
would use the -A option within a page to show or hide links to  
something in a page depending on whether that person has access to  
that link.


For example, to hide the link to JIRA from those that don't have  
access to JIRA, do this:


!--#if expr=-A /jira/--
trtda href=/jira/secure/Dashboard.jspa?os_authType=basicJIRA/ 
a/tdtdli/td/tr

!--#endif--

It works the exact same way that mod_autoindex works, which also sets  
up subrequests to answer the question should I display this  
particular file in the directory listing. If the subrequest returns  
some kind of error (= 400), the module goes oh well, access to that  
file not permitted, will leave it off the list.


Regards,
Graham
--



Re: mod_include and ap_expr

2011-05-15 Thread Graham Leggett

On 15 May 2011, at 3:18 PM, Stefan Fritsch wrote:

Maybe the -A option was a bad example, then, because it allows only  
access to resources that can be viewed directly, too. But ap_expr  
would allow things like


!--#if expr=file('/etc/passwd') =~ /.../ 

This only allows to leak one bit of the file contents per request,  
but if used often enough, it could be used to reconstruct the whole  
file. For .htaccess, this is not a new problem (see SSLRequire), but  
for shtml files, it would be.


Hmmm...

In the mod_include case, having file() without having the file going  
through the normal httpd subrequest mechanism to determine whether the  
user has access to the file is indeed a security problem. The simplest  
would be to perhaps define a restricted mode for ap_expr, which  
disallowed certain dangerous functions.


You would enable restricted mode if you were parsing shtml,  
or .htaccess, but leave restricted mode disabled otherwise. Does that  
sound sensible?


Regards,
Graham
--



Re: add chkdigest.pl to download.xml

2011-05-15 Thread Graham Leggett

On 15 May 2011, at 10:26 PM, Guenter Knauf wrote:


I'd like to add:
http://people.apache.org/~fuankg/chkdigest/
as a cross-platform tool for verifying checksums to the last section  
on download.xml - any thoughts?


The simplest way to check the checksum is to, using the operating  
system of choice, copy the output from md5sum (or local tool of  
choice) on the binary downloaded, and paste this into the find  
functionality of the web browser of choice while displaying the md5  
checksum of the file as published by us. If the md5 hashes match, the  
find will be successful.


This mechanism relies on software already present on and trusted by  
the end user's computer (within reason), and is simple to understand  
without introducing trust on a new tool.


Regards,
Graham
--



Re: [VOTE] Release httpd 2.3.12 as beta

2011-05-14 Thread Graham Leggett

On 14 May 2011, at 12:54 PM, William A. Rowe Jr. wrote:


We should /not/ be halting a *beta* when one platform, one feature, or
any other single documented issue has an issue.  Versions and releases
are cheap, release it and get on with the next beta :)

The windows issue is a non-issue (cruft in the bin/ directory, oh  
well)

and the apu issue affects users who configure auth_ldap, right?  Betas
are not meant to be perfect, and they are no fun if there are no bugs.


+1.

Regards,
Graham
--




Re: svn commit: r1103015 - /httpd/httpd/trunk/STATUS

2011-05-14 Thread Graham Leggett

On 15 May 2011, at 1:46 AM, William A. Rowe Jr. wrote:

No argument, but there are 1) minor quibbles with the apr-2  
interface, and
2) some significant work to replace the original with the new  
interface, and
not sure who has cycles to attack this in the near term.  If it is  
fixed,
re-adding mod_session during 2.4.x cycle would be relatively  
painless, no?


The only module affected is mod_session_crypto.c, I'm not sure how the  
scope has expanded to cover mod_session*.


Taking out mod_session* would also mean taking out mod_auth_form,  
which in turn would be a lot of work, and would take out a major new  
feature of v2.4. It would be a far better use of our time just taking  
out mod_session_crypto.c if it had to come down to it.


Full docs are here:

http://httpd.apache.org/docs/trunk/mod/mod_auth_form.html
http://httpd.apache.org/docs/trunk/mod/mod_session.html
http://httpd.apache.org/docs/trunk/mod/mod_session_cookie.html
http://httpd.apache.org/docs/trunk/mod/mod_session_dbd.html
http://httpd.apache.org/docs/trunk/mod/mod_session_crypto.html

Regards,
Graham
--



Re: svn commit: r1098105 - /httpd/httpd/branches/2.2.x/STATUS

2011-05-07 Thread Graham Leggett

On 30 Apr 2011, at 2:22 PM, traw...@apache.org wrote:


  * mod_cache: Realign the cache_quick_handler() to behave identically
to the default_handler() when reacting to errors when writing to  
the

@@ -132,6 +132,8 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
Trunk patches: http://svn.apache.org/viewvc?view=revisionrevision=1003913
2.2.x patch: 
http://people.apache.org/~minfrin/httpd-mod_cache-errorfix-22.patch
+1: minfrin
+trawick: any reason it shouldn't be completely aligned with  
default_handler's

+ choice to return OK vs. 500?


Hmmm...

In the cache, we care about AP_FILTER_ERROR, and pass that back if  
present. In the default handler, we ignore AP_FILTER_ERROR completely,  
and instead check for this following:


if (status == APR_SUCCESS
|| r-status != HTTP_OK
|| c-aborted) {
return OK;
}

I'm not sure whether merging these is safe enough to backport?

The problem the original patch solves as that an APR error is  
appearing in the access_log, and that has caused much confusion for  
some people: http://www.google.co.uk/search?q=http+error+103


Ideally, this should be fixed at the very least, and we can then look  
at aligning the behaviour to be closer matched. Does this make sense,  
or am I being excessively paranoid?


Regards,
Graham
--





Re: How to do a sub-request from input filter?

2011-04-27 Thread Graham Leggett

On 27 Apr 2011, at 7:07 PM, Micha Lenk wrote:

I am using Apache as a reverse proxy to forward requests to a  
backend web
server protected by some self-written Apache modules. Now I would  
like to do a

sub-request to a different location from within an input filter.

What is the easiest way to achieve this? Reading Nick Kew's  
excellent Apache
Modules Book I've found the function ap_internal_redirect(), but  
does this
function also work from an input filter? Currently I doubt it  
because the first
line of the HTTP request for the backend web server is built very  
early in
ap_proxy_http_request(), apparently before running the input  
filters...

Do you have any better suggestion?

What else should I keep in mind in this situation?


It depends, do you want the subrequest to replace the main request? Or  
do you want the subrequest to happen, and then for the main request to  
continue as it was before?


Regards,
Graham
--



Re: blocking Upgrade

2011-03-30 Thread Graham Leggett

On 30 Mar 2011, at 10:49 AM, Roy T. Fielding wrote:


On Mar 29, 2011, at 11:16 PM, Greg Stein wrote:


Do you have an internet draft spec for some context here? Is there a
proposal for HTTP/2.0?


websockets


In theory, over and above natively supporting websockets, it may be  
useful to teach mod_proxy how to reverse proxy the ws scheme to  
another websockets server. Would a module like mod_proxy_websockets or  
mod_proxy_ws make sense?


Regards,
Graham
--



Re: A timestamp for mod_log_forensic (?)

2011-03-30 Thread Graham Leggett

On 30 Mar 2011, at 3:23 PM, Christian Folini wrote:


Mod_log_forensic is saving my day while debugging a crashing
apache. But matching the right request with the crash and its
corefile is difficult.


Have you taken a look at Jeff's mod_whatkilledus?

http://people.apache.org/~trawick/exception_hook.html

Regards,
Graham
--



Re: blocking Upgrade

2011-03-30 Thread Graham Leggett

On 30 Mar 2011, at 3:53 PM, Roy T. Fielding wrote:


No, websockets is not designed to work with intermediaries.
There is no standard behavior beyond opening the connection,
so connections through proxies should use CONNECT.


Does a websocket client have a way of knowing it should use CONNECT?

Regards,
Graham
--



Re: blocking Upgrade

2011-03-30 Thread Graham Leggett

On 30 Mar 2011, at 4:41 PM, Roy T. Fielding wrote:


My guess is that it would if it were told to use a proxy for ws.

Keep in mind that when I say proxy, I do not mean to include  
reverse proxy.
A reverse proxy of websockets is just an implementation of  
websockets or

a tunnel.  I consider both to be pretty dangerous and better done in
a special-purpose server rather than as a child of Apache httpd.


When you say pretty dangerous, are you referring to the danger of  
reaching the limit of slots willing to be served by the server, or  
something else?


I would say that any server that supports the idea of long lived  
connections faces this danger, I don't see why httpd (suitably and  
sensibly configured obviously, probably with an event mpm and a limit  
as to the number of slots we allow to be long lived) can't play too.


Regards,
Graham
--



Re: blocking Upgrade

2011-03-30 Thread Graham Leggett

On 30 Mar 2011, at 5:48 PM, Greg Stein wrote:


I think that Roy's point is simply that httpd would be nothing more
than a socket-listener and tunnel. There is very little that it can
bring to the table at that point, so it doesn't make a lot of sense to
lump in websockets capabilities.


In theory, for somebody it might be a socket listener and a tunnel,  
using the same HTTP endpoint as their existing HTTP server, with the  
same port and the same digital certificate, perhaps additionally  
secured by an authn or authz module.


While I don't doubt that for a gateway like this to be practical there  
would be issues to overcome, such as ensuring long lived requests  
cannot starve out other server requests, however I do see some value  
in trying it. Disclaimer: ENOTIME right now, so no plans to do so for  
a while.


Regards,
Graham
--



Re: new ProxyPass/ProxyPassReverse feature for 2.4??

2011-03-28 Thread Graham Leggett

On 29 Mar 2011, at 12:10 AM, Jim Jagielski wrote:


I *think* we're talking the same thing here... you seem to be
focusing on how ProxyPassReverse is currently implemented, which
is horribly slow, but but making ProxyPass automatically handle
the default PPR case means we don't need to use that implementation.


What I did to handle the slow list case for both ProxyPass and  
CacheEnable (which was modeled on ProxyPass originally) was to teach  
the directive to have a once-only per-directory value, which takes  
precedence if set, but reverts to the slow list if unset.


It may be worth doing the same with ProxyPassReverse?

And if nothing matches, we can default to sensible behaviour (ie  
automated ProxyPassReverse). Does this make sense?


Regards,
Graham
--



Re: ldap unreleasable

2011-03-28 Thread Graham Leggett

On 29 Mar 2011, at 12:13 AM, William A. Rowe Jr. wrote:


/* LDAP cache state information */
typedef struct util_ldap_state_t {
...
   int connectionPoolTTL;
} util_ldap_state_t;


I'm continue to grow more worried that the state of ldap in httpd
and in apr enjoys very little granularity, oversight, or quality...

1. Hungarian?  Forgot to eat breakfast that day?  Out of bounds
   per httpd style rules.

2. int?  Really?  This is assigned an apr_interval_time_t in its
   config code.

Please review and fix the style violations, and ensure that timeouts
are doing what they were meant to do.


Please be aware that the person who donated this code to us isn't an  
ASF person, and donated the code in good faith. If it doesn't fit our  
style guide, we should fix it, and show respect for their original  
contribution.


Looks like mod_ssl needs some similar cleanup, there are still style  
violations in there, not limited to the examples below. Now would be a  
good time to overhaul style.


typedef struct {
  ...
} SSLConnRec;

typedef struct {
pid_t   pid;
apr_pool_t *pPool;
BOOLbFixed;

apr_global_mutex_t   *pMutex;
apr_array_header_t   *aRandSeed;
apr_hash_t *tVHostKeys;
void   *pTmpKeys[SSL_TMP_KEY_MAX];

apr_hash_t *tPublicCert;
apr_hash_t *tPrivateKey;

const char *szCryptoDevice;

struct {
void *pV1, *pV2, *pV3, *pV4, *pV5, *pV6, *pV7, *pV8, *pV9,  
*pV10;

} rCtx;
} SSLModConfigRec;

Regards,
Graham
--



Re: new ProxyPass/ProxyPassReverse feature for 2.4??

2011-03-28 Thread Graham Leggett

On 29 Mar 2011, at 1:08 AM, William A. Rowe Jr. wrote:

My suggestion isn't per-dir.  It's a r-notes which is even more  
efficient.


One further thing - it would need to work sanely from within a  
LocationMatch too.


Regards,
Graham
--



Re: mod_fcgid in httpd tarball?

2011-03-23 Thread Graham Leggett

On 19 Mar 2011, at 12:07 AM, William A. Rowe Jr. wrote:


It seems like mod_fcgid has made huge progress and is now in a much
more stable bugfix epoch of it's life, similar to how mod_proxy had
progressed when development was kicked out of core for major http/1.1
rework, and brought back in when a vast percentage of it's bugs had
been addressed.

Do we want to introduce mod_fcgid now into httpd 2.3.x for the next  
beta?


How do we reconcile mod_fcgid with mod_proxy_fcgid?

Regards,
Graham
--



Re: ap_read_config() from remote resource

2011-03-01 Thread Graham Leggett

On 28 Feb 2011, at 10:32 PM, Igor Galić wrote:


I think we discussed such possibilities last year at the retreat
but didn't really follow up on it, so lets assume it's a fresh new
idea which I just thought of :)

ap_read_config() passes the config file it gets more or less
directly to ap_pcfg_openfile() which again pass it more or less
directly to apr_file_open() -- now what if we wanted to place
our configuration files on a remote (HTTP ;) resource. Of course
it sounds silly to boot-strap an HTTP server from an HTTP server
but JBoss for instance supports that -- in part at least: For the
application's properties files.


I looked at this concept a number of years ago, and from the research  
I did it's definitely possible.


Ultimately, we have a mechanism that produces lines, and each line is  
fed into a parser that parses the config. Right now, that mechanism is  
open file, read it in line by line, but there is nothing stopping us  
from making this pluggable.


The idea I had in mind was to teach the Include directive how to  
support URLs, for example like this:


Include http://somewhere/something.conf

or something like this:

Include ldap://some-ldap-url

where the default is this:

Include file:///some-path

AKA

Include /some-path

All we would need to do to start would be to define a suitable hook to  
consume the lines, and then refactor the existing file based code to  
use the hook. At a later date, people can add implementations as and  
when they see fit.


Regards,
Graham
--



Re: Bug #30865 -- mod_disk_cache leaves many temporary files slowing file system

2011-02-27 Thread Graham Leggett

On 27 Feb 2011, at 1:21 PM, Dirk-Willem van Gulik wrote:


Reudiger,

Why is:

https://issues.apache.org/bugzilla/show_bug.cgi?id=30865

still open ? You are not sure it was fixed ? Or we just forgot about  
it ?


This is fixed in httpd-trunk, I suspect it can be closed at this point.

What mod_disk_cache was doing was attempting to clean up after itself  
in certain well defined failure cases, but didn't register a pool  
cleanup to do it. As a result, if a filter decided to fail, such as  
the network filter because a client went away, the temp files were  
left behind and became orphaned.


Regards,
Graham
--



Re: Time to start planning for httpd 2.3.11-BETA ?

2011-02-22 Thread Graham Leggett
On 22 Feb 2011, at 17:13, Jim Jagielski j...@jagunet.com wrote:

 I think we're about ready... My plan is to TR 2.3.11-beta the start
 of next week, allowing this week for some final touches...

Remind me, at what point does the API  freeze?

Regards,
Graham
--



Re: svn commit: r1070075 - in /httpd/httpd/trunk: CHANGES modules/cache/cache_util.c

2011-02-14 Thread Graham Leggett

On 14 Feb 2011, at 9:22 AM, Ruediger Pluem wrote:


What happens if str is supplied as a, b?
I mean why token + 1 and not token?


I guess it's because we know *token isn't a separator, so there is no
point checking if it is one a second time.


*token might not be a separator, but it might be .


Sorry - I thought you were highlighting the comma and space, rather  
than the quotes around it.


You're right, the +1 does skip an opening quote, which is wrong. It's  
fixed in r1070699.


Regards,
Graham
--



Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h

2011-02-13 Thread Graham Leggett

On 13 Feb 2011, at 9:59 AM, Roy T. Fielding wrote:


URL: http://svn.apache.org/viewvc?rev=1070179view=rev
Log:
mod_cache: When a request other than GET or HEAD arrives, we must
invalidate existing cache entities as per RFC2616 13.10. PR 15868.


Cache entries should not be invalidated unless the response comes
back as a success (2xx).  Likewise, it only applies to methods with
known write effects (not including TRACE, OPTIONS, PROPFIND, etc.).

This has already been updated in httpbis p6.


Hmmm...

So is it right to say that POST, PUT and DELETE should invalidate,  
while all others would be left alone?


Or do we need some kind of mechanism for modules that respond to new  
methods to declare those methods as needing to be invalidated or not?


Should the cache try and cache any of the other methods, or is  
cacheability limited to GET only?


Regards,
Graham
--



Re: svn commit: r1070075 - in /httpd/httpd/trunk: CHANGES modules/cache/cache_util.c

2011-02-13 Thread Graham Leggett

On 13 Feb 2011, at 5:22 PM, Ruediger Pluem wrote:


+/* skip characters in sep (will terminate at '\0') */
+while (*str  strchr(sep, *str)) {
+++str;
+}
+
+if (!*str) {/* no more tokens */
+return NULL;
+}
+
+token = str;
+
+/* skip valid token characters to terminate token and
+ * prepare for the next call (will terminate at '\0)
+ * on the way, ignore all quoted strings, and within
+ * quoted strings, escaped characters.
+ */
+*last = token + 1;


What happens if str is supplied as a, b?
I mean why token + 1 and not token?


I guess it's because we know *token isn't a separator, so there is no  
point checking if it is one a second time.


The same pattern exists in apr_strtok.c:

https://svn.apache.org/repos/asf/apr/apr/trunk/strings/apr_strtok.c

Regards,
Graham
--



Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h

2011-02-13 Thread Graham Leggett

On 13 Feb 2011, at 5:08 PM, Ruediger Pluem wrote:


+/*
+ * invalidate a specific URL entity in all caches
+ *
+ * All cached entities for this URL are removed, usually in
+ * response to a POST/PUT or DELETE.
+ *
+ * This function returns OK if at least one entity was found and
+ * removed, and DECLINED if no cached entities were removed.
+ */
+int cache_invalidate(cache_request_rec *cache, request_rec *r)


Why not adjusting cache_remove_url accordingly? It does nearly the  
same and

has the same prototype.


Nearly the same, but not enough. cache_remove_url() only takes effect  
if a previous cached entry was found, while cache_invalidate() is  
unconditional.


As it turns out, invalidating the entry unconditionally is wrong, as  
we need to do the invalidation only on 2xx. This means this code needs  
to be changed, waiting for Roy to confirm the exact behaviour before  
changing it.


Regards,
Graham
--



Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h

2011-02-13 Thread Graham Leggett

On 14 Feb 2011, at 1:56 AM, Paul Querna wrote:


Additionally, this should be a configurable behavior.

Lets say you run a popular website that depends on mod_cache to
protect backend systems from complete overload.

All you need to do now as an attacker is POST / DELETE to / or another
important URL every 200ms, and the cache becomes invalidated, causing
a flood of requests to backends that might not be able to support it.

Thoughts?


How is this different from Cache-Control: no-cache in the request?

Regards,
Graham
--



Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h

2011-02-13 Thread Graham Leggett

On 14 Feb 2011, at 2:15 AM, Paul Querna wrote:


It does a single request to the backend, but doesn't _invalidate_ the
existing cache, which would cause a flood of other, non-attacker
clients to come in.


I think that would be the origin of Roy saying that we should only  
invalidate if the result is 2xx. Someone trying methods in the hope  
they would do something would get a 405 Method Not Supported.


Regards,
Graham
--



Re: svn commit: r1069942 - in /httpd/httpd/trunk: CHANGES modules/cache/cache_util.c

2011-02-11 Thread Graham Leggett

On 11 Feb 2011, at 10:17 PM, Jim Jagielski wrote:


Changes with Apache 2.3.11

-  *) mod_ssl: Fix a possible startup failure if multiple SSL vhosts
- are configured with the same ServerName and private key file.
- [Masahiro Matsuya mmatsuya redhat.com, Joe Orton]
+  *) mod_cache: Respect s-maxage as described by RFC2616 14.9.3,  
which must

+ take precedence if present. PR 35247. [Graham Leggett]

 *) mod_socache_dc: Make module compile by fixing some typos.
PR 50735 [Mark Montague mark catseye.org]



Why the removal of the mod_ssl entry?


Eek, how did that happen. Fixed in r1069947.

Regards,
Graham
--



Bug 50199 - ap_cache_liststr fails to parse quoted strings

2011-02-11 Thread Graham Leggett

Hi all,

The current parser for the Cache-Control header doesn't take into  
account quoted-string extensions to the header.


To fix this, I have created a modified implementation of apr_strtok()  
called cache_strqtok(), that tokenises strings, but ignores the quoted  
part of the strings, leaving them intact.


My question is, does cache_strqtok() belong in cache_util.c, as per  
the patch below, or would it be a better idea to add it to apr  
alongside apr_strtok() as apr_strqtok()?


Index: modules/cache/cache_util.c
===
--- modules/cache/cache_util.c  (revision 1069969)
+++ modules/cache/cache_util.c  (working copy)
@@ -27,6 +27,8 @@

 extern module AP_MODULE_DECLARE_DATA cache_module;

+#define CACHE_SEPARATOR ,   
+
 /* Determine if url matches the hostname, scheme and port and path
  * in filter. All but the path comparisons are case-insensitive.
  */
@@ -1022,6 +1024,74 @@
 }

 /**
+ * String tokenizer that ignores separator characters within quoted  
strings

+ * and escaped characters, as per RFC2616 section 2.2.
+ */
+static char *cache_strqtok(char *str, const char *sep, char **last)
+{
+char *token;
+int quoted = 0;
+
+if (!str) { /* subsequent call */
+str = *last;/* start where we left off */
+}
+
+/* skip characters in sep (will terminate at '\0') */
+while (*str  strchr(sep, *str)) {
+++str;
+}
+
+if (!*str) {/* no more tokens */
+return NULL;
+}
+
+token = str;
+
+/* skip valid token characters to terminate token and
+ * prepare for the next call (will terminate at '\0)
+ * on the way, ignore all quoted strings, and within
+ * quoted strings, escaped characters.
+ */
+*last = token + 1;
+while (**last) {
+if (!quoted) {
+if (**last == '\') {
+quoted = 1;
+++*last;
+}
+else if (!strchr(sep, **last)) {
+++*last;
+}
+else {
+break;
+}
+}
+else {
+if (**last == '\') {
+quoted = 0;
+++*last;
+}
+else if (**last == '\\') {
+++*last;
+if (**last) {
+++*last;
+}
+}
+else {
+++*last;
+}
+}
+}
+
+if (**last) {
+**last = '\0';
+++*last;
+}
+
+return token;
+}
+
+/**
  * Parse the Cache-Control and Pragma headers in one go, marking
  * which tokens appear within the header. Populate the structure
  * passed in.
@@ -1043,7 +1113,7 @@

 if (pragma_header) {
 char *header = apr_pstrdup(r-pool, pragma_header);
-const char *token = apr_strtok(header, , , last);
+const char *token = cache_strqtok(header, CACHE_SEPARATOR,  
last);

 while (token) {
 /* handle most common quickest case... */
 if (!strcmp(token, no-cache)) {
@@ -1053,14 +1123,14 @@
 else if (!strcasecmp(token, no-cache)) {
 cc-no_cache = 1;
 }
-token = apr_strtok(NULL, , , last);
+token = cache_strqtok(NULL, CACHE_SEPARATOR, last);
 }
 cc-pragma = 1;
 }

 if (cc_header) {
 char *header = apr_pstrdup(r-pool, cc_header);
-const char *token = apr_strtok(header, , , last);
+const char *token = cache_strqtok(header, CACHE_SEPARATOR,  
last);

 while (token) {
 switch (token[0]) {
 case 'n':
@@ -1178,7 +1248,7 @@
 break;
 }
 }
-token = apr_strtok(NULL, , , last);
+token = cache_strqtok(NULL, CACHE_SEPARATOR, last);
 }
 cc-cache_control = 1;
 }

Regards,
Graham
--



Re: svn commit: r1067178 - /httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c

2011-02-04 Thread Graham Leggett

On 04 Feb 2011, at 3:50 PM, j...@apache.org wrote:


URL: http://svn.apache.org/viewvc?rev=1067178view=rev
Log:
And yet more balancer params that can be changed at runtime via
the b-m application...

next up, of course, is adding new workers ;)


If it became possible to add workers on the fly in response to long  
running multicast DNS requests (zeroconf), it would be made of awesome.


(ENOTIME right now)

Regards,
Graham
--



Re: svn commit: r1067178 - /httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c

2011-02-04 Thread Graham Leggett

On 04 Feb 2011, at 6:14 PM, Jim Jagielski wrote:

Right now, the idea is for Apache to respond to simple GET requests,  
ala
balancer-manager, to add workers. Of course, anything authorized  
could

send those GET requests ;) ;)


We've finished zeroconf-ing one inhouse application we use, and we're  
keen to do the same for httpd at some point as a possible balancer  
module. As long as there was some or other function we could call  
(optional or otherwise) when a worker was to be added or removed, that  
would be enough for us.


Regards,
Graham
--



Re: svn commit: r1059910 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_io.c

2011-01-17 Thread Graham Leggett

On 17 Jan 2011, at 3:14 PM, jor...@apache.org wrote:


Author: jorton
Date: Mon Jan 17 13:14:21 2011
New Revision: 1059910

URL: http://svn.apache.org/viewvc?rev=1059910view=rev
Log:
* modules/ssl/ssl_engine_io.c: Revamp output buffering: add a
 coalesce filter which buffers the plaintext, and remove buffering
 of the SSL records -- i.e. buffer before the SSL output filter,
 rather than after it.  This aims to reduce the network overhead
 imposed by the output of many small brigades (such as produced by
 chunked HTTP response), which can now be transformed into a few
 large TLS records rather than many small ones.


Is this not a duplicate of the BUFFER filter in mod_buffer?

Regards,
Graham
--



Re: svn commit: r1059910 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_io.c

2011-01-17 Thread Graham Leggett

On 17 Jan 2011, at 4:35 PM, Joe Orton wrote:


Is this not a duplicate of the BUFFER filter in mod_buffer?


Ah, I forgot that was in the tree.  It is similar, but that's a  
content

filter which requires manual user configuration, this is a
connection-level filter which does not.  Yes, it would certainly be  
very

nice to have a more abstract buffering filter with an API.


True.

What I've had in mind for a while is a connection level buffering  
filter that allows the backend request to complete and for r-pool to  
be destroyed (and all resources released), while the frontend takes  
its time consuming the data. I suspect that buffering filters are  
simple enough that trying to optimise them may be over the top.


Regards,,
Graham
--



Re: svn commit: r1056448 - /httpd/httpd/trunk/docs/manual/mod/mod_log_config.xml

2011-01-08 Thread Graham Leggett

On 07 Jan 2011, at 9:05 PM, William A. Rowe Jr. wrote:

If caution is to be exercised for other reasons, such as losing  
data in the event of a

crash, that's a separate issue, and should be handled separately.


It should be used with caution.  Instead of arguing, replace the  
caution.

Or I'm happy to revert.



I've got a better idea. Why not collaborate with the other people in  
this project instead of attempting to bully them with the threat of  
yet-another-veto? Use rpluem's email as an example of what to do, he  
suggested a potential alternative warning, and invited others to chime  
in with an improvement. This resulted in r1056693, and a resolution to  
the issue.


Regards,
Graham
--



Re: svn commit: r1056448 - /httpd/httpd/trunk/docs/manual/mod/mod_log_config.xml

2011-01-07 Thread Graham Leggett

On 07 Jan 2011, at 8:44 PM, William A. Rowe Jr. wrote:


On 1/7/2011 12:37 PM, minf...@apache.org wrote:

--- httpd/httpd/trunk/docs/manual/mod/mod_log_config.xml (original)
+++ httpd/httpd/trunk/docs/manual/mod/mod_log_config.xml Fri Jan  7  
18:37:25 2011

@@ -345,9 +345,6 @@
efficient disk access and hence higher performance.  It may be
set only once for the entire server; it cannot be configured
per virtual-host./p
-
-noteThis directive is experimental and should be used with
-caution./note


Did you eliminate all cautions?  E.g. that a server crash may neglect
to complete a final write of possibly relevant clues leading up to the
crash?


The original caution stated that caution should be exercised because  
the directive is experimental.


If caution is to be exercised for other reasons, such as losing data  
in the event of a crash, that's a separate issue, and should be  
handled separately.


Regards,
Graham
--



Re: svn commit: r1055250 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

2011-01-05 Thread Graham Leggett

On 05 Jan 2011, at 9:43 AM, Ruediger Pluem wrote:

What about the comment in mod_proxy.h about the r element of  
proxy_conn_rec?


/* Request record of the frontend request
* which the backend currently answers. */

Doesn't this comment need to be adjusted now?


It does - I've fixed it in r1055367.

-rp-headers_in = apr_table_copy(r-pool, r- 
headers_out);
+backend-r-headers_in = apr_table_copy(backend-r- 
pool, r-headers_out);


Doesn't that mean that we will get entries in backend-r-headers  
that have been allocated

from r-pool instead of backend-r-pool?


Hmmm... it does yes, I think we need an apr_table_clone here.


/*
 * Restore Transfer-Encoding header from response if we  
saved

 * one before and there is none left. We need it for the
 * ap_http_filter. See above.
 */
-if (te  !apr_table_get(rp-headers_in, Transfer- 
Encoding)) {
-apr_table_add(rp-headers_in, Transfer-Encoding,  
te);
+if (te  !apr_table_get(backend-r-headers_in,  
Transfer-Encoding)) {
+apr_table_add(backend-r-headers_in, Transfer- 
Encoding, te);


Doesn't that mean that we will get entries in backend-r-headers  
that have been allocated
from r-pool instead of backend-r-pool? te is an element from r- 
headers_out.


In this case we shouldn't, as apr_table_add() makes a copy of te from  
backend-r-pool. If we'd used apr_table_addn(), then it would have  
been yes.


Regards,
Graham
--



Re: SetVirtualDocumentRoot / per request document root / context root?

2010-12-13 Thread Graham Leggett

On 13 Dec 2010, at 5:11 AM, William A. Rowe Jr. wrote:


An idea from left field.

Is there a reason that DocumentRoot is a virtual host wide setting?


Yes, root describes /, there is only one Location /.


I can understand why such a setting would be required, but I still  
don't see why that requirement binds us to it being set virtual host  
wide.


You could either treat the lack of a DocumentRoot defined at the /  
level as a fatal error that prevents the server from starting, or you  
could fall back on a precompiled default directory.


In theory, the only reason I can see for requiring it virtual host  
wide is if the value is required before the location walk. Is it?



Does it not make sense to make it per-Location instead?


No, but MountFilePath /path/to/foo in a Location basis makes sense.


What I had in mind was:

DocumentRoot /somewhere
Location /bar
  DocumentRoot /somewhereelse
/Location

which would be equivalent to MountFilePath.


You might think I just said the same thing, but I had brought this up
before back around the beginnings of 2.0.  DocumentRoot is an awfully
ancient and limiting construct.  Some suggest Alias is the same thing
(which it is, in one sense), but it seems like we could fold both into
a single concept.

The reason you are still stuck with legacy docroot constructs is  
CGI, etc.


I don't mind being stuck with legacy docroot constructs, however I do  
see a different way of implementing them though, and I entirely agree  
on the assessment that it is ancient and limiting, which is why I am  
exploring the options by asking what if.


Regards,
Graham
--



Crash inside worker mpm's ap_mpm_pod_check()

2010-12-13 Thread Graham Leggett

Hi all,

I am currently trying to get to the bottom of a crash that is  
occurring under load (during an Avalanche load test, most  
specifically) inside the worker mpm, with the stacktrace as below.


Most specifically, it segfaults inside the read() below:

AP_DECLARE(int) ap_mpm_pod_check(ap_pod_t *pod)
{
char c;
apr_os_file_t fd;
int rc;

/* we need to surface EINTR so we'll have to grab the
 * native file descriptor and do the OS read() ourselves
 */
apr_os_file_get(fd, pod-pod_in);
rc = read(fd, c, 1);

In theory, looking at the parameters of read(), c and 1 are  
calculated by the compiler, which leaves the possibility that the fd  
is invalid, although fd does seem to have a sensible value (4).


What does seem strange is this line in the backtrace:

#1  0x2aca224a5e77 in read (pod=value optimized out)

The parameters of read() are (int fd, void *buf, size_t nbytes), not  
(ap_pod_t *pod), this looks a bit odd.


The next step for me is to attempt to deploy a non optimised httpd to  
see if I can reproduce it, in the mean time, does this ring any bells  
with anybody?


(gdb) bt full
#0  0x2aca23d3491b in read () from /lib64/libpthread.so.0
No symbol table info available.
#1  0x2aca224a5e77 in read (pod=value optimized out)
at /usr/include/bits/unistd.h:35
No locals.
#2  ap_mpm_pod_check (pod=value optimized out)
at /usr/src/debug/httpd-2.2.17/server/mpm/worker/pod.c:54
c = value optimized out
fd = 4
#3  0x2aca224a3e42 in child_main (child_num_arg=value optimized  
out)

at /usr/src/debug/httpd-2.2.17/server/mpm/worker/worker.c:1258
threads = value optimized out
rv = 577498496
ts = 0x2aca2c4883f8
thread_attr = 0x2aca2c488418
start_thread_id = value optimized out
#4  0x2aca224a3ff5 in make_child (s=0x2aca287d5848, slot=2)
at /usr/src/debug/httpd-2.2.17/server/mpm/worker/worker.c:1341
pid = 0
#5  0x2aca224a40af in startup_children (number_to_start=8)
at /usr/src/debug/httpd-2.2.17/server/mpm/worker/worker.c:1375
i = 2
#6  0x2aca224a4b7f in ap_mpm_run (_pconf=0x2aca287ce138,
---Type return to continue, or q return to quit---
plog=value optimized out, s=0x2aca287d5848)
at /usr/src/debug/httpd-2.2.17/server/mpm/worker/worker.c:1742
remaining_children_to_start = 10
rv = value optimized out
#7  0x2aca2247dc73 in main (argc=4, argv=0x7fff25e2ca78)
at /usr/src/debug/httpd-2.2.17/server/main.c:739
c = 68 'D'
configtestonly = 0
confname = 0x2aca224a7478 conf/httpd.conf
def_server_root = 0x2aca224a7488 /etc/httpd
temp_error_log = 0x0
error = value optimized out
process = 0x2aca287cc218
server_conf = value optimized out
pglobal = 0x2aca287cc128
pconf = 0x2aca287ce138
plog = 0x2aca288002c8
ptemp = 0x2aca287d2158
pcommands = 0x2aca287d0148
opt = 0x2aca287d0238
rv = value optimized out
optarg = 0x7fff25e2cf32 SERVER_CLASS_STATICP
(gdb)

Regards,
Graham
--



Re: SetVirtualDocumentRoot / per request document root / context root?

2010-12-12 Thread Graham Leggett

On 13 Dec 2010, at 12:23 AM, Stefan Fritsch wrote:


I have looked at the patch and it looks reasonable. The fact that two
known modules (mod_vhost_ldap and mod_ftp) copy the whole server_rec
just to change the document root means that this feature is needed.


An idea from left field.

Is there a reason that DocumentRoot is a virtual host wide setting?

Does it not make sense to make it per-Location instead?

Regards,
Graham
--



rotatelogs: support for echoing to stdout

2010-12-03 Thread Graham Leggett

Hi all,

When processing logs in real time by more than one tool, it is useful  
for rotatelogs to optionally pass logs through to stdout for further  
processing by the next tool in the chain using reliable piped logs.  
This patch makes this possible.


Regards,
Graham
--


httpd-rotatelogs-echo.patch
Description: Binary data




Re: disk cache file rename errors on Windows

2010-12-02 Thread Graham Leggett

On 23 Nov 2010, at 8:21 PM, Dan Poirier wrote:


We're seeing errors like this from mod_disk_cache on Windows only:

(OS 5)Access is denied.  : disk_cache: rename tempfile to datafile
failed: c:/temp/HTTPServer7/aptmpV0JKJ8 -
c:/temp/HTTPServer7/wHY/FhW/b...@muvttlk@V4w.data

under moderate to heavy load, resulting in requests failing.


Looking at the code, the error message is thrown when  
apr_file_rename() fails, specifically when a temporary file is swung  
into place in the cache.


Looking at the APR code, there is a forest of ifdefs that seem to  
choose one of MoveFileEx(), MoveFileW(), MoveFileExW() or MoveFile().  
Ideally, the Microsoft API documentation should explain under what  
conditions the error Access is denied is thrown.


Does Windows allow you to move a file into place while the replaced  
file is still open for read?


Regards,
Graham
--



Re: create a pool associated with the MPM generation?

2010-11-22 Thread Graham Leggett

On 22 Nov 2010, at 2:45 PM, Jeff Trawick wrote:


The purpose of the pool would be the allocation in the parent of
resources which must live as long as any child processes from a
certain generation are still running.  It helps bridge the gap between
pconf and the process pool when dealing with graceful restart.

Each MPM would be responsible for destroying the pool once the last
child of the corresponding generation has exited.  Since graceful
restart can be invoked again before all the children of older
generations have exited, there could be an arbitrary number of
generation pools at a given time.

It might not be trivial to implement, but it would be a huge help for
modules faced with this issue; it is not practical for modules to try
to solve this lifetime issue.

main() owns the generation (for (;;) {) and would create the pool.
The natural place to access the pool would be an additional parameter
to hooks which take pconf currently, but that would be disruptive to
almost every module.   The mpm hook (implemented by modules which must
be modified anyway) would take a pointer to pgeneration, and an API
call would be provided for other modules to use; the API would only
support access of the current pgeneration; only the MPM would know
about the pool for earlier generations.  Each instance of the pool
would be a child of the process pool, created by main(), and destroyed
by the MPM at some arbitrary point later.

Problem worth solving?
Simpler solution available?


I'd say definitely a problem worth solving, if only for the  
elimination of nasty edge cases. The solution sounds sensible.


Regards,
Graham
--



Re: Proposed: PKI Authentication for secure web access

2010-11-21 Thread Graham Leggett

On 21 Nov 2010, at 6:59 AM, Sander Temme wrote:


Thanks for the link Issac. If this is already in Apache, why isn't
everyone using it?


Because key management is just too freaking hard, and too much of a  
management and support burden.


For God's sake, if we can't even get the Apache developer community  
to use PGP without handholding, how would you expect the general  
public to handle this tech?


In our experience, the hardest part about using certificates is  
overcoming the perception held by technical people that it's hard to  
use certificates.


Over the last three years, we have rolled out a certificate based  
infrastructure across a large organisation, with certs for all  
employees and external suppliers. The basic premise is that usernames  
and passwords are banned (unless completely unavoidable), and that  
your certificate gives you whatever access you need. Everything that  
requires registration of some kind has been configured to auto- 
register people from details in the certificates, so we have no  
centralised directory of any kind for people with certificates. Lots  
of problems evaporated as a result. When the certificate expires, or  
is revoked, the portcullis comes crashing down and you're locked out  
everywhere. There are no residual does person X still have access  
problems.


For end users, life is simple. If you need to access something, you  
simply go there, job done. No login forms, no registration, no asking  
somebody for access, no forgot your password forms, no obscure  
username that is annoyingly different to all your other usernames.


In our experience, unlike technical people, end users don't know that  
certificates are supposed to be hard, and so have never known they  
were supposed to consider certificates a problem. As a result, it's  
been very successful.


Regards,
Graham
--



Re: BufferedLogs

2010-11-21 Thread Graham Leggett

On 21 Nov 2010, at 7:49 PM, Stefan Fritsch wrote:


Considering that it's so old and there don't seem to be open bug
reports about it, I would remove the 'experimental'.


+1.

Regards,
Graham
--



Re: Proposed: PKI Authentication for secure web access

2010-11-20 Thread Graham Leggett

On 20 Nov 2010, at 10:27 AM, Rob Lemaster wrote:


SSH allows a user to create a public/private key pair and use that for
authentication. This is much more secure than simply using passwords
and adds the ability to add 'something you have' for multi-factor
authentication. I propose that the same functionality would be enabled
for web authentication.

This functionality would require support on the server and in the
client browser. The server would need to have the ability to store and
recognize a public keys for authentication. The client browser would
need to have the ability to create public/private keys and store them
securely. It would also need to have the ability to copy the keys to
other computers (home/work) or store them on a USB thumb drive for
remote access.

This functionality would be used primarily for web sites that require
secure authentication, such as banks, Ebay, and Paypal.

Do you think this is a good idea?


Is there anything here that isn't already done by X509 client  
certificates, as offered by mod_ssl?


Regards,
Graham
--



Re: mod_disk_cache - mod_cache_disk

2010-11-20 Thread Graham Leggett

On 14 Oct 2010, at 8:50 PM, Ruediger Pluem wrote:


The naming of mod_disk_cache currently goes against the naming
convention of other grouped modules in the server, such as  
mod_proxy_*,

and mod_socache_*.

Are there any objections to me renaming mod_disk_cache to  
mod_cache_disk

for httpd v2.4?


+1


I am about to go ahead and change this, would it be possible for  
people doing builds for other than unix to check whether I've made the  
updates correctly?


Regards,
Graham
--



Re: mod_include: include virtual and error handling

2010-11-20 Thread Graham Leggett

On 02 Nov 2010, at 10:34 PM, Nick Kew wrote:


The lack of this one feature is the most cited reason I've been given
for why people have moved away from mod_include as a template
processor to other template processors within other servers. Rather
than moving to an entirely new type of server, I'd rather we just fix
the core problem.


Wouldn't the same argument support an onerror=url clause too?

Yes, you can use an errordocument.  But there seem to be a lot of
users who find that a difficult concept to grasp (an error document
that we intentionally use???), so it's not really a great answer.
Besides, an errordocument could easily end up getting overloaded!

The implementation should presumably be straightforward alongside
what you propose, and could use an errordocument processing path.


+1.

Turns out they would both work a very similar way.

Regards,
Graham
--



Re: Proposed: PKI Authentication for secure web access

2010-11-20 Thread Graham Leggett

On 20 Nov 2010, at 10:19 PM, Rob Lemaster wrote:


Isn't mod_ssl used solely for HTTPS (browser-server encryption)? I
would like to use PKI for user authentication like you can in SSH on
top of the encryption provided by HTTPS. The most secure option I see
available for web authentication currently is OTP tokens (RSA,etc)
that only work on one web site.


mod_ssl is used solely for https, yes, but the feature you're looking  
for is built into https by default already.


Certificates work symmetrically, both sides have the power to require  
the other side to present a valid certificate.


In the case you might be most familiar with, only one side has a  
certificate (the server). The other side (the browser) has no  
certificate. In this scenario, the browser can be sure it is speaking  
to the right server, because the server presented a signed  
certificate, but the server has no idea about the browser. Usually,  
some other authentication mechanism is used to identify the browser,  
of varying strengths (passwords, etc).


In the case you want however, both sides of the connection are  
configured to require a certificate from the other side. The  
certificates do the same job as the keys that are exchanged in your  
SSH configuration, they allow the other side to say yup, I trust  
you, and that trust works both ways.


Unlike an SSH key however, a certificate contains embedded within it  
details of the person (or thing) that owns the certificate, but these  
are details as far as the protocol is concerned.


Regards,
Graham
--



Re: svn commit: r1035605 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c

2010-11-19 Thread Graham Leggett

On 19 Nov 2010, at 8:33 AM, Ruediger Pluem wrote:


Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1035605r1=1035604r2=1035605view=diff
=
=
=
=
=
=
=
=
=
=
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Tue Nov 16  
12:08:38 2010

   * left waiting for a slow client to eventually

@@ -1930,6 +1946,7 @@ apr_status_t ap_proxy_http_process_respo

/* make sure we always clean up after ourselves  
*/

apr_brigade_cleanup(pass_bb);
+apr_brigade_cleanup(bb);


Is this safe? If we released the backend connection we know that bb  
is empty, because
we already called apr_brigade_cleanup(bb);, but what if the pool and  
the allocator
of the backend connection have died meanwhile? Is this operation  
still safe in this case?


bb is created like so:

bb = apr_brigade_create(p, c-bucket_alloc);

In other words, from the front side connection's allocator. When we've  
closed early, bb will be empty, which means this is a noop, but the  
brigade is guaranteed to still exist.


Regards,
Graham
--



mod_ssl's proxy support: make it per directory

2010-11-19 Thread Graham Leggett

Hi all,

For a while, mod_ssl has been able to secure connections from  
mod_proxy, backwards towards some backend server.


For some reason however, the directives that control this behavior  
SSLProxy* are all scoped virtual host only, making it possible to SSL  
protect just one single ProxyPass going backwards, and not more than  
one, something that severely limits the usefulness of the feature.


I would like to lift this restriction by making SSLProxy* directives  
per-directory/location. For obvious reasons, the existing forward SSL  
behaviour will remain unchanged.


Thoughts?

Regards,
Graham
--



mod_ssl: inserting cert parameters into headers

2010-11-19 Thread Graham Leggett

Hi all,

For a while we've been relying on our load balancers to terminate SSL  
for us, and place details of client certs into HTTP headers before  
passing a connection backwards (through a further SSL protected  
connection).


We're in a situation where we want to use httpd instead of a load  
balancer for this, and I've noticed that we can't place cert  
parameters into headers like the load balancer can, but are rather  
limited to placing the information into the CGI environment only.


Being extensible, ideally we should have a hook which is called that  
takes the SSL parameters, and then offer default implementations that  
insert the information into the CGI environment, and insert the  
information into the request headers, or any other possible  
implementation as the admin requires it in some external module.


Most specifically, if SSLOptions +StdEnvVars is specified, the hook  
gets called with the data, and an implementation writes them to the  
subprocess environment, or headers_in, as appropriate (and as  
configured).


Regards,
Graham
--



Re: svn commit: r1035605 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c

2010-11-19 Thread Graham Leggett

On 19 Nov 2010, at 3:19 PM, Igor Galić wrote:


I believe to have, for the first time in months, understood what it
is with mod_proxy, and why it's so heavily discussed:

There are a number separate and related issues, because mod_proxy:

* handles connections to the front end
 (To large numbers of differently behaving clients)
* handles (highly configurable) connections to the back-end
* can speak very different protocols to the back-ends
 (In terms of timeouts, keep-alives, etc..)
* can be a transparent proxy as well as a reverse proxy

* The code might, or might not be heavily interwoven.

is that remotely right?


It can be safe to say mod_proxy relies heavily on pool lifetimes  
being correct.


In most cases in the server, the only pool you care about is the  
request pool. When the request is cleaned up, so is your data, and  
you're done. In these cases, you simply don't care about pool  
lifetimes, and because you're only using a pool you've already been  
given, the lifetime simply isn't your problem.


In the case of mod_proxy however, a backend connection may start life  
either before or after the frontend connection, and may terminate  
before or after the frontend connection terminates. The backend has a  
completely separate lifetime to the frontend.


What this means is, if there are pool lifetime issues hidden in the  
server, proxy is likely to show them up first.


Regards,
Graham
--



Re: mod_ssl's proxy support: make it per directory

2010-11-19 Thread Graham Leggett

On 19 Nov 2010, at 3:15 PM, Plüm, Rüdiger, VF-Group wrote:


For a while, mod_ssl has been able to secure connections from
mod_proxy, backwards towards some backend server.

For some reason however, the directives that control this behavior
SSLProxy* are all scoped virtual host only, making it
possible to SSL
protect just one single ProxyPass going backwards, and not more than
one, something that severely limits the usefulness of the feature.


What limits do you see with the actual per virtual host  
configuration?


Most specifically, any attempt to set a client certificate to a  
particular proxypass ends up being valid server wide.


Each backend server which a reverse proxy proxies to has the potential  
to have different requirements for SSL, from client certs, to ciphers  
used, etc.


We have worked around this to date by either delegating this task to  
load balancers, or writing little php apps to proxy the connections,  
but this is really ugly, when mod_proxy+mod_ssl can potentially do  
this itself.


Regards,
Graham
--



Re: mod_ssl: inserting cert parameters into headers

2010-11-19 Thread Graham Leggett

On 19 Nov 2010, at 3:19 PM, Plüm, Rüdiger, VF-Group wrote:


Does

RequestHeader add some_header %{SSL_ENVIRONMENT_VARIABLE}s

not work for you?


It could, but it isn't very clean at all. You are adding a KV pair to  
one table, then manually copying it into another table.


If a hook existed to do this, a module might do anything anyone wanted  
to.


Regards,
Graham
--



Re: mod_ssl: inserting cert parameters into headers

2010-11-19 Thread Graham Leggett

On 19 Nov 2010, at 6:24 PM, Nick Kew wrote:


Most specifically, if SSLOptions +StdEnvVars is specified, the hook
gets called with the data, and an implementation writes them to the
subprocess environment, or headers_in, as appropriate (and as
configured).


A hook?  That suggests you expect further generality!


Indeed I do, yes.


Why not just an alternative SSLOption to set headers_in in
place of subprocess_env?


Because as soon as you have two different ways of doing something, you  
than need to ask yourself but what if I want to do it a third way, or  
a fourth way, or maybe some custom way unique to my application, and  
you then come to the conclusion why not provide a hook, so the admin  
can configure it any way he wants, and it stops being httpd's problem  
at all.


To explain more of where I am coming from.

We have ZXTM load balancers, which front various httpds that do  
various things. We have client certificate protected connections to  
the load balancers, which terminate the SSL, then establish a new  
connection pool to the backend httpds (also SSL, but with different  
certs).


In order to expose details of the original client certificate, ZXTM  
injects many headers into the request, just as we inject CGI variables  
into the subprocess environment. Applications behind in turn pick up  
DNs and other details from these headers. The headers are ZXTM specific.


For various reasons we don't want to use ZXTMs for everything, and so  
some httpds terminate the SSL themselves, however we face a problem:  
no clean ZXTM compatible capability exists to expose the headers to  
the backend.


Our first prize would be to develop for ourselves a  
mod_ssl_zxtm_compat module, that when turned on would just send ZXTM  
compatible headers, but we lack the hook to hook into.


Sure, we can try and kludge it, by trying to pick up and rewriting the  
CGI variables, and then worrying that our module runs after the  
mod_ssl module, or we can simply add twenty RequestHeader statements  
next to each ProxyPass, but both of these scream kludge, and  
kludge is what we're trying to move away from.


Regards,
Graham
--



Re: svn commit: r1034916 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h modules/proxy/proxy_util.c

2010-11-19 Thread Graham Leggett

On 16 Nov 2010, at 2:35 AM, Nick Kew wrote:


Well, you *could*.  You'd just (probably) sacrifice the optimisation.
Much the same story as a bunch of chars.

FWIW, if I'd been designing the above from scratch, those flags
would be a bitfield and a set of #defines, thus occupying a
fixed/known width in the struct.  Compared to that, using :1
just enables the compiler to optimise to an indeterminate size
according to its alignment rules.


Given that v2.4 isn't baked right now, it looks like a very sensible  
suggestion to change it in this way.


At the very least, it gives us the option to add bit fields (to a  
sensible limit) without having to stick them on the end.


Regards,
Graham
--



Re: svn commit: r1035504 - in /httpd/httpd/trunk: include/ap_mmn.h modules/proxy/mod_proxy.h modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

2010-11-16 Thread Graham Leggett

On 16 Nov 2010, at 8:56 AM, Ruediger Pluem wrote:


Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1035504r1=1035503r2=1035504view=diff
=
=
=
=
=
=
=
=
=
=
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Tue Nov 16  
00:23:37 2010

@@ -2643,19 +2643,17 @@ PROXY_DECLARE(int) ap_proxy_connection_c
apr_sockaddr_t *backend_addr = conn-addr;
int rc;
apr_interval_time_t current_timeout;
-apr_bucket_alloc_t *bucket_alloc;

if (conn-connection) {
return OK;
}

-bucket_alloc = apr_bucket_alloc_create(conn-scpool);
/*
 * The socket is now open, create a new backend server connection
 */
conn-connection = ap_run_create_connection(conn-scpool, s,  
conn-sock,

0, NULL,
-bucket_alloc);
+c-bucket_alloc);


-1 Veto. This does not work.


Just to clear up any possible perception of otherwise, I have spent  
hours and hours trying to pick apart this code and try to understand  
exactly what both mod_proxy and mod_ssl are doing, and why  
mod_proxy_http is behaving so dramatically differently to  
mod_proxy_ftp and mod_proxy_connect, in order to fix a very real  
problem in a very real set of datacentres. I would appreciate it if  
you could help me get to the bottom of any issues I may not be  
understanding so that this can be fixed once and for all. You don't  
need to veto anything to get my attention, you can just say this  
won't work and this is why. :)



The socket bucket of the backend connection is created from
this bucket allocator. Hence the reuse of connections from the  
backend will be broken
as c-bucket_alloc will be gone when the backend connection and  
hence the socket bucket

is reused.


Ok, I originally understood that the conn_rec was recreated each time,  
but this isn't the case.


This is making more sense.

What it seems we need to do is keep wrapping buckets in transient  
buckets as we were doing before, and then, for the final EOS case,  
force the setaside to guarantee that that last set of buckets have  
been physically moved to the frontend connection, and the backend can  
be safely released and reused.


Regards,
Graham
--



Re: svn commit: r1035504 - in /httpd/httpd/trunk: include/ap_mmn.h modules/proxy/mod_proxy.h modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

2010-11-16 Thread Graham Leggett

On 16 Nov 2010, at 12:17 PM, Plüm, Rüdiger, VF-Group wrote:


Sorry for having been in grumpy mode this morning, but I saw this code
which is what I pointed out before to be not working :-).


As you've pointed out, the code is definitely wrong, I suspect you  
choked on your coffee and for that I apologise :)


That looks like a plan. *Before* we sent the final brigade (which  
had its

buckets transformed to the transient buckets of c-bucket_alloc)
containing the EOS bucket do a setaside on each bucket in this brigade
(maybe via ap_save_brigade).


That is what the new patch did, the mistake I made was to think that  
the forced setaside could replace the wrapping in transient buckets,  
when in reality it needed to be done in addition to the wrapping in  
transient buckets.


This is the part of the original patch that does it:

Index: modules/proxy/mod_proxy_http.c
===
--- modules/proxy/mod_proxy_http.c  (revision 1035578)
+++ modules/proxy/mod_proxy_http.c  (working copy)
@@ -1902,7 +1902,6 @@

 /* Switch the allocator lifetime of the buckets */
 ap_proxy_buckets_lifetime_transform(r, bb,  
pass_bb);

-apr_brigade_cleanup(bb);

 /* found the last brigade? */
 if  
(APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(pass_bb))) {

@@ -1910,6 +1909,23 @@
 /* signal that we must leave */
 finish = TRUE;

+/* the brigade may contain transient buckets  
that contain
+ * data that lives only as long as the  
backend connection.
+ * Force a setaside so these transient  
buckets become heap

+ * buckets that live as long as the request.
+ */
+for (e = APR_BRIGADE_FIRST(pass_bb); e
+!= APR_BRIGADE_SENTINEL(pass_bb); e
+= APR_BUCKET_NEXT(e)) {
+apr_bucket_setaside(e, r-pool);
+}
+
+/* finally it is safe to clean up the brigade  
from the
+ * connection pool, as we have forced a  
setaside on all

+ * buckets.
+ */
+apr_brigade_cleanup(bb);
+
 /* make sure we release the backend  
connection as soon
  * as we know we are done, so that the  
backend isn't
  * left waiting for a slow client to  
eventually

@@ -1930,6 +1946,7 @@

 /* make sure we always clean up after ourselves */
 apr_brigade_cleanup(pass_bb);
+apr_brigade_cleanup(bb);

 } while (!finish);
 }

Regards,
Graham
--



Re: Error returned on broken chunking

2010-11-13 Thread Graham Leggett

On 13 Nov 2010, at 1:22 AM, Nick Kew wrote:


Any strong rationale for 503, or should I just change
it to 400?


400 looks like the correct code to use, yes.

Regards,
Graham
--



Re: Bug 29404 - Allow definition of source address of outgoing connections from mod_proxy

2010-11-13 Thread Graham Leggett

On 13 Nov 2010, at 8:11 PM, Deanna Siemaszko wrote:

Allow definition of source address of outgoing connections from  
mod_proxy.   There was a patch added to this ticket. https://issues.apache.org/bugzilla/show_bug.cgi?id=29404


What is preventing this patch from being merged into the trunk? I'd  
be happy to test it once it's committed.  I ran into this issue  
recently out in the wild. It's a common feature on other proxy  
applications.


Looking at it now, the latest patch is for v2.2 rather than trunk. It  
would need to go to trunk first before being considered for v2.2.


Regards,
Graham
--



Re: svn commit: r1033779 - in /httpd/httpd/trunk: modules/cache/cache_common.h modules/cache/mod_cache.h modules/cache/mod_disk_cache.h support/htcacheclean.c

2010-11-11 Thread Graham Leggett

On 11 Nov 2010, at 3:35 AM, Guenter Knauf wrote:

so you suggest the stuff extracted from mod_disk_cache.h should go  
into a separate disk_cache_common.h ?


If possible, yes. htcacheclean and mod_disk_cache are tied together  
strongly, but are independent of mod_cache (as a provider).


Regards,
Graham
--



Re: Proxy regressions

2010-11-10 Thread Graham Leggett

On 10 Nov 2010, at 9:02 AM, Ruediger Pluem wrote:


The fix in r1030855 is wrong: ap_proxy_buckets_lifetime_transform is
not copying the data but only creates transient buckets from the data
in the buckets in bb. If you then destroy bb before passing pass_bb,
the data where the buckets in pass_bb point gets freed and later gets
overwritten.


Good catch. I reviewed the code again and then remembered the idea why
we use transient buckets: If everything goes well and the data is  
sent out
by ap_pass_brigade no copying of the contents of the buckets is  
needed.
Only if things are buffered somewhere down the chain, the according  
filter

needs to set the buckets aside (which causes copying).
So I guess with the approach to release the backend connection  
immediately

we will loose this advantage. That is regrettable.
I guess an easy solution would be to use heap buckets instead of  
transient

buckets.


Have we not created a pool lifetime problem for ourselves here?

In theory, any attempt to read from the backend connection should  
create buckets allocated from the r-connection-bucket_alloc  
allocator, which should be removed from the backend connection when  
the backend connection is returned to the pool.


Instead, it seems we create our own bucket_alloc from the backend  
connection's pool, and then we only work because we're holding onto  
the backend until we've stopped writing buckets - far longer than we  
should be, and all along we've been working, but only by accident.


Regards,
Graham
--



Re: Proxy regressions

2010-11-10 Thread Graham Leggett

On 10 Nov 2010, at 1:09 PM, Plüm, Rüdiger, VF-Group wrote:


The proxy currently creates the allocator in
ap_proxy_connection_create(), and then passes the allocator to the
various submodules via the ap_run_create_connection() hook, so it
looks like we just passing the wrong allocator.


The problem is that we keep the connrec structure in the conn pool.
It is not created each time we fetch a connection from the conn pool.
This is required to enable keepalives with SSL backends.
As said if we pass the bucket allocator from the front end connection
we possibly end up with other pool lifetime issues and as I speak of
it SSL comes to my mind.


This doesn't sound right to me - ideally anything doing a read of  
anything that will ultimately be sent up the filter stack should use  
the allocator belonging to the frontend connection. When the backend  
connection is returned to the pool, the allocator should be removed,  
and the next allocator inserted when the backend connection is  
subsequently reused.


Currently what we seem to have is data allocated out of a pool that  
has a lifetime completely unrelated to the frontend request, and then  
we're working around this by trying to keep this unrelated pool alive  
way longer than it's useful lifetime, and at least as long as the  
original request. This seems broken to me, we should really be using  
the correct pools all the way through.



Right now, we are holding backend connections open for as long as it
takes for a frontend connection to acknowledge the request. A
typical
backend could be finished within milliseconds, while the
connection to
the frontend often takes hundreds, sometimes thousands of
milliseconds. While the backend connection is being held open, that
slot cannot be used by anyone else.


Used by whom?


Another worker in httpd.


As said if you put it back in the pool and your pool has the
same max size as the number of threads in the process then there is  
some
chance that this connection will idle in the pool until the actual  
thread
sent data to the client and fetches the connection from the pool  
again.
As said I can only follow if the max pool size is configured to be  
smaller

than the number of threads in the process. Do you do this?


Yes. Threads in an application server are expensive, while threads in  
httpd are cheap.


A further issue is with backend servers where keepalive is switched  
off. Instead of acknowledging the connection close and releasing the  
connection, we hold the connection open for ages until the client  
finally acknowledges the request as finished.



This issue is a regression that was introduced in httpd v2.2, httpd
2.0 released the connection as soon as it was done.


Because it had a completly different architecture and the released  
connection
was not usable by anyone else but the same frontend connection  
because it was stored
in the conn structure of the frontend request. So the result with  
2.0 is the same

as with 2.2.


In v2.0, it was only saved in the connection if a keepalive was  
present. If there was no keepalive, it was released immediately.


Regards,
Graham
--



Re: Proxy regressions

2010-11-10 Thread Graham Leggett

On 10 Nov 2010, at 11:49 AM, Plüm, Rüdiger, VF-Group wrote:


Have we not created a pool lifetime problem for ourselves here?

In theory, any attempt to read from the backend connection should
create buckets allocated from the r-connection-bucket_alloc
allocator, which should be removed from the backend connection when
the backend connection is returned to the pool.


I guess we need a dedicated bucket_allocator at least in the beginning
as we cannot guarantee that anyone in the create_connection hook uses
the bucket_allocator to create an object that should persist until the
connrec of the backend connection dies.

Exchanging the allocator later each time we get the connection from
the conn pool might create similar risks. But I admit that the later
is only a gut feeling and I simply do not feel well with exchanging  
the

allocator. I have no real hard facts why this cannot be done.


The proxy currently creates the allocator in  
ap_proxy_connection_create(), and then passes the allocator to the  
various submodules via the ap_run_create_connection() hook, so it  
looks like we just passing the wrong allocator.


So without trying to offend anyone, can we see the use case for the  
asap returning

again?


Right now, we are holding backend connections open for as long as it  
takes for a frontend connection to acknowledge the request. A typical  
backend could be finished within milliseconds, while the connection to  
the frontend often takes hundreds, sometimes thousands of  
milliseconds. While the backend connection is being held open, that  
slot cannot be used by anyone else.


In addition, when backend keepalives are kept short (as ours are), the  
time it takes to serve a frontend request can exceed the keepalive  
timeout, creating unnecessary errors.


This issue is a regression that was introduced in httpd v2.2, httpd  
2.0 released the connection as soon as it was done.


Regards,
Graham
--



Re: Proxy regressions

2010-11-10 Thread Graham Leggett

On 10 Nov 2010, at 4:13 PM, Plüm, Rüdiger, VF-Group wrote:


The core input filter in ap_core_input_filter which is used to read
the response from the backend creates the socket bucket from the  
conn rec

bucket allocator. So the used bucket allocator must live as long
as the conn rec of the backend connection lives.
And the conn rec of the backend connection lives longer then one  
request
and possibly longer than the frontend connection if the backend has  
keepalive
enabled, in fact it is reused by the next frontend request that  
leases the

backend connection from the backend connection pool.


In that case, the way to fix this would be to insert the frontend  
conn_rec's allocator into the backend conn_rec when we take it out of  
the pool, and then remove this allocator again when we're finished and  
place the connection back into the pool.


This way, all our buckets would live as long as the frontend  
allocator, and we would be free to return the backend conn_rec back to  
the pool at any time.


Regards,
Graham
--



Re: Proxy regressions

2010-11-10 Thread Graham Leggett

On 10 Nov 2010, at 3:54 PM, Plüm, Rüdiger, VF-Group wrote:

As said this sounds doable for http backends, but not for https  
backends
where we need to keep some data regarding the SSL state in the conn  
rec

of the backend connection.


This is entirely fine, it's only the contents of the buckets in the  
brigade that need to have a lifetime as long as the request, other SSL  
specific data can work as it does now, there is no need for it to be  
changed.



A further issue is with backend servers where keepalive is switched
off. Instead of acknowledging the connection close and releasing the
connection, we hold the connection open for ages until the client
finally acknowledges the request as finished.


Is this a problem of a too long lingering close period on the  
backend server

blocking the expensive backend server threads?
I mean in general the backend server is the one who closes the  
connection

if its keepalive timeout was used up and hence it can close the socket
from its side. The only thing that comes to mind that could keep it
blocked is a lingering close. Is this the case here?


We do see this, yes, and it caused two outages in our testing  
environment recently caused by the initial version of the patch and  
the original pool lifetime bug.



In v2.0, it was only saved in the connection if a keepalive was
present. If there was no keepalive, it was released immediately.


Which resulted in no connection pooling at all.


Having a connection pool isn't a win if it comes at the cost of  
performance drag elsewhere. Ultimately the connections themselves in  
our case are far cheaper than the application servers behind them.


Regards,
Graham
--



Re: svn commit: r1033779 - in /httpd/httpd/trunk: modules/cache/cache_common.h modules/cache/mod_cache.h modules/cache/mod_disk_cache.h support/htcacheclean.c

2010-11-10 Thread Graham Leggett

On 11 Nov 2010, at 1:43 AM, fua...@apache.org wrote:


Author: fuankg
Date: Wed Nov 10 23:43:06 2010
New Revision: 1033779

URL: http://svn.apache.org/viewvc?rev=1033779view=rev
Log:
Splitted off cache defines/structs used by htcacheclean.

This makes htcacheclean again independent from httpd.h.


Being a little picky, but ideally mod_disk_cache should have it's own  
common header for htcacheclean to depend on, so that mod_disk_cache  
doesn't melt back into mod_cache.


Regards,
Graham
--



Re: svn commit: r1032059 - /httpd/httpd/trunk/modules/filters/mod_include.c

2010-11-07 Thread Graham Leggett

On 06 Nov 2010, at 10:32 PM, Stefan Fritsch wrote:

I think I have made my intentions clear in my first mail from  
October 23rd. But maybe I should have mentioned it also in the later  
mails.


The grammar and in particular the string handling in the SSI  
expression parser is so weird that it would be very difficult to  
handle it in a backward compatible way with the same parser as the  
ssl_expr grammar. At the minimum, one would need to rewrite the  
tokenizer. Therefore I thought it a much more sensible approach to  
handle the SSI expression compatibility by just putting that parser  
back into mod_include. So the total number of parsers in httpd has  
not changed by my work, but we now have the better parser as  
univeral parser.


Of course it would be trivial to offer the new ap_expr as an  
alternative in mod_include. Either by allowing to select the parser  
with a config directive or by an alternative syntax in the SSI  
document, like
'!--#if apexpr=...--'. Does this sound reasonable? Which of the  
two alternatives would you prefer? It may also be possible to at  
least make the generalized variable lookup available in the old SSI  
parser.


If we can offer a toggle switch directive, and declare the old syntax  
as deprecated (and remove it in v2.6/v3.0), I think it would be ideal.


BTW, there are a number of other places where I want to allow  
ap_expr as an alternative to the current syntax:


- SetEnvIf (maybe as SetEnvIf expr=...?)
- Conditional access log (simply by using expr=... instead of  
env=...)

- maybe RewriteCond


+1.

Regards,
Graham
--



Re: [RFC] Error directive to generate custom error messages from httpd.conf

2010-11-07 Thread Graham Leggett

On 08 Nov 2010, at 2:35 AM, Jeff Trawick wrote:


With the Error directive:

IfModule !mod_include.c
Error mod_foo requires mod_include!  Use the LoadModule directive to
load mod_include.
/IfModule


$ ./httpd -t
Syntax error on line 486 of /home/trawick/inst/23/conf/httpd.conf:
mod_foo requires mod_include!  Use the LoadModule directive to load  
mod_include.


Definite +1, clear error messages are a very good thing.

Regards,
Graham
--



Re: mod_rewrite: RewriteEngine off doesn't work in a directory context

2010-11-06 Thread Graham Leggett

On 06 Nov 2010, at 5:21 PM, Ruediger Pluem wrote:


Does this patch make sense?


Do we always have a valid dconf in hook_uri2file? And if yes, why do  
we need the

state field in the server conf any longer?


It's because of the following function, called from a post config  
hook, at which point request_rec and per-directory configs don't yet  
exist:


static apr_status_t run_rewritemap_programs(server_rec *s, apr_pool_t  
*p)


If it wasn't for this function, we wouldn't need the server wide flag  
at all.


Regards,
Graham
--



Re: svn commit: r1032059 - /httpd/httpd/trunk/modules/filters/mod_include.c

2010-11-06 Thread Graham Leggett

On 06 Nov 2010, at 4:03 PM, s...@apache.org wrote:


Author: sf
Date: Sat Nov  6 14:03:13 2010
New Revision: 1032059

URL: http://svn.apache.org/viewvc?rev=1032059view=rev
Log:
Put the expression parser back into mod_include
This reverts r642559 and r642978



-#include ap_expr.h


I don't follow, I was understanding that mod_include and mod_ssl would  
now use the same parser?


Regards,
Graham
--



Re: mod_rewrite: RewriteEngine off doesn't work in a directory context

2010-11-06 Thread Graham Leggett

On 06 Nov 2010, at 5:21 PM, Ruediger Pluem wrote:


Does this patch make sense?


Do we always have a valid dconf in hook_uri2file?


To be safe, I have added a check for dconf being NULL.

Regards,
Graham
--


httpd-mod_rewrite-rewriteengine-fix.patch
Description: Binary data




Re: Proxy regressions

2010-11-05 Thread Graham Leggett

On 05 Nov 2010, at 10:52 PM, Jim Jagielski wrote:


FWIW, I can't recreate this:

[warning] setting ulimit to allow core files
ulimit -c unlimited; /opt/local/bin/perl /Users/jim/src/asf/code/ 
stable/httpd-test/framework/t/TEST
/usr/local/apache2/bin/httpd  -d /Users/jim/src/asf/code/stable/ 
httpd-test/framework/t -f /Users/jim/src/asf/code/stable/httpd-test/ 
framework/t/conf/httpd.conf -D APACHE2

using Apache/2.3.9-dev (event MPM)

...
t/modules/proxy.ok
t/modules/proxy_balancerok
...

proxy.t was maybe a bit slow though...

OS X 10.6.4


I see the issue on MacOSX 10.5.8, wonder if it's a Leopard thing, or  
an apr thing?


Regards,
Graham
--



<    4   5   6   7   8   9   10   11   12   13   >