Re: Multiple AAA providers

[Justin Erenkrantz]

On Wed, Mar 02, 2005 at 07:52:30AM -0600, Jess Holle wrote:
> This is essentially describing Graham's first option -- which is 
> workable.  It just requires extra work from each and every auth module 
> author -- thus leading to a likelihood of some providing this and others 
> not doing so.  It would be good to have a level of consistency.  Though 
> my primary concern is with multiple LDAPs, the multiple password file 
> case makes good sense, for instance.

I believe adding a 'generic' auth containter would introduce far more
complexity in the core for a benefit that only mod_authnz_ldap would actually
use.  -- justin

Re: Multiple AAA providers

[Justin Erenkrantz]

On Wed, Mar 02, 2005 at 08:24:25AM -0500, Geoffrey Young wrote:
> while I don't claim to have more than a cursory understanding of ldap, I
> would think these cases could be handled by extending the current situation
> a bit.  for instance, for the file provider something like
> 
> AuthBasicProvider file
> AuthFileName file1 file2
> 
> if AuthFileName were ITERATE mod_authn_file would know that it should not
> return AUTH_USER_NOT_FOUND until it has checked all the files present.  or
> somesuch off the top of my head.

Correct.  That is the approach that makes the most sense to me.  The provider
itself can loop as long as it wants to using its own config syntax.  

However, there is nothing that prohibits one authn module from registering
multiple providers dynamically.  Remember that the providers are only looked
up at request-time.  So, if mod_auth_ldap were to have a syntax like:

AuthLDAPProvider foo-1 ldap://ldap.example.com/cn=?
AuthLDAPProvider foo-2 ldap://ldap2.example.com/cn=?

AuthBasicProvider foo-1 foo-2

That would work, as long as mod_auth_ldap calls ap_register_provider x number
of times.  -- justin

Re: Multiple AAA providers

[Justin Erenkrantz]

On Wed, Mar 02, 2005 at 04:10:35PM +0200, Graham Leggett wrote:
> The end goal is to simplify the providers so that you do not have to teach
> each one how to handle multiple sources. The problem with implementing
> multiple sources in one provider is that the end users assumes that the
> same is possible in other providers, and is surprised when they find out
> the hard way it's not.

I don't believe it would simplify anything other than mod_auth_ldap.

> To fill out the example of the "Auth" container to better illustrate what
> I mean, you might have this:

You could do this today with a RAW_ARGS for 

Re: Multiple AAA providers

[Justin Erenkrantz]

--On Wednesday, March 2, 2005 12:14 PM -0600 "William A. Rowe, Jr." 
<[EMAIL PROTECTED]> wrote:

Bleh.  Wouldn't it be easier not to rearchitect the whole thing?
I believe this config syntax is orthogonal to the auth provider scheme. 
There are completely independent of each other.

What about the core or mod_auth respecting something like;

  
  AuthFile users1
  
  
  AuthFile users2
  
Simply use the existing scope, inheritance, and so on.  Whenever
multiple AuthConfigs apply to a given scope, iterate them until
satisfied.
The issue here is that it doesn't indicate which auth mechanism to use 
(basic or digest).  That's the critical part.  And, is the order now 
implicit based on the ordering in the conf file?  I think it should be 
explicit.  So, there's lots of pragmatic issues with this approach.

I'm concerned that the more complex each auth provider needs to
be, the more probability that there will be logic errors in the
provider.
I really feel that most auth providers would become more complex if you 
take this approach.  We'd also have to spend a lot of cycles verifying that 
the 'AuthConfig' block is valid at config-time - AuthFile and AuthDBFile 
and AuthLDAPURL couldn't be in the same block.

I'd prefer that we use provider-specific blocks instead. Furthermore, I 
would suggest that mod_authnz_ldap grows this grouping first and then we 
can make a judgment about how complex it is and how much reuse is realistic 
between modules.  -- justin

Re: Multiple AAA providers

[Justin Erenkrantz]

--On Wednesday, March 2, 2005 12:36 PM -0700 Brad Nicholes 
<[EMAIL PROTECTED]> wrote:

Although I agree that this would probably be the best way to go, I don't
think it will be that simple.  Authnz_ldap stores the LDAPurl and other
information (bind user id, bind password, certs, etc) in a per-Dir
structure.  At the very least, authnz_ldap would have to be taught how
to store multiple configurations per-dir.  Other auth modules may have
the same structure.
It'd be trivial to store this as part of the authn_structure as a 'context' 
field.  Each provider could then store its config info in that field 
instead.  To reiterate, I think the ideal here is to come up with a config 
syntax that allows config-time registration of providers with associated 
configuration information.  -- justin

Re: Multiple AAA providers

[Justin Erenkrantz]

--On Wednesday, March 2, 2005 2:07 PM -0600 "William A. Rowe, Jr." 
<[EMAIL PROTECTED]> wrote:

No - simply create a per-dir config, and use dirconfig to represent;
 
 AuthFile users1
 
This would give us the best of both worlds.  It's no different from
the use of Location, Directory, and File per-dir blocks.
How does that work?  The issue here is that we want to use the same module 
multiple times in the same location/directory/file.  So, there'd only be 
one per-dir-config instance.  -- justin

Re: Multiple AAA providers

[Justin Erenkrantz]

On Thu, Mar 03, 2005 at 08:40:22PM -0600, William A. Rowe, Jr. wrote:
> And attached is the module for comment.  I have no time till this
> weekend if then, so I've got the build system changes and will
> commit if we like.

My question as to how this would interact with the auth providers is still
unanswered.

Remember that the auth providers don't implement the check_user_id hook - only
the auth mechanisms (basic/digest) implement those hooks.  So, this module
acts counter to the entire notion of providers by just blindly re-running the
entire hook process instead.  (check_user_id now becomes recursive - yikes!)
We'd now incur the overhead of the auth mechanism hooks when there is little
need to do so.  Plus, we lose the ability to sanely chain providers as was the
original intent.

I still maintain the better way to do this is to handle it in the provider
modules themselves by leveraging the provider API instead.  

To reiterate, in my mind, the ideal syntax is:


  
  ...config options for mod_authnz_ldap...
  
  
  ...config options for mod_authnz_ldap...
  
  
  ...config options for hypothetical mod_authn_dbd...
  

  ...config options for mod_authnz_ldap...

  AuthUserFile conf/foo

  AuthBasicProvider ldap1 ldap2 ldap file my_db


This isolates the config directly to the module, and if we so desire, we could
add helper functions which promote re-use of this strategy by other provider
modules as needed.  -- justin

Re: Multiple AAA providers

[Justin Erenkrantz]

--On Friday, March 4, 2005 8:56 AM -0700 Brad Nicholes 
<[EMAIL PROTECTED]> wrote:

Actually I think the better syntax would be:

   ...config options for mod_authnz_ldap...


   ...config options for mod_authnz_ldap...


   ...config options for mod_auth...


   AuthProvider Myldap1 Myfile1
   ...Other config options...


   AuthProvider Myldap2 Myfile1
   ...Other config options...


   AuthProvider Myldap1 Myldap2
   ...Other config options...

This would allow you to mix-match-reuse-redefine auth configurations
anywhere you like.  I haven't thought this through completely from an
implementation standpoint, but by moving the provider definitions into
mod_auth_basic and then extending the framework for handling multiple
provider definitions into the providers themselves, I think we can come
out with something very useful and reuseable.
Actually, I think we could move this into a (new) mod_authn_alias/config or 
something and make it generic - tieing it into mod_auth_basic would mean it 
couldn't be used by mod_auth_digest.  The key here from the implementation 
perspective is that we would know what the 'real' provider name is as well 
- so we wouldn't have to tie it to the module.  mod_authn_alias would 
register a 'fake' provider that merges the auth's per_dir_config before 
executing the 'real' provider.

So, yah, I like this.  -- justin

Re: Multiple AAA providers

[Justin Erenkrantz]

--On Friday, March 4, 2005 11:27 AM -0600 "William A. Rowe, Jr." 
<[EMAIL PROTECTED]> wrote:

yup, that's what mod_auth_config did.  However, mod_auth_config;
 1. invokes auth for the local directives (not  sectioned)
 2. invokes auth for all  sections.
providing the explicit list in AuthBasicProvider would ensure we walk
the provider configs correctly.
No, the module that was posted had no concept of providers.  The right 
place to do this is at the provider level not the hook level.  -- justin

Re: mod_cache

[Justin Erenkrantz]

--On Friday, March 4, 2005 11:55 PM +0100 Sander Striker <[EMAIL PROTECTED]> 
wrote:

--
modules/cache/mod_cache.c:271
/* If the request has Cache-Control: no-store from RFC 2616, don't store
 * unless CacheStoreNoStore is active.
 */
cc_in = apr_table_get(r->headers_in, "Cache-Control");
if (r->no_cache ||
(!conf->store_nostore &&
 ap_cache_liststr(NULL, cc_in, "no-store", NULL))) {
ap_remove_output_filter(f);
return ap_pass_brigade(f->next, in);
}
What happens if the 'Cache-Control: no-store' header came in with a
304 Not Modified and the original request wasn't conditional?
If I read the spec correctly a 304 can carry a Cache-Control header,
if it has a different value since a previous 200 (or 304).
Hmm.  Good point.  What a goofy corner case though.  I won't lose much sleep 
if we don't fix this.  Care to add a FIXME?  =)

--
modules/cache/mod_cache.c:308
/* have we already run the cachability check and set up the
 * cached file handle?  */
if (cache->in_checked) {
/* pass the brigades into the cache, then pass them
 * up the filter stack
 */
I haven't tracked cache->in_checked fully, so I wonder if it is
possible that this is set on a validating request?  That would
cause the cache not being updated, which is what I am trying to
track down FWIW.  This is not 'my' bug though, since I am seeing
the following line in the log:
  [debug] mod_disk_cache.c(616): disk_cache: Stored headers for URL xxx
However the cache files on disk don't change... I'm a bit puzzled why
not from looking at the code.
Yes, it would be set.  in_checked is set after we know that we want to save 
the response (i.e. all of the cacheability checks passed successfully). 
However, note that if we are 'blocking' the response (i.e. we revalidated the 
response succesfully with a 304), we set block_response which is checked right 
before in_checked.

(I'll come back to your mod_disk_cache problem at the end.)
--
modules/cache/mod_cache.c:371
/*
 * what responses should we not cache?
 *
 * At this point we decide based on the response headers whether it
 * is appropriate _NOT_ to cache the data from the server. There are
 * a whole lot of conditions that prevent us from caching this data.
 * They are tested here one by one to be clear and unambiguous.  */
if (r->status != HTTP_OK && r->status != HTTP_NON_AUTHORITATIVE
&& r->status != HTTP_MULTIPLE_CHOICES
&& r->status != HTTP_MOVED_PERMANENTLY
&& r->status != HTTP_NOT_MODIFIED) {
/* RFC2616 13.4 we are allowed to cache 200, 203, 206, 300, 301 or
410
 * We don't cache 206, because we don't (yet) cache partial
responses.
 * We include 304 Not Modified here too as this is the origin server
 * telling us to serve the cached copy.
 */
reason = apr_psprintf(p, "Response status %d", r->status);
}
AIUI, we can cache "302 Found" (HTTP_MOVED_TEMPORARILY) when it has an
Expires
or Cache-Control indicating that the request can be cached.
Fair enough.  Feel free to add it, if you like.
--
modules/cache/mod_cache.c:685
/* Did we just update the cached headers on a revalidated response?
 *
 * If so, we can now decide what to serve to the client:
 * - If the original request was conditional and is satisified, send 304.
 * - Otherwise, send the cached body.
*/
if (rv == APR_SUCCESS && cache->stale_handle) {
apr_bucket_brigade *bb;
apr_bucket *bkt;
bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
/* Were we initially a conditional request? */
if (ap_cache_request_is_conditional(cache->stale_headers)) {
/* FIXME: We must ensure that the request meets conditions. */
/* Set the status to be a 304. */
r->status = HTTP_NOT_MODIFIED;
Is this as simple as clearing r->headers_in, overwriting with
cache->stale_headers,
and the calling ap_meets_conditions()?
Yes, I *believe* so.  But, we should double-check that ap_meets_condition 
would do the 'right' thing.  I'm not 100% sure here.

Okay, back to your real bug:
possible that this is set on a validating request?  That would
cause the cache not being updated, which is what I am trying to
track down FWIW.  This is not 'my' bug though, since I am seeing
the following line in the log:
  [debug] mod_disk_cache.c(616): disk_cache: Stored headers for URL xxx
However the cache files on disk don't change... I'm a bit puzzled why
not from looking at the code.
Please correct me if I'm wrong.  My understanding is that you have an expired 
cache entry which needs to be revalidated and the updated headers aren't 
updating properly.

Now that I read the code I'm betting you are hitting that else block with 'XXX 
log message' in mod_disk_cache's store_headers.  The sequence that I'm 
envisioning is:

- We have a stale cached entity/handle.
- We send an If-Modif

Re: Multiple AAA providers

[Justin Erenkrantz]

--On Friday, March 4, 2005 12:42 PM -0600 "William A. Rowe, Jr." 
<[EMAIL PROTECTED]> wrote:

We have a naming problem.  Provider is meaningless.
No, it's not.  It's directly from the code and API itself.  See 
ap_provider.h.
Is a provider Basic, Digest?
No.  These are the auth mechanisms which *call* providers.
Or is a provider File, DBM, LDAP?
Yes.
If you mean the former, I'm right there with you.  However, that
won't work on antique (2.0, even 1.3) auth modules, so my concept
is still valid.  It should simply be replaced for httpd-2.0 with
something clearer :)
Well, obviously, you're not with me here.  =)  -- justin

Re: Multiple AAA providers

[Justin Erenkrantz]

On Sat, Mar 05, 2005 at 10:59:30PM -0600, William A. Rowe, Jr. wrote:
> Ok, as Justin and I are in significant disagreement ... to summarize;
> 
> we (collectively) would like to see some mechanism for multiple
> configurations of the same 'provider' (defined above).  There are
> logically three places this can happen, so as a straw poll, would
> those interested in *coding* auth schemas please vote (end users,
> our choice will be transparent enough to you that we prefer the
> developers to indicate their preferences.)
> 
> [ ] Implement in each provider (e.g. mod_authnz_ldap, mod_authn_file)
> Different   sections
> 
> [ ] Implement in each scheme (e.g. basic, digest)
> Different   sections
> 
> [ ] Implement globally across schemes and providers
> Single  directive, but as it's not in the scheme
> which iterates the providers, control isn't as fine-grained.

These choices overlook Brad's suggestion, which I still think is the best:

  [ ] Implement across providers
  Single  directive.

This does not tie it into either the provider or the scheme and allows
the config and overhead of the merging to be contained to the provider'd
alias.

We could probably find a better name than AuthProviderAlias though.

My $.02.  -- justin

Re: mod_cache

[Justin Erenkrantz]

--On Sunday, March 6, 2005 1:54 PM +0100 Sander Striker <[EMAIL PROTECTED]> 
wrote:

I completely agree.  So much even that I just committed it (r156306).
Why are we storing the header fd in the disk object anyways?  I haven't
gone through mod_disk_cache.c yet, but at least for store_headers() it
doesn't seem to make any sense.
It's not really the 'disk' object per se - it's part of mod_disk_cache's 
private cache handle.  When we open the cache handle, we also open the 
associated file descriptors in open_entity to ensure that we have something 
valid cached.  We then use those open descriptors in recall_headers and 
recall_body later on.

Note that we're a little tricky and we partially read the header file in 
open_entity to get some meta-data via file_cache_recall_mydata, but we hold 
off parsing the full MIME headers from disk until recall_headers time.

When it comes to store_headers(), shouldn't that be done using a temp
file and moving it into place atomically just like store_body?  Are
we relying on OS buffering and header size being small enough to pull
that off?  Or am I just being paranoid? :)
Yes, there's probably a race condition here.  However, store_headers is only 
called once, so the window is relatively small compared to store_body which 
can be called multiple times.  Feel free to have at it.  ;-)  -- justin

Re: mod_cache

[Justin Erenkrantz]

--On Monday, March 7, 2005 2:03 AM +0100 Sander Striker <[EMAIL PROTECTED]> 
wrote:

However, I do think you are right that ap_meets_conditions() doesn't do the
right thing.  But that is in general, not just in this case.  It doesn't
seem to take responses other than 2xx into account.  In those cases it
shouldn't
return a 304, yet it does.  We'll have to visit all the places where
ap_meets_conditions() is called to make sure r->status is set, and check
r->status in ap_meets_conditions() to fix this.
*nod*  This is what I was worried about.
Luckily for us, there is more work left even in mod_cache.  Right now,
whenever we hit a Cache-Control: no-cache in the request, the cache declines
to handle the request, while it could be handling it (be it with a required
validation request to the backend).  That would be a lot more efficient.
And within bounds of the spec.
I'm not sure what you mean.  Do you mean that if we get a Cache-Control: 
no-cache, that we should attempt to treat it as mandatory revalidation of the 
content?  Interesting.  Right now, we just get out of the way.

One approach would be to remove the no-cache check and move it down into 
ap_cache_check_freshness() and make our response always be stale in this case.

Furthermore, Cache-Control: max-age=0 or even max-age=X seems to be
completely
ignored.  A response is given back with an Age header with a larger value
then
what max-age was set to in the request.
This *should* be handled by ap_cache_check_freshness().  I'll admit that I 
haven't spent a lot of time in there.  On a cursory look, it seems that it 
should be handling this correctly.  -- justin

Re: mod_cache

[Justin Erenkrantz]

--On Monday, March 7, 2005 7:47 AM +0100 Sander Striker <[EMAIL PROTECTED]> 
wrote:

That's what I initially thought when I glanced over it.  Then I started
wondering why headers are retrieved from h->req_hdrs, instead of
r->headers_in.  I noticed we save the request headers of the request
that got a resource into the cache.  Are we maybe mistakenly using
those stored headers instead of the ones from the current request?
I need to go over the code, but this is what popped into my head on
first glance.
Doh.  req_hdrs shouldn't be used there at all.  Fixed in r156401.
Thanks!  -- justin

Re: Multiple AAA providers

[Justin Erenkrantz]

On Mon, Mar 07, 2005 at 12:16:05AM -0600, William A. Rowe, Jr. wrote:
> >These choices overlook Brad's suggestion, which I still think is the best:
> >
> >  [ ] Implement across providers
> >  Single  directive.
> 
> I did not overlook it.
> 
> What layer do you propose to code it at?

Did you forget my reply to Brad?  I said that it'd be a module that exposes
'fake' new providers that merges in the appropriate per_dir_config before
calling the real auth providers.  I have not yet heard a reason why this would
be an unsatisfactory solution.  -- justin

Re: [VOTE] 2.1.3 as beta

[Justin Erenkrantz]

On Mon, Mar 07, 2005 at 12:19:45AM -0600, William A. Rowe, Jr. wrote:
> At 07:22 AM 3/6/2005, Sander Striker wrote:
> >I assume we are in agreement that the current AAA discussion shouldn't
> >hold up moving to 2.2 either.
> 
> Absolutely it does.  Either 2.1-dev has made implementing this 
> worse (my essentially workable proposal for 2.0 would no longer 
> work at all, with no workaround) or 2.1-dev has made implementing
> such a feature possible, even trivial, even if it's not part of
> the httpd-2.2 core.  Suggesting we push out 2.2 'as is, whatever'
> would be like having shoved out either Ryan's or Greg's original
> filter stack without the group coming to concensus (and best of
> breed solution.)

I believe that analogy is completely without merit.

The filter stack is at the heart of httpd's module design.  The AAA
functionality that is being discussed is relatively minor and the specific
issue that has been raised has *never* been present before.

When we come to agreement upon a solution for the AAA issue, then, per my
proposal previously sent to [EMAIL PROTECTED], with 3 +1s we can change the API 
in the
beta series.  Yet, I don't believe *any* proposal or suggestion in the AAA
threads so far makes changes the API in any way - it only adds functionality.

Currently, we are being held up by gunpoint because you want to wait for a
perfect beta.  By definition, a perfect release should be a GA not a beta!
So, I reiterate that this one minor issue should not be holding us up from a
beta.  This AAA code has been in the tree for *years* and you are just now
complaining about a feature that has never existed?  I kindly ask that you
please stop getting in the way of forward progress.  -- justin

Re: Multiple AAA providers

[Justin Erenkrantz]

--On Monday, March 7, 2005 5:47 PM -0600 "William A. Rowe, Jr." 
<[EMAIL PROTECTED]> wrote:

Absolutely not my intention.  Again, I do not want to have each
provider have to reimplement the same code and parsing.  I want
a single module to do so, and the providers to be oblivious
(but still work.)
Brad's suggestion satisfies those exact requirements while being at the 
provider layer - which is infinitely more efficient than the AuthConfig 
that has been proposed.

Again, what is specifically unsatisfactory with the idea?  -- justin

Re: [VOTE] 2.1.3 as beta

[Justin Erenkrantz]

--On Monday, March 7, 2005 5:37 PM -0600 "William A. Rowe, Jr." 
<[EMAIL PROTECTED]> wrote:

++1 - and I've always agreed.  My only question is does the new API
make it impossible to do simple things.
...
If the new API makes things more difficult, it's a regression.
This AAA provider discussion just offers us the opportunity to
really evaluate the usefulness of the new API, are things going
to be better or worse when you want to do something beyond add
a single-use provider.
And maybe the API doesn't interfere in which case we prove that
the new API doesn't hinder development, but promotes it.  We
already know the new API makes it simpler to implement SQL, ODBC
or other back ends folks care to create or port to 2.2
Considering that the AAA rewrite we have today does not change *any* 
existing AAA APIs that were in 2.0, but only adds APIs - I find it 
extremely hard to believe that 2.1 can not do things that 2.0 could.  -- 
justin

Re: [VOTE] 2.1.3 as beta

[Justin Erenkrantz]

On Tue, Mar 08, 2005 at 08:49:52AM +0100, Sander Striker wrote:
> So what is failing to build?  2.1.3?  Or trunk?  Only on
> win32, or on other platforms as well?  Can you reproduce?

Note that 2.1.3 fails to build on Win32 because APR 1.1.0 doesn't build.

This Win32 failure is *not* an httpd problem.  Mladen reported that using
APR trunk worked fine with 2.1.3.

The Win32-savvy APR developers need to figure out what's wrong and help us
backport the changes to APR 1.1.x and help us release 1.1.1.   (IMHO, with my
APR hat on, releasing 1.2.0 is not an option for APR as apr_dbd isn't ready.)

I and others have repeatedly said that we refuse to tie *any* httpd release to
a non-released version of APR.  And, I don't like to see httpd -1ed because
APR screwed something up.  -- justin

Re: [PATCH] mod_cache, expand impact of CacheIgnoreCacheControl

[Justin Erenkrantz]

On Tue, Mar 08, 2005 at 03:57:55AM +0100, Sander Striker wrote:
> Hi,
> 
> Currently CacheIgnoreCacheControl On only ignores Cache-Control: no-cache
> and Pragma: no-cache.  I'd like to add ignoring Cache-Control: max-age=...
> and Cache-Control: min-fresh=... as well.
> 
> This would give the admin more control, and would also make the directive
> slightly more intuitive IMO.  This because different browsers do different
> things.  One will send a Cache-Control: no-cache on a refresh, and one will
> send a Cache-Control: max-age=...  It would be nice if the effect would
> be the same for both.
> 
> Thoughts?

While I think this is a good idea, I'd like to consider renaming this
particular directive as I think the name is really confusing.  My concern is
that CacheIgnoreCacheControl only refers to the request's Cache-Control not
the origin response's Cache-Control header.  But, I like that separation
because having it refer to both is too coarse-grained, I think.

But, I don't have any real ideas for what an alternative name is.  -- justin

Re: [PATCH] mod_cache, expand impact of CacheIgnoreCacheControl

[Justin Erenkrantz]

On Tue, Mar 08, 2005 at 06:01:35PM +0100, Sander Striker wrote:
> >While I think this is a good idea, I'd like to consider renaming this
> >particular directive as I think the name is really confusing.
> 
> Does that mean you want me to hold off on committing this patch pending
> a directive rename?  Isn't that a seperate issue?

Nah, go ahead and commit if you like.  It's just that you brought up the point
of making the directive more intuitive - and I have problems from the word go
on this particular directive being intuitive.  It's not.

In order to understand what this directive does, you need to know what
Cache-Control from the RFC means - and that's not intuitive.  I'd like
something that expresses the concept that we will serve cached content even if
the client asks for 'fresh' content.

The closest I can come up with is 'CacheServeStale' - but that's not quite
right or even precise either.

> >My concern is that CacheIgnoreCacheControl only refers to the request's
> >Cache-Control not the origin response's Cache-Control header.  But, I like
> >that separation because having it refer to both is too coarse-grained, I 
> >think.
> 
> Definately agreed.  Ignoring response Cache-Control is in another league 
> than
> ignoring the request Cache-Control.

Correct.

> >But, I don't have any real ideas for what an alternative name is.  -- 
> >justin
> 
> CacheIgnoreServerCacheControl?

Per above, I don't like the phrase Cache-Control.  -- justin

Re: [PATCH] mod_cache, expand impact of CacheIgnoreCacheControl

[Justin Erenkrantz]

--On Tuesday, March 8, 2005 8:12 PM +0200 Eli Marmor <[EMAIL PROTECTED]> 
wrote:

If I recall correctly, there were MANY conditions in mod_cache that
prevented caching (like checking for a POST method, no-store, no-cache,
auth, GET args, private, public, must-revalidate, maxage, etc.).
My idea was to have one directive, with an option for each of them,
including the conditions that are already supported, plus two special
options - one that represents the empty set of options, and one that
turns on all of the options.
Hmm.  That's an interesting approach.  How about an ITERATE directive with 
a bit-wise field that represent their value in the config structure?  I 
sort of like that...  =)

To be clear, something like:
CacheOptions +StorePrivate +IgnoreClientControl +IgnoreServerControl 
+CachePOST +CacheAuth
CacheOptions +all
CacheOptions -all

Feel like writing a patch?  =)  -- justin

Re: [PATCH] mod_cache, expand impact of CacheIgnoreCacheControl

[Justin Erenkrantz]

--On Tuesday, March 8, 2005 9:38 PM +0200 Eli Marmor <[EMAIL PROTECTED]> 
wrote:

It depends if you need it only for the server configuration, or for
dir_config;
In the latter case, you don't have another choice, you just NEED the +-
Actually, cache can't respect any dir config's (because it is a quick 
handler) so Joshua is right - we shouldn't follow the +-.  -- justin

Re: [PATCH] mod_cache, expand impact of CacheIgnoreCacheControl

[Justin Erenkrantz]

--On Wednesday, March 9, 2005 9:47 AM +0200 Eli Marmor <[EMAIL PROTECTED]> 
wrote:

Time to define the exact directive and names?
I'd start with all of the directive that mod_cache currently exposes that 
are binary (on/off).

At a quick glance, that looks like CacheIgnoreCacheControl, 
CacheIgnoreNoLastMod, CacheStoreNoStore, CacheStorePrivate.  For a first 
cut, it probably just makes sense to drop Cache from the prefix and see how 
it goes.  -- justin

Re: towards a 2.05 release

[Justin Erenkrantz]

--On Wednesday, March 9, 2005 10:00 AM -0600 "William A. Rowe, Jr." 
<[EMAIL PROTECTED]> wrote:

With the approach of httpd 2.1-beta (in anticipation of 2.2 GA)
I'd like to propose the httpd project integrate apreq into the
core distribution.  This project has evolved considerably since
it was first considered.
Comment or vote?
In general, I'm not really comfortable with adding a large amount of perl 
code to our tree.  httpd is a C project not a perl project - I'm not 
overjoyed at the prospect of having to maintain perl code.  I, of course, 
am from the camp that anything in our tree I am collectively responsible 
for.

Additionally, I think merging apreq should be something for 2.4 not 2.2. 
Adding it right *now* would cause a lot of delays.  For example, from a 
quick glance, they use automake - which is a big no-no.  So, it'd take a 
lot of effort to get it integrated in our build system.  I really think 
that effort would best be placed in getting a solid 2.2 release out sooner.

If there is still a lot of interest, this can be something we merge in 
right after we branch 2.2.x off into 2.3.x.  And can be an impetus for 
2.4.x's release.

So, -0 in concept.  And, -1 for doing it today - it's not in a state that 
can just slide into our tree at all without issues.  -- justin

Re: [PATCH] mod_cache, expand impact of CacheIgnoreCacheControl

[Justin Erenkrantz]

--On Wednesday, March 9, 2005 7:42 PM +0200 Eli Marmor <[EMAIL PROTECTED]> 
wrote:

That's all?!
Let me quote myself (and this is not the complete list):
If I recall correctly, there were MANY conditions in mod_cache that
prevented caching (like checking for a POST method, no-store, no-cache,
auth, GET args, private, public, must-revalidate, maxage, etc.).
The complete list may be long, but if we want to allow offline caching,
we must precede a condition before any rule of mod_cache.c that
prevents caching in any case, and I don't see any serious difference
(performance, code size, memory size) between "if (conf->something!=0)"
and "if (conf->something & SOMETHING != 0)". So we don't need to have
one directive/bit for many conditions, as long as it is done in a
friendly way for the users (i.e. there are 3-4 pre-defined constants
which mean "cache nothing", "default cache", and "cache everything -
for offline browsing").
As you know, we don't currently have directives for most of these items. 
We can gradually add them.

However, to make your patch easier to review, I'd suggest taking it 
piecemeal - convert the current binary options to a CacheOptions directive 
and then once that is complete and accepted in the tree, we can then add 
new options one at a time.  We like smaller and easier-to-review patches. 
=)

Does this make sense?  -- justin

Re: towards a 2.05 release

[Justin Erenkrantz]

--On Wednesday, March 9, 2005 1:17 PM -0600 "William A. Rowe, Jr." 
<[EMAIL PROTECTED]> wrote:

Bingo.
I see any perl-wrapper as surviving very nicely in mod_perl, given
that it would be reflecting functions exposed from the core distro.
As they mention, the perl wrappers still need refreshing.
AFAICT, that module requires libapreq2.  So, we'd have to bring it all into 
our tree.  So, my -1 for trunk still holds until after 2.2.x is branched 
off and on its way.  I just don't think it is in a state where it'd be easy 
to bring into our tree and not have it delay the beta process unduly.

IMHO, it can wait for 2.4.  -- justin

Re: towards a 2.05 release

[Justin Erenkrantz]

--On Wednesday, March 9, 2005 1:12 PM -0800 Paul Querna 
<[EMAIL PROTECTED]> wrote:

I don't see how an optional module that depends on an external library is
that helpful compared to the current situation. (Module bundled with the
Library.)
It makes the most sense to me to include the library and module if we do
this.  I don't think including both should be done at this time.
That is precisely my feelings as well.  -- justin

Re: ApacheCon 2005 US

[Justin Erenkrantz]

--On Friday, March 11, 2005 9:33 AM + Colm MacCarthaigh <[EMAIL PROTECTED]> 
wrote:

Does anyone know what the story is with the website? I've been trying
to submit a presentation, but the website is down, and I've not gotten
a response from info@ , though admitadly I only tried this last night,
but the deadline, *cough* tomorrow *cough* makes me wonder if there
are any out-of-band submission procedures?
The whole ApacheCon system is AWOL:

I'd imagine they're going to have to extend the deadline again.  -- justin

Re: Rolling 2.1.4...

[Justin Erenkrantz]

On Mon, Mar 14, 2005 at 08:03:43PM -0800, Paul Querna wrote:
> I would like to roll the 2.1.4 alpha right after APR 1.1.1 is released.
> 
> I plan on rolling APR tonight or Tuesday morning.  If there arent any 
> problems, I am hoping to create 2.1.4 on Thursday.  Any big outstanding 
> issues?

As far as I can tell, the Win32 fixes still haven't been backported to APR
1.1.x.  Or, am I missing something?  -- justin

Re: [PATCH] ReceiveBufferSize directive for large POST

[Justin Erenkrantz]

--On Tuesday, March 15, 2005 2:52 PM -0500 Eric Covener <[EMAIL PROTECTED]> 
wrote:

Adds a simple directive (like SendBufferSize) to allow setting the TCP
receive buffer to help accomodate large POSTS.
This is Independent of the ProxyReceiveBufferSize.
Looks fine.  Applied in r157583.  Thanks!  -- justin

Re: 2.1.4 available for testing

[Justin Erenkrantz]

   --On Wednesday, March 16, 2005 10:00 PM +0100 Sander Striker 
<[EMAIL PROTECTED]> wrote:

Hi all,
There are some 2.1.4-alpha tarballs waiting to be tested at:
  http://httpd.apache.org/dev/dist/
Please report back with any problems.
+1 for beta on Darwin (modulo httpd-test issues from last time).
Thanks!  -- justin

Re: do we still want sendfile enabled with our default conf files?

[Justin Erenkrantz]

On Thu, Mar 17, 2005 at 02:12:50PM +, Colm MacCarthaigh wrote:
> On Thu, Mar 17, 2005 at 08:47:08AM -0500, Joshua Slive wrote:
> > +1, as before.
> > 
> > From the users' perspective, sendfile results in unexplained corruption 
> > or uninterpretable error messages pretty much any time a network 
> > filesystem is used to host content (and random other times on win32).
> 
> Judging by my inbox over the last year, similar behaviour on Linux with 
> IPv6 is becoming more and more prevalant, as adoption ramps up. So
> a default of off would be a very great help there too.

These seem like broken OSes and not a suitable justification to disable
sendfile.  We should fix the code - perhaps by teaching APR not to enable the
sendfile-variants on these buggy platforms - not disable it entirely.  For
those platforms that don't have bugs, disabling sendfile would be a ridiculous
performance hit.  -1.  -- justin

Re: do we still want sendfile enabled with our default conf files?

[Justin Erenkrantz]

On Thu, Mar 17, 2005 at 05:37:38PM +, Colm MacCarthaigh wrote:
> On Thu, Mar 17, 2005 at 07:27:54AM -0800, Justin Erenkrantz wrote:
> > These seem like broken OSes and not a suitable justification to disable
> > sendfile.  We should fix the code - perhaps by teaching APR not to enable 
> > the
> > sendfile-variants on these buggy platforms - not disable it entirely.  For
> > those platforms that don't have bugs, 
> 
> It's not simply a question of OS bugs. The Linux/IPv6 problem is
> actually a symptom of TCP checksum-offloading bugs in most network
> interface cards. In an effort to speed up sendfile() even more, Linux

Fine, disable sendfile by default with IPv6 sockets on Linux if you are really
concerned that this is an issue.  And, disable it on Win32 by default if
that's a proven problem there as well.

I'd do this by removing EnableSendfile in our default config and having it
default to, indeed, a 'maybe'/'unset' option that allows httpd to make
run-time determinations.  If it is 'on', then we don't second-guess and if it
is 'off', we never use it.

> > disabling sendfile would be a ridiculous performance hit.  -1.  -- justin
> 
> Is there any quantification of this? I'm not entirely convinced of the
> performance hit, mostly - network writes tend to be network-IO bound no
> matter what, and I've yet to see much of a CPU/Memory hit using my own
> benchmarks, with MMap. But I've only run them on Linux and the ammount
> of Ram I have may be distorting the truth of it.

We did performance tests years ago when we added sendfile into the core
(specifically when I added sendfilev from Solaris).  It helped Solaris
at the time on the hardware we had.

Shutting off sendfile means that OSes that are sane and non-buggy have to pay
the penalty and that's absurd.  And, we should not require admins to
specifically tune the machine for optimum performance because a few buggy OSes
and drivers are at fault.  We'd be throwing the baby out with the bathwater
and that's bad.  -- justin

Re: do we still want sendfile enabled with our default conf files?

[Justin Erenkrantz]

On Thu, Mar 17, 2005 at 07:01:49PM -0500, Jeff Trawick wrote:
> Is that "-1" a vote, or a veto against the idea?  If the latter,
> please explain in at least a little detail how a technical solution
> can be implemented that will avoid some of the types of problems
> triggered by the use of sendfile.  After a year or two that these
> issues have been known, the only thing that anybody truly knows how to
> do is to disable sendfile.

A solution that disables sendfile for OSes that do not have these sendfile
issues (say *BSD and Solaris) are going to get my veto.  If we need to do some
platform/run-time detection to identify those platforms that we know may be
buggy, then we add those in.

This is the first time I can recall a thread that says sendfile is broken.
Perhaps there's been threads and I just don't remember them.  *shrug*

It's bad behavior on our part to turn off features because one or two OSes
have problems.  The proper course of action should be to identify those OSes,
and, if possible, the circumstances that cause the brokenness and have our
code act accordingly.  In fact, I bet all of the run-time checks *could* be
placed inside of APR's apr_socket_sendfile().  -- justin

Re: 2.1.4 available for testing

[Justin Erenkrantz]

On Thu, Mar 17, 2005 at 05:37:01PM -0600, William A. Rowe, Jr. wrote:
> First, I'm -1 on moving 2.1.4 to -beta- until the conflicts 
> between apr-iconv 0.9 and 1.x are resolved.  The apr team is
> currently addressing this.

Would you be against re-rolling 2.1.4 with an updated apr-iconv that uses the
APR_ICONV1_PATH environment variable instead and calling that 2.1.4 tarball
beta?  My understanding is that this change is done entirely within APR-iconv
and does not have any direct effect on httpd.  Therefore, the httpd code
is identical and unchanged: only the dependency is updated.

I'm reluctant to waste yet another release because a dependency has bugs that
have nothing whatsoever to do with httpd.  I also still feel that these are
issues that would be safe to resolve during the beta process.  -- justin

Re: do we still want sendfile enabled with our default conf files?

[Justin Erenkrantz]

--On Friday, March 18, 2005 5:59 AM -0500 Jeff Trawick <[EMAIL PROTECTED]> 
wrote:

...snip, snip...
AIX:
Doesn't really fail in the normal sense of not putting the right data
on the wire, but can trigger a kernel memory issue if some kernel
tuning is incorrect.  So always fail if the APR_SENDFILE_AUTODETECT is
on.  (This kernel tuning is irrelevant unless sendfile or more obscure
TCP usage is actually occuring, so the tuning issue has typically been
there all along without hurting anything.)
Is the kernel turning incorrect on AIX by default?  Will this be fixed in some 
future releases?  You could do lots of things to corrupt your kernel by tuning 
in other ways - so unless this is by default, I can't see why we should block 
this.

...snip, snip...
+1 to this list of exceptions and adding a new flag called APR_SENDFILE_CHECK 
(or APR_SENDFILE_AUTODETECT) to apr_socket_sendfile.  -- justin

Re: do we still want sendfile enabled with our default conf files?

[Justin Erenkrantz]

--On Friday, March 18, 2005 11:12 AM -0500 Ryan Bloom <[EMAIL PROTECTED]> wrote:
funny, I took the list of exceptions to be so large and hard to
maintain that it made more sense to go with Jeff's original idea of
just disabling sendfile by default unless a user specifically decided
to enable it.  I just had to debug a problem for a friend with
sendfile on Linux.  I don't know what caused the problem, but
disabling sendfile solved it immediately.  Seems to me that until our
sendfile support is better, we should err on the side of always
sending the data correctly instead of absolutely as fast as possible.
I absolutely refuse to punish users who are using good OSes because some OSes 
are brain-dead.  This is exactly the role that APR is meant to fill: if we 
know of conditions where it is unsafe to use sendfile, we won't use it unless 
explicitly told so by the user.

The minimal check can be:
if (flags & APR_SENDFILE_CHECK) {
#ifdef LINUX || WIN32 || AIX || HPUX
   return APR_ENOTIMPL;
#endif
}
As people determine what conditions sendfile is safe (or causes problems), 
then we can add those.

Feel free to advocate Linux always returning APR_ENOTIMPL for sendfile - I 
don't care.  However, blocking sendfile on non-buggy OSes is not a solution 
that I am willing to sign off on.  -- justin

Re: do we still want sendfile enabled with our default conf files?

[Justin Erenkrantz]

--On Friday, March 18, 2005 11:51 AM -0500 Jeff Trawick <[EMAIL PROTECTED]> 
wrote:

ouch, add one more brain-dead OS to the list: FreeBSD; some levels
have had filesystems which don't handle sendfile correctly (same type
of issue which hits some Linux users)
Do you have details?  FreeBSD's sendfile was broken for some versions (i.e. 
<4.10 with threading).  We already have configure checks in APR for some of 
this.

I find it amusing that Linux-centric developers feel special in that they 
should disable things across the board because their OS has a bug while the 
FreeBSD-using developers just add in checks to isolate the issue on their 
platform.  =)  -- justin

Re: do we still want sendfile enabled with our default conf files?

[Justin Erenkrantz]

--On Friday, March 18, 2005 11:27 AM -0500 Ryan Bloom <[EMAIL PROTECTED]> wrote:
That's fine.  Pay attention to what I suggested.  Default to
non-native sendfile, until we have know that it works.  If you have an
OS that you know for a fact does sendfile correctly, then that would
be a case where we know that it works.
I tend to prefer Jeff's solution of having APR return APR_ENOTIMPL when the 
APR_SENDFILE_AUTODETECT flag is set and we'd fail.  I'm ambivalent if we 
decide to have apr_socket_sendfile() internally call emulate_sendfile because 
apr_socket_sendfile() has always been an optional function (APR_HAS_SENDFILE). 
If we go this route of having it mask the choice, then apr_socket_sendfile() 
should always be present and we can clean up the code in httpd accordingly.

I also think that we likely already know the cases when sendfile is going to 
succeed on a particular platform.  I haven't heard any claims that sendfile() 
fails on Linux when using only IPv4 and ext{23}.  So, yes, I think we can do 
better than a straight APR_ENOTIMPL - but if people don't want to write the 
checks, then we'll just live with writev() on that platform.  -- justin

Re: do we still want sendfile enabled with our default conf files?

[Justin Erenkrantz]

--On Friday, March 18, 2005 4:34 PM + Colm MacCarthaigh <[EMAIL PROTECTED]> 
wrote:

In my experience, more users are brain-dead than OS's ;) Surely it's the
users who don't have a hope of diagnosing the kinds of intermittent problems
that using sendfile can trigger who should be kept in mind?
Users whose performance requirements are such that they will benefit
from the use of Sendfile will find it, it's not like the option would
dissapear. Even more so if it left on in the high performance config.
The issue is that the APR developers can assist in this by using our knowledge 
of when it is safe to use sendfile() or not.  We should not be tasking 
administrators with this responsibility.  I abhor knobs that have to be turned 
on in order for the system to work to the best of its ability.  It's bad 
behavior on our part: we shouldn't need a 'high performance config'.  If we 
do, then it means that the portability layer failed.

Therefore, we can make it such that the httpd's default configuration does the 
right thing a heck of a lot more often than it does now.  And, if that means 
we disable sendfile() across the board on more platforms - so be it.  -- justin

Re: do we still want sendfile enabled with our default conf files?

[Justin Erenkrantz]

--On Friday, March 18, 2005 11:53 AM -0500 Jeff Trawick <[EMAIL PROTECTED]> 
wrote:

there are fixes available for the default tuning;
So, will we be able to isolate down to a revision of AIX that has these 
fixes?
I have no
information on whether or not users have shot themselves in the foot
in addition to getting bitten by the default tuning
I have little sympathy for users who custom tune their kernel and see 
something bust because of that.  I'm not going to lose sleep over that.  -- 
justin

Re: do we still want sendfile enabled with our default conf files?

[Justin Erenkrantz]

--On Friday, March 18, 2005 12:20 PM -0500 Jeff Trawick <[EMAIL PROTECTED]> 
wrote:

google does; keywords "ntfs" "smbfs" and some other "UDF" ("Universal
Disk Format (UDF) ", which I've never heard of
"ntfs sendfile freebsd" 2nd hit on Google is a patch I sent to FreeBSD lists 
years ago about a sendfile() problem in ftpd.  =)

I don't see any checks in APR by FreeBSD-using developers to try to
figure out if the file being served just might not be served properly
due to filesystem problems.
There are already autoconf checks and run-time version checks for FreeBSD 
sendfile() issues.  Yes, there aren't any run-time checks based on 
filesystems, but that's just never been brought up.

I don't understand where you're coming from on this "brain dead"
"buggy OS" crap.
Every OS is a complete PITA at one point or another.  It is not a
constructive talking point.  We're beyond that and are trying to find
a practical way to avoid some pain for our users.
All I'm saying is that we should put forth a solid effort to have APR deal 
with these cases as best as we can rather than forcing the admin to know the 
details of their operating system.  In fact, I think I'm displaying more 
sympathy to the admins than 'we can't help you - you need to guess'.  -- justin

Re: do we still want sendfile enabled with our default conf files?

[Justin Erenkrantz]

--On Friday, March 18, 2005 5:18 PM + Colm MacCarthaigh <[EMAIL PROTECTED]> 
wrote:

I think it's just one of those cases where it would be highly
non-trivial and inefficient to put all of the checks in APR, simply due
to the real-world nature of the bugs, but that at the same time there is
a clear benefit available to those willing to take the time to read the
manual and decide for themselves if it will work in their situation.
For those OSes that have a large number of cases where sendfile() doesn't 
work, then we can disable sendfile() rather than checking for these cases. 
However, your position mandates that the admins must do a lot of legwork to 
understand if their OS has issues or not.  It's not always obvious what the 
corner cases are here: I'm in favor of placing that work in APR - where there 
are people willing to produce appropriate patches to relax the restrictions on 
that OS.

One fstatfs call will allow us to detect the FS issues we've seen: either 
blacklist or whitelist fstypes per OS, I don't much care.  And, we can check 
for IPv6 sockets on Linux.  Ideally, we'd have some way of minimizing the need 
for fstatfs calls, but even without a cache of some sort (I don't know how 
slow or fast fstatfs tends to be), that's a fair price to pay for correctness 
and a viable attempt to maintain zero-copy performance.

Yet, I believe most of these are outright OS or driver bugs that will, over 
the long run, be fixed upstream.  If your OS or driver maintainers don't fix 
problems with their software, then you have other issues.  -- justin

Re: do we still want sendfile enabled with our default conf files?

[Justin Erenkrantz]

--On Friday, March 18, 2005 7:44 PM + Colm MacCarthaigh <[EMAIL PROTECTED]> 
wrote:

either lacklist or whitelist fstypes per OS, I don't much care.  And, we
can  check for IPv6 sockets on Linux.
This is still unfair on admins. Some network cards work fine, why
shouldn't their owners get the benfits of sendfile?
Those cases they can explicitly use 'EnableSendfile on'.  IIRC, you're the one 
who said most network cards are broken with IPv6 and sendfile on Linux.

The issue here is to be more sane with our default behavior than we are today. 
I think Jeff's patch with a switch in our default httpd.conf to 
'EnableSendfile autodetect' will resolve most people's concerns while 
attempting to allow platforms that have straightforward (or non-expensive) 
checks to still use sendfile().  -- justin

Re: svn commit: r158303 - httpd/httpd/trunk/buildconf

[Justin Erenkrantz]

On Mon, Mar 21, 2005 at 05:19:56PM -0600, William A. Rowe, Jr. wrote:
>  I'm not keen on this change, since it complicates things
> unnecessarily - some day we discover a tag and roll organized like 
> this out of the blue?
> 
> Does config.nice not do what you want?  Especially if you rename
> it config.me with all your absolute options that you don't tweak?

Since moving to Subversion, having apr and apr-util at the same level as httpd
is my typical working copy layout, so I'm definitely +1 on this change.
srclib/apr and srclib/apr-util don't really make sense anymore, IMO.  -- justin

Re: svn commit: r158303 - httpd/httpd/trunk/buildconf

[Justin Erenkrantz]

--On Tuesday, March 22, 2005 3:22 PM -0600 "William A. Rowe, Jr." 
<[EMAIL PROTECTED]> wrote:

Of course!  Now I'm on the same page with you.  Actually,
I believe a buildconf.nice is a better solution (for reasons
in my other note.)  We really have no say-so about what is
sitting in the directory above our httpd snapshot.
It only matters if the directory is present and contains the files we need. 
I just think a buildconf.nice would be overkill here.  -- justin

Re: "Supported" Compilers

[Justin Erenkrantz]

--On Wednesday, March 23, 2005 12:11 PM -0500 Brian Akins 
<[EMAIL PROTECTED]> wrote:

Is there a list of "supported" compilers?  I am having to compile using
gcc 2.96 and having some wierdness, but works fine on 3.3.  It may be
something else with the box, but just wanted to know if there was an
"official" list.
It should work fine with gcc 2.96.  What errors are you seeing?  -- justin

Re: So what is the real status of 2.1.x...

[Justin Erenkrantz]

--On Tuesday, March 29, 2005 11:22 AM -0700 Brad Nicholes 
<[EMAIL PROTECTED]> wrote:

Are we BETA yet or not?  I am assuming that the true status is:
OtherBill has consistently repeated that he will -1 anything entering beta. 
So, until he resolves his issues, we're at a standstill.

My current understanding is that OtherBill's -1 has some thing to do with 
APR-iconv and nothing at all with httpd.  As such, there's absolutely 
nothing [EMAIL PROTECTED] can do to remove his -1.  Every time I have tried to 
remove his stated arguments against going beta (I lost count at 4 different 
rationales against beta), OtherBill suddenly presents more arguments as to 
why httpd can't enter beta.

I'm frankly very tired of shooting at moving targets and trying to persuade 
folks that we should have entered beta when we have committers looking for 
excuses to prevent us from making forward progress.  Hence, any desire that 
I had to see 2.2 start the release cycle is diminished because I don't have 
time for more arguments as to why we should start the beta cycle.

I still maintain that the current state of trunk is sufficient to branch 
off 2.2.x (keeping the branch version at 2.1.x until we go GA with 2.2.0) 
and consequently bump trunk to 2.3.x today.  -- justin

Re: [PATCH] mod_proxy_http

[Justin Erenkrantz]

--On Wednesday, March 30, 2005 7:40 PM +0200 Sander Striker 
<[EMAIL PROTECTED]> wrote:

In short: Some eyes please...
+1.  Looks fine to me.  -- justin

Re: simple-conf branch

[Justin Erenkrantz]

--On Saturday, April 2, 2005 2:58 PM -0500 Joshua Slive <[EMAIL PROTECTED]> 
wrote:

If you have suggestions for the implementation, speak up or just commit
away on the branch.
Thanks for taking this on.
Here are some things I would like to see done on this branch.  Feel free
to jump in.
1. Fix "make install" to deal with the extra/ directory.  Probably need
to change the name of all those config files to add "-std".  (I suggest
making another directory called "standard" or "as-installed" or something
to put the pristine copies of all the config files.)
2. Various tweaks on comments/ordering/etc to make things clearer.  I
believe the main httpd.conf can still be paired down a little more.
Some thoughts on what else we could trim.  In fact, most of my comments 
could move to a 'httpd-default.conf' that represent what our hard-coded 
defaults are.

Do we really need LockFile, ScoreBoardFile?
Do we really need PidFile?  The MPMs default to:
#define DEFAULT_PIDLOG DEFAULT_REL_RUNTIMEDIR "/httpd.pid"
Which is the same.  So, I'd toss PidFile in to httpd-default.conf.
Same goes for Timeout and KeppAliveTimeout as DEFAULT_TIMEOUT is 300 and 
DEFAULT_KEEPALIVE_TIMEOUT is 15.  Ditto for MaxKeepAliveRequests and 
KeepAliveTimeout.

AccessFileName defaults to DEFAULT_ACCESS_FNAME anyway.
I wouldn't have ServerTokens and/or ServerSignature in the default 
httpd.conf.  They can read docs for that.  =)  (Perhaps being in 
httpd-default.conf.in is fine though - not sure.)

If these changes look fine to folks here, I can make these changes on the 
branch.  -- justin

Re: simple-conf branch

[Justin Erenkrantz]

--On Monday, April 4, 2005 1:21 PM -0400 Jeff Trawick <[EMAIL PROTECTED]> 
wrote:

fairly common to have two web server instances using same ServerRoot;
that's another case to use LockFile, so that difference instances use
different lock...  no big deal with fcntl, big deal with flock...
I ended up not touching LockFile/ScoreboardFile in r160063.  I'm just not 
wild about having huge swaths of commented-out directives in the httpd.conf 
if we intend for it to be simple.  =)  -- justin

Re: simple-conf branch

[Justin Erenkrantz]

On Mon, Apr 04, 2005 at 03:01:34PM -0700, Greg Stein wrote:
> Sorry, but I very much disagree. I think back to the old days of
> access.conf, httpd.conf, and srm.conf. As an administrator, I absolutely
> detested that layout. I could NEVER figure out which file a given
> configuration was in. I always had to search, then edit.
 
Some of us have never left that world.  Darnit.

> We've been to the "multiple .conf world" before. It sucked. We pulled
> everything back into a single .conf to get the hell outta there.

Agreed.

> Small examples are fine. The default configuration should remain as a
> single .conf file.

+1.  -- justin

Re: simple-conf ready for merge

[Justin Erenkrantz]

--On Wednesday, April 6, 2005 10:42 AM -0400 Joshua Slive <[EMAIL PROTECTED]> 
wrote:

I'm pretty-much done shredding the default config and I will give a
couple days for review before I merge it.  Feel free to correct any
problems you see.
+1 to merge back to trunk.  (14k vs. 38k.  Yay!)
One last thought: do we really need 'UseCanonicalName Off' - isn't our 
unset default equivalent?  Same goes for HostnameLookups (the unset default 
is Off).  IMHO, these seem more appropriate for httpd-default.conf.  -- 
justin

Re: Default Modules

[Justin Erenkrantz]

--On Wednesday, April 6, 2005 12:29 PM -0400 Rich Bowen 
<[EMAIL PROTECTED]> wrote:

How about mod_ssl being on in "most"?
In the past, we've said that SSL must be explicit because of the crypto 
legal restrictions.  -- justin

Re: Default Modules

[Justin Erenkrantz]

--On Wednesday, April 6, 2005 9:15 AM -0700 Paul Querna 
<[EMAIL PROTECTED]> wrote:

I changed mod_imap this morning from 'yes' to 'most', because I was tired
of disabling it every time I do a new install.  I think we should
reconsider what modules are enabled by default.  Here is my list of
suggested changes:
mod_version: all -> yes
Sure.
mod_asis: yes -> no
mod_imap: most -> no
I would prefer we keep mod_imap as most.  Probably the same for mod_asis. 
These were default modules in 2.0 - therefore, I think disabling them 
unless explicit in 2.2 could be worrisome.

mod_dumpio: most -> all
mod_dumpio shouldn't have been most as it is debugging only...
mod_rewrite: most -> yes
I would rather prefer we make mod_rewrite 'no', but that's me.  =)
I'm also fine promoting mod_proxy and mod_cache to most in 2.2.  -- justin

Re: svn commit: r158798 - in httpd/httpd/trunk: CHANGES server/protocol.c

[Justin Erenkrantz]

--On Wednesday, March 23, 2005 4:36 PM + [EMAIL PROTECTED] wrote:
another @@ -1008,7 +1023,15 @@
 rnew->status  = HTTP_OK;
-rnew->headers_in  = r->headers_in;
+/* did the original request have a body?  (e.g. POST w/SSI tags)
+ * if so, make sure the subrequest doesn't inherit body headers
+ */
+if (r->read_length) {
+clone_headers_no_body(rnew, r);
+} else {
+/* no body (common case).  clone headers the cheap way */
+rnew->headers_in  = r->headers_in;
+}
As I just noted in STATUS for 2.0, read_length isn't a sufficient check. 
It'd only be set if the client has *already* read the body *and* they used 
the 1.3.x mechanisms for reading the request body.

The proper check is to look for Transfer-Encoding and Content-Length in the 
request headers.  This is what ap_http_filter in http_filters.c does to 
look for a request body.  -- justin

Re: Default Modules

[Justin Erenkrantz]

--On Wednesday, April 6, 2005 1:30 PM -0400 Rich Bowen <[EMAIL PROTECTED]> 
wrote:

Have you ever used mod_imap? Or, at least, since 1996? I have a hard
Yes.  Yes.
I do wish it were renamed to mod_imagemap though!  mod_imap is a poor name.
Note that we could always re-introduce the imagemap CGI program.  ;-)
*chuckle*  -- justin

Re: simple-conf ready for merge

[Justin Erenkrantz]

--On Wednesday, April 6, 2005 12:49 PM -0500 "William A. Rowe, Jr." 
<[EMAIL PROTECTED]> wrote:

The reason not to drop them is that when the gods of httpd
([EMAIL PROTECTED]) decide to change their minds about the 'default'
choice, it doesn't harm existing installations which were
explicitly configured tested and deployed.
Isn't that a good thing?  If the admin doesn't know any better, why should 
our prior mistakes come back to haunt us?  -- justin

Re: svn commit: r160313 - httpd/httpd/branches/2.0.x/STATUS

[Justin Erenkrantz]

--On Wednesday, April 6, 2005 12:54 PM -0500 "William A. Rowe, Jr." 
<[EMAIL PROTECTED]> wrote:

At 12:17 PM 4/6/2005, you wrote:
Author: jerenkrantz
Remove merged backport, block one, vote for one.
@@ -382,6 +381,7 @@
  non experimental status.
  +1: bnicholes, wrowe
  +0: minfrin (wait till the last cache bugs are ironed out)
+  -1: jerenkrantz
Technical justification for veto?  Or is this just a straightforward
objection for majority vote?
I took that section to be majority vote not veto.  -- justin

Re: 2.0.54 release candidate tarball available for testing

[Justin Erenkrantz]

--On Tuesday, April 12, 2005 9:25 AM +0200 Sander Striker 
<[EMAIL PROTECTED]> wrote:

Hi,
As usual the tarballs are at:
  http://httpd.apache.org/dev/dist/
Please give 2.0.54 a whirl and report any problems (or successes) ;)
+1.
Passes httpd-test on Darwin with the unexpected success on include (#51) - 
I'm guessing that the test framework needs to be updated to reflect a 
recent backport?

Thanks!  -- justin

Re: AP_MODE_EATCRLF considered indigestible

[Justin Erenkrantz]

On Fri, Apr 15, 2005 at 10:16:29AM -0500, Rici Lake wrote:
> Perhaps I wasn't being clear. AP_MODE_EATCRLF is not a renaming
> of AP_MODE_GETLINE; it is a renaming of AP_MODE_PEEK. (The renaming
> happened some three years ago in revision 92928, obviously I'm not
> al tanto.) To be more precise, it's a renaming of what the behaviour
> of AP_MODE_PEEK was.
> 
> The documented behaviour of AP_MODE_EATCRLF is:
> /** The filter should implicitly eat any CRLF pairs that it sees. */

Remember that this is for older browsers which send extra CRLFs after each
request.

> The actual behaviour (at least in the only implementation of it in the 
> tree) is somewhat more complex:
> 
> 1) Regardless of the readtype, it only reads non-blocking
> 2) Similar to MODE_SPECULATIVE, it retains data for subsequent reads 
> (but see below)
> 3) It does not, however, ever return any data.
> 4) It returns APR_SUCCESS only if data is immediately available. In 
> determining whether or not data is immediately available, it ignores 
> ([CR]?[LF})+ (but with a bug as noted in my first message)
> 5) If the only data immediately available is ignored line-endings as 
> per (4), these line endings are not retained for subsequent reads. 
> (Obviously, this cannot be relied upon to delete leading CR?LF's, since 
> the scan only applies to currently available data.)
> 
> My argument is that this is extremely idiosyncratic behaviour and has 
> no place in a reasonable interface design. It seems to me that the only 
> consumer of AP_MODE_EATCRLF could just as well use AP_MODE_SPECULATIVE 
> (which is implemented by the only provider which implements 
> AP_MODE_EATCRLF) to do what it wants to do. Consequently, 
> AP_MODE_EATCRLF could just be honourably dispatched to the retirement 
> home for awkward interfaces, and imho we'd all be better off.

Yes, PEEK could now be rewritten using the speculative reads.  When I renamed
PEEK and added speculative reads, there was a lot of other more important and
lower-hanging fruit to deal with.  ;-)

> The following (as-yet-untested) code should do the trick. It is 
> somewhat longer than the existing check_pipeline_flush but that needs 
> to be balanced against the removal of an equal-sized chunk of 
> server/core_filters.c and the end of AP_MODE_EATCRLF:
> 
> static void check_pipeline_flush(request rec *r)
> {
> apr_bucket *e
> apr_bucket_brigade *bb;
> conn_rec *c = r->connection;
> bb = apr_brigade_create(r->pool, c->bucket_alloc);
> if (c->keepalive != AP_CONN_CLOSE) {
> /* The 8 is entirely arbitrary. 4 is probably sufficient */
> if (ap_get_brigade(r->input_filters, bb, AP_MODE_SPECULATIVE,
>APR_NONBLOCK_READ, 8) == APR_SUCCESS) {

The problem with this code is that it'll only read 8 characters worth of
CRLFs.  We'd probably want to retry the reads until we don't get more CRLF
pairs.  It's not determinate how many extra CRLFs we will get.

So, yes, I'd be fine with removing EATCRLF for 2.2 if you provide a tested
patch.  ;-)  -- justin

Re: AP_MODE_EATCRLF considered indigestible

[Justin Erenkrantz]

On Fri, Apr 15, 2005 at 11:36:07AM -0500, Rici Lake wrote:
> Are there browsers which send more than four extraneous CRLF's but 
> still support keepalive?
> 
> If this code reads four CRLF's without finding any data, it will assume 
> that there is no immediate incoming request, and send a flush down the 
> filter chain. That does no harm aside from wasting some cycles; on the 
> assumption that the number of cycles wasted is not that enormous and 
> the number of browsers which exhibit that particular behaviour is not 
> huge, I'd say it's not really worth completely retrying the read.

True.

> I believe that the current code may exhibit the opposite behaviour: it 
> can falsely report that data is present and thereby omit the flush with 
> the result that a response is not terminated until the following 
> request. If the connection is not being pipelined, the following 
> request is unlikely to show up until the response is terminated, so 
> that would lead to a request which stalls very near to the end. I 
> believe I may have actually seen this symptom in real life, now that I 
> think about it.
> 
> >So, yes, I'd be fine with removing EATCRLF for 2.2 if you provide a 
> >tested
> >patch.  ;-)  -- justin
> 
> OK, I'll start by at least compiling and testing what I posted.

Just remember that you might want to go straight to the connection filters and
bypass all of the request and protocol filters.  That would likely save a
bunch of cycles.  -- justin

Re: AP_MODE_EATCRLF considered indigestible

[Justin Erenkrantz]

On Fri, Apr 15, 2005 at 11:56:38AM -0400, Greg Ames wrote:
> Rici Lake wrote:
> >I was taking a look at the implementation of the renamed (but still 
> >misleading) AP_MODE_EATCRLF, 
> 
> AP_MODE_PEEK was more accurate, but whatever...

No, it certainly wasn't, but let's not re-open old wounds.  ;-)

> the reason that this and the corresponding 1.3 BUFF logic exists is to 
> minimize "tinygrams" - ip packets that are less than a full mtu size.  
> tinygrams definately degrade network performance and trigger things like 
> Nagle's algorithm.  in this particular case we are trying to combine 
> response data from two or more different pipelined requests into a single 
> packet.

I'm not sure this optimization is really necessary any more.  It seems overly
complicated given that most responses span multiple IP packets.  And, we
specifically disabled Nagle anyway.

> however our post-filters implementation is very costly in terms of cpu.  we 
> make an extra trip down both filter chains. on the first trip down the 
> output filter chain we are almost ready to call apr to transmit the data 
> when the core output filter stashes the output data, temporarily abandons 
> it, and unwinds.  then we go down the input filter chain with 

This will happen for anything less than AP_MIN_BYTES_TO_WRITE anyway.

> AP_MODE_EAT_CRLF to see if there is more input data stashed (and do an 
> extra socket read() syscall that doesn't happen in 1.3).  assuming the 
> answer is no (typical) we send a flush bucket down the output filter chain, 
> get back to the core output filter, encounter numerous cache misses reading 
> all the instructions and data back into the cpu caches to pick up where we 
> left off, then finally hand off the response data to the apr socket 
> functions.
>
> what I'd like to see instead is for the input filter chain to keep track of 
> whether there is any stashed input any time it is called.  then the core 

There's no real way to do that across all filters.  (Nor should there be,
IMHO.)

> output filter could do a simple check on the first trip down the output 
> filter chain to see whether it's ok to transmit.  the downside is that we 
> may have accumulated extra baggage that is dependent on AP_MODE_EAT_CRLF or 
> flush buckets, so this change would definately need to wait for a version 
> boundary.  in the mean time we should avoid adding any new function 
> dependent on AP_MODE_EAT_CRLF or standalone flush buckets.

Dorky idea: we could move the pipeline ready check to the if EOS clause in
ap_pass_brigade where we mark eos_sent.  If there is no data ready (via a
speculative read: I see no way to not do that), then we add the flush bucket.
We'd also take out that EOS check for the deferred writes in
core_output_filter as we're doing the check earlier on.  So, by the time we
hit core_output_filter, we already know if we should hang on to the
connection.

This would save us from the extra round trip.  I'm not sure where else we
could even place such a check besides ap_pass_brigade.  -- justin

Re: AP_MODE_EATCRLF considered indigestible

[Justin Erenkrantz]

On Fri, Apr 15, 2005 at 10:09:39AM -0700, Justin Erenkrantz wrote:
> This would save us from the extra round trip.  I'm not sure where else we
> could even place such a check besides ap_pass_brigade.  -- justin

Thinking about this a little bit more:

There's no reason we couldn't do the following in ap_http_header_filter:

1. If ap_set_keepalive() decides to enable keepalive (AP_CONN_KEEPALIVE),
   add a 'pipeline keepalive check' output filter as the last request-level
   filter.
2. This pipeline check simply passes through all data until it sees an EOS.
   If it sees an EOS, then it knows that the request is done and that it
   should perform the nonblocking read().  If there is no data, then it 
   adds a FLUSH bucket.  In all cases, it'd strip the EOS from the brigade.

One nice side effect of this is that we could remove the annoying EOC bucket
convention and go back to EOS meaning 'close' to connection-level filters.

What do you think?  -- justin

Re: AP_MODE_EATCRLF considered indigestible

[Justin Erenkrantz]

On Fri, Apr 15, 2005 at 03:46:37PM -0400, Greg Ames wrote:
> it is sounding better all the time as far as performance.  if I understand 
> you correctly I think this one eliminates the extra trips down the input 
> and output filter chains.  but unfortunately we still have the extra read() 
> compared to 1.3, and what about data stashed in the input filter chain?

Well, I think the extra read() is required if we want to see if there is a
pending request in the pipeline (heh).  I wouldn't see a way to know that
without that read() - note that if we've already read the data from the socket
(in our previous core input filter reads), we won't have to go to the network
as the brigade contains the previously read() data in a heap bucket ('stashed
away').  But, in the common case where there isn't a request in the pipeline,
then it would call read() - albeit nonblocking.  I'm guessing 1.3 just didn't
even bother doing a read() on the socket...which sort ofbegs the question, why
are we doing that in 2.x then?

But, moving it to a request output filter triggered by EOS presence does
eliminate one round trip through the filter chain which can save CPU cycles.

So, I guess what we should do is let Rici finish up the patch to implement
EATCRLF entirely via speculative reads - ensure that works and then we can
move around that code as necessary.  -- justin

Re: mod_cache caching the 301 Moved Permanently

[Justin Erenkrantz]

On Thu, Apr 21, 2005 at 10:04:54AM +0530, Devendra Singh wrote:
> Hi,
> 
> I am writing to the Developer List because I did not get any response on 
> the Users List and thought that the topic might be relevant to the dev list.
> 
> If a request comes for a directory w/o trailing slash, it gets cached and 
> the subsequent requests see:
> 
> Moved Permanently
> The document has moved here
> 
> for try to access the URL:
> http://www.beach-clothing.com/where-to-buy

I don't get it.  What's your problem?  -- justin

Re: Proposed patch: always cleanup brigades in ap_pass_brigade

[Justin Erenkrantz]

--On Monday, April 25, 2005 12:47 PM -0500 Rici Lake <[EMAIL PROTECTED]> 
wrote:

Regardless of any other changes to the brigade API, this seems to me to
be a good idea:
Index: util_filter.c
===
--- util_filter.c   (revision 158730)
+++ util_filter.c   (working copy)
@@ -500,6 +500,7 @@
  AP_DECLARE(apr_status_t) ap_pass_brigade(ap_filter_t *next,
   apr_bucket_brigade *bb)
  {
+apr_status_t result = AP_NOBODY_WROTE;
  if (next) {
  apr_bucket *e;
  if ((e = APR_BRIGADE_LAST(bb)) && APR_BUCKET_IS_EOS(e) &&
next->r) {
@@ -523,9 +524,10 @@
  }
  }
  }
-return next->frec->filter_func.out_func(next, bb);
+result = next->frec->filter_func.out_func(next, bb);
+apr_brigade_cleanup(bb);
  }
-return AP_NOBODY_WROTE;
+return result;
  }
This introduces a really large overhead as we'd be calling the cleanup for 
*every* output filter we invoke.  So, -0..  -- justin

Re: Proposed patch: always cleanup brigades in ap_pass_brigade

[Justin Erenkrantz]

--On Monday, April 25, 2005 3:06 PM -0500 Rici Lake <[EMAIL PROTECTED]> 
wrote:

Can you quantify that "large overhead"? In the case of a 'compliant'
filter, it consists of:
With Joe's suggestion for 'creator must destroy', this step is wholly 
unnecessary and only overcomplicates things.   -- justin

Re: For review: teach mod_include.c to recycle brigades

[Justin Erenkrantz]

--On Monday, April 25, 2005 4:47 PM -0500 Rici Lake <[EMAIL PROTECTED]> 
wrote:

explicit cleanup, while we continue thrashing out that issue. (nd -- do
you have a test suite, by any chance?)
httpd-test's has a bunch of tests for mod_include.  -- justin

Re: Proposed patch: always cleanup brigades in ap_pass_brigade

[Justin Erenkrantz]

--On Monday, April 25, 2005 9:34 PM -0700 Paul Querna <[EMAIL PROTECTED]> 
wrote:

Plus, I hear those new dual core opterons are really nice.  I would
rather have a better and cleaner API, than save a few CPU cycles here.
The posted patch does nothing to make the brigades or filter a cleaner API. 
All we'd have then is that ap_pass_brigade() does some 'magic' that has 
nothing at all to do with passing a brigade.  Yuck.  -- justin

Re: [PROPOSAL] Branch 2.1.x on May 13

[Justin Erenkrantz]

--On Friday, April 29, 2005 3:45 PM -0700 Paul Querna 
<[EMAIL PROTECTED]> wrote:

I am proposing the branch date be May 13, 2005.
++1.  -- justin

Re: [PROPOSAL] Branch 2.1.x on May 13

[Justin Erenkrantz]

--On Monday, May 2, 2005 3:26 PM -0400 Jim Jagielski <[EMAIL PROTECTED]> 
wrote:

I'm +1 for branching 2.2-alpha... However, there are 2 outstanding
show-stoppers. Do we expect these to be addressed before the branch.
I think so, especially if they require API changes
Why couldn't we fix those up after the branch?  The point would be to stop 
making 2.1.x a moving target so that we can fix the showstoppers.  -- justin

Re: [PROPOSAL] Branch 2.1.x on May 13

[Justin Erenkrantz]

--On Monday, May 2, 2005 3:33 PM -0400 Jim Jagielski <[EMAIL PROTECTED]> 
wrote:

I thought the whole idea about having a 2.1 dev version was to avoid
monkeying around with the API and the problems when we were doing
1.3 and 2.0. Once we branch, it is possible that we'll run into
issues that may require a bump, but I think entering into a 2.2
tree "expecting" one may not be prudent. And it's only the
2nd showstopper which could be considered "valid" enough to
delay the branch.
Paul proposed branching 2.1.x not 2.2.x.  -- justin

Re: [VOTE] 2.2.0 Alpha on Friday

[Justin Erenkrantz]

--On Friday, May 13, 2005 9:07 PM +0200 André Malo <[EMAIL PROTECTED]> wrote:
Instead of calling it branches/2.1.x, on IRC wrowe suggested going
straight to branches/2.2.x, and on further thought I agree.
I don't agree.
Votes on going straight to 2.2.0-alpha?
-0.5 on calling it 2.2.x.
I'm seeing it like this:
Once forked off, 2.1.x would be *stabilizing* branch, that finally leads
to a 2.2.x branch, when we feel, it's stable (svn mv 2.1.x 2.2.x?). From the
2.1.x branch we tag alpha and beta releases; from *stable* 2.2.x rc and
stable release. I think that's exactly the point of the odd/even system -
2.2.0 being a GA version.
Correct.
We should create branches/2.1.x - when we decide that branch is GA, then we 
rename the directory to 2.2.x and bump the version number inside of there 
accordingly.

Under no circumstances should we release anything called 2.2.0 until it is GA.
I see (now :-) that we should have already branched 2.1.x the first time we
released a 2.1 version.
Per the discussion at ApacheCon, it was said that we would vote on a release 
that would form the creation of the new branch.  However, all of the 2.1.x 
releases I created were vetoed.  -- justin

Re: [VOTE] Commit Policy on 2.2.x

[Justin Erenkrantz]

--On Wednesday, May 11, 2005 6:49 PM -0700 Paul Querna <[EMAIL PROTECTED]> 
wrote:

I propose the following policy apply to the 2.2.x branch, once (and if)
it is created:
Before GA: A 'soft' CTR.  Any small bug fixes can be directly committed.
  Any API changes must be reviewed by the list. (lazy consensus).  Any
large changes must get voted on.
Per my previous email that I sent to the list a while back, I would prefer 
that a stabilization branch require that binary API changes get prior approval 
through RTC (i.e. *not* lazy consensus).  Any changes that are committed to 
the trunk during this process that do not modify the binary API can be 
committed via lazy consensus (i.e. CTR).

The governing factor is changes to the API must be voted on before it can be 
merged into the stabilization branch.

After first GA: Standard RTC, as done on 2.0.x.
+1.  -- justin

Re: [PATCH] mod_cache, don't always run as a quick handler.

[Justin Erenkrantz]

On Thu, May 12, 2005 at 11:34:38AM -0500, William A. Rowe, Jr. wrote:
> Let me suggest, instead, that every proxy between here and
> Timbuktu suffers the same problem from your example.
> 
> Better, methinks, is for mod_rewrite to toggle Vary: for the
> envvar that triggered its rewrite.  This is symptomatic of a
> more sinister side effect of mod_rewrite.

Exactly my thoughts as well.  If the Vary header were in the response, then
mod_cache would act accordingly.

Without that header, every HTTP cache in between the client and the server
would operate identically to mod_cache as a quick_handler.  So, this doesn't
actually fix the problem if mod_rewrite is lame.  In short, this patch doesn't
fix anything and only hides the underlying cause.  -- justin

Re: problems w/ geronimo wiki (cache???)

[Justin Erenkrantz]

On Mon, Jun 06, 2005 at 03:40:41PM +0200, Sander Striker wrote:
> Justin, how's your stack of round tuits?

Not plentiful at the moment.  =(

Can we keep the caching running on port 81 on ajax?  That'll make it easier to
debug if I do get some time.  We'd really need to see what the
request/response chain is.  I thought someone said that MoinMoin doesn't know
how to handle 304s - if so, this doesn't make much sense.  -- justin

Re: svn commit: r180333 - /httpd/httpd/branches/ssl-fips-dev

[Justin Erenkrantz]

--On Monday, June 6, 2005 8:24 PM +0200 Sander Striker <[EMAIL PROTECTED]> 
wrote:



Personally I prefer entire tree branches.


Agreed.  This setup is going to make it difficult to test.  -- justin

Re: svn commit: r188837 - /httpd/httpd/branches/fips-dev/srclib/apr

[Justin Erenkrantz]

--On Tuesday, June 7, 2005 6:16 PM + [EMAIL PROTECTED] wrote:


Author: wrowe
Date: Tue Jun  7 11:16:12 2005
New Revision: 188837

URL: http://svn.apache.org/viewcvs?rev=188837&view=rev
Log:
Sandbox of apr/trunk/ for fips integration. Here may lurk fips issues
(MD5 etc)

Added:
httpd/httpd/branches/fips-dev/srclib/apr/
  - copied from r188836, apr/apr/trunk/


I realize we *can* do this.  However, I'm iffy on cross-project branches 
like this.  -- justin

Re: problems w/ geronimo wiki (cache???)

[Justin Erenkrantz]

--On Monday, June 6, 2005 4:01 PM +0200 Sander Striker <[EMAIL PROTECTED]> 
wrote:



So this is the same problem reported here:
http://mail-archives.apache.org/mod_mbox/httpd-dev/200505.mbox/%3c428F94
[EMAIL PROTECTED]

which Sander said he was working on.  Any progress?  The cache is kind
of useless at the moment, dogfood-wise.


In short, with details left out:  The problem was that the 304 was not
propagated down into the filterstack; leaving the CACHE_SAVE filter no
chance to morph the 304 into a full response in case the client did a
non-conditional request.

This problem was solved, but apparently we have regressed.


So, I had a chance to look at it.  It's the same problem but in a different 
module.  cgi_handler never sends an EOS down the filter chain when it gets 
a 304 (anything non-200).  The problem above was that mod_proxy wouldn't 
send an EOS either in this same case.


We really need a way to generically solve this problem to ensure that an 
EOS is always sent for every request when the request's filter chains are 
still active.


The short-term solution is to get cgi_handler to send the EOS in 
cgi_handler (circa line 924 in the log_script path); but there really has 
to be a better way.  For now, I'll commit a fix that matches what we did 
with mod_proxy; but this pattern keeps emerging.  -- justin

Re: problems w/ geronimo wiki (cache???)

[Justin Erenkrantz]

--On Tuesday, June 7, 2005 1:50 PM -0700 Justin Erenkrantz 
<[EMAIL PROTECTED]> wrote:



to be a better way.  For now, I'll commit a fix that matches what we did
with mod_proxy; but this pattern keeps emerging.  -- justin


I'm not going to commit it yet.  However, I'm building a new httpd on ajax 
with the following patch.  -- justin


Index: mod_cgi.c
===
--- mod_cgi.c   (revision 188838)
+++ mod_cgi.c   (working copy)
@@ -921,7 +921,15 @@
int ret;

if ((ret = ap_scan_script_header_err_brigade(r, bb, sbuf))) {
-return log_script(r, conf, ret, dbuf, sbuf, bb, script_err);
+ret = log_script(r, conf, ret, dbuf, sbuf, bb, script_err);
+
+/* Pass EOS bucket down the filter chain. */
+apr_brigade_cleanup(bb);
+b = apr_bucket_eos_create(c->bucket_alloc);
+APR_BRIGADE_INSERT_TAIL(bb, b);
+ap_pass_brigade(r->output_filters, bb);
+
+return ret;
}

location = apr_table_get(r->headers_out, "Location");

Re: svn commit: r188846 - in /httpd/httpd/trunk: include/mpm_common.h server/mpm_common.c

[Justin Erenkrantz]

--On Tuesday, June 7, 2005 7:13 PM + [EMAIL PROTECTED] wrote:


+APR_HOOK_STRUCT(
+#if AP_ENABLE_EXCEPTION_HOOK
+APR_HOOK_LINK(fatal_exception)
+#endif
+APR_HOOK_LINK(monitor)
+)
+


gcc (at least the one on RHEL3) forbids the use of preprocessor 
conditionals inside of a macro.  So, this needs to be:


#if AP_ENABLE_EXCEPTION_HOOK
...define with it in there...
#else
...define without fatal_exception...
#endif

Thanks.  -- justin

Re: problems w/ geronimo wiki (cache???)

[Justin Erenkrantz]

--On Tuesday, June 7, 2005 2:01 PM -0700 Justin Erenkrantz 
<[EMAIL PROTECTED]> wrote:



--On Tuesday, June 7, 2005 1:50 PM -0700 Justin Erenkrantz
<[EMAIL PROTECTED]> wrote:


to be a better way.  For now, I'll commit a fix that matches what we did
with mod_proxy; but this pattern keeps emerging.  -- justin


I'm not going to commit it yet.  However, I'm building a new httpd on
ajax with the following patch.  -- justin


The patch needed one more tweak to set the status before calling the 
filters.  So far, it looks okay on ajax off: http://wiki.apache.org:81/


Perhaps we should try again to stick it into production?  Heh.  -- justin

Index: mod_cgi.c
===
--- mod_cgi.c   (revision 188864)
+++ mod_cgi.c   (working copy)
@@ -921,7 +921,18 @@
int ret;

if ((ret = ap_scan_script_header_err_brigade(r, bb, sbuf))) {
-return log_script(r, conf, ret, dbuf, sbuf, bb, script_err);
+ret = log_script(r, conf, ret, dbuf, sbuf, bb, script_err);
+
+/* Set our status. */
+r->status = ret;
+
+/* Pass EOS bucket down the filter chain. */
+apr_brigade_cleanup(bb);
+b = apr_bucket_eos_create(c->bucket_alloc);
+APR_BRIGADE_INSERT_TAIL(bb, b);
+ap_pass_brigade(r->output_filters, bb);
+
+return ret;
}

location = apr_table_get(r->headers_out, "Location");

Re: problems w/ geronimo wiki (cache???)

[Justin Erenkrantz]

--On Tuesday, June 7, 2005 11:42 PM +0200 [EMAIL PROTECTED] wrote:


Sorry, if this is a stupid quickshot, but what about something like:


Nope, it's not stupid as that's about the only sane place I can think of 
sticking it as well.  The only catch is that if a handler exits this way, 
it probably means that r->status isn't correct either.  So, perhaps we also 
set r->status explicitly in this additional code path.  -- justin

Re: problems w/ geronimo wiki (cache???)

[Justin Erenkrantz]

--On Wednesday, June 8, 2005 12:09 AM +0200 Sander Striker 
<[EMAIL PROTECTED]> wrote:



Yes you are right I forgot this. So I think this would be better:


FWIW, I *think* the patch should return result rather than the value of 
ap_pass_brigade.  (Although I have a feeling that might not be correct, 
either.)



I'm pondering if we shouldn't just abort() right here (wrap it in debug
defines
maybe) and fix all the core modules.

OTOH, we can decide the modules aren't broken at all.  Whatever we do will
require some documenting on what a module should and should not do.


Yah, I don't really know what the right thing to do is here.  I know that 
when we did the mod_proxy fix, we had agreed that the handler was broken 
and it should always be sending an EOS.  My gut still says that's right; 
but the failure case is downright nasty.  -- justin

Re: problems w/ geronimo wiki (cache???)

[Justin Erenkrantz]

--On Tuesday, June 7, 2005 7:14 PM -0400 Joshua Slive <[EMAIL PROTECTED]> 
wrote:



Go for it.  What are our production servers for if not testing httpd
patches ;-)


As a data point (and one I think you alluded to earlier), the LA on the box 
was about 30 before I turned on the cache.  It's now settled down to around 
5-6.  So, yah, it'd be a good idea to get this working.  ;-)  -- justin

Re: svn commit: r188846 - in /httpd/httpd/trunk: include/mpm_common.h server/mpm_common.c

[Justin Erenkrantz]

--On Wednesday, June 8, 2005 12:49 AM +0100 Nick Kew <[EMAIL PROTECTED]> 
wrote:



OK, I've updated it thusly.  But what version of gcc is that?
I'm using gcc 3.3.4, and that was happy with it.


% gcc -v
Reading specs from /usr/lib/gcc-lib/ia64-redhat-linux/3.2.3/specs
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man 
--infodir=/usr/share/info --enable-shared --enable-threads=posix 
--disable-checking --with-system-zlib --enable-__cxa_atexit 
--host=ia64-redhat-linux

Thread model: posix
gcc version 3.2.3 20030502 (Red Hat Linux 3.2.3-52)

HTH.  Thanks!  -- justin

Re: [PATCH] mod_disk_cache: Handling of Varied Content

[Justin Erenkrantz]

On Fri, Jun 10, 2005 at 06:14:09PM -0700, Paul Querna wrote:

Comments inline.

> Index: modules/cache/mod_disk_cache.c
> ===
> --- modules/cache/mod_disk_cache.c(revision 190047)
> +++ modules/cache/mod_disk_cache.c(working copy)
> @@ -59,10 +59,13 @@
>  int dirlevels;  /* Number of levels of subdirectories */
>  int dirlength;/* Length of subdirectory names */
>  #endif
> +int varied;
> +char *varyfile;  /* name of file where the vary info will go */
>  char *datafile;  /* name of file where the data will go */
>  char *hdrsfile;  /* name of file where the hdrs will go */
>  char *hashfile;  /* Computed hash key for this URI */
>  char *name;
> +char *key;
>  apr_file_t *fd;  /* data file */
>  apr_file_t *hfd; /* headers file */
>  apr_file_t *tfd; /* temporary file for data */

I don't think you're ever using the varied field, correct?

And I think we may want to clarify name / key a bit.  My suggestion:

 char *name;  /* Requested URI without vary bits - suitable for mortals. */
 char *key;   /* Requested URI with Vary bits (if present); on-disk prefix. */

> @@ -92,18 +95,37 @@
>  
>  module AP_MODULE_DECLARE_DATA disk_cache_module;
>  
> +#ifndef DISK_CACHE_DEBUG
> +#define DISK_CACHE_DEBUG 0
> +#endif
> +

Do we really need an #ifdef?  The debug verbosity log level should be fine.
These's no need for more #ifdefs - it clutters up the code too much.

>  /* Forward declarations */
>  static int remove_entity(cache_handle_t *h);
>  static apr_status_t store_headers(cache_handle_t *h, request_rec *r, 
> cache_info *i);
>  static apr_status_t store_body(cache_handle_t *h, request_rec *r, 
> apr_bucket_brigade *b);
>  static apr_status_t recall_headers(cache_handle_t *h, request_rec *r);
>  static apr_status_t recall_body(cache_handle_t *h, apr_pool_t *p, 
> apr_bucket_brigade *bb);
> +static apr_status_t read_array(request_rec *r, apr_array_header_t* arr, 
> +   apr_file_t *file);
>  
>  /*
>   * Local static functions
>   */
> +#define CACHE_VARY_SUFFIX   ".vary"
>  #define CACHE_HEADER_SUFFIX ".header"
>  #define CACHE_DATA_SUFFIX   ".data"
> +
> +static char *vary_file(apr_pool_t *p, disk_cache_conf *conf,
> + disk_cache_object_t *dobj, const char *name)
> +{
> +if (!dobj->hashfile) {
> +dobj->hashfile = ap_cache_generate_name(p, conf->dirlevels, 
> +conf->dirlength, name);
> +}
> +return apr_pstrcat(p, conf->cache_root, "/", dobj->hashfile,
> +   CACHE_VARY_SUFFIX, NULL);
> +}
> +
>  static char *header_file(apr_pool_t *p, disk_cache_conf *conf,
>   disk_cache_object_t *dobj, const char *name)
>  {
> @@ -242,6 +264,59 @@
>  return APR_SUCCESS;
>  }
>  
> +static char* regen_key(apr_pool_t* p, apr_table_t* headers,
> +   apr_array_header_t* varray, const char *oldkey)
> +{
> +struct iovec *iov;
> +int i,k;
> +int nvec;
> +const char *header;
> +const char **elts;
> +
> +nvec = (varray->nelts * 2) + 2;
> +iov = apr_palloc(p, sizeof(struct iovec) * nvec);
> +elts = (const char **) varray->elts;
> +for(i=0, k=0; i < varray->nelts; i++) {
> +header = apr_table_get(headers, elts[i]);
> +if (!header) {
> +header = "";
> +}
> +iov[k].iov_base = (char*) elts[i];
> +iov[k].iov_len = strlen(elts[i]);
> +k++;
> +iov[k].iov_base = (char*) header;
> +iov[k].iov_len = strlen(header);
> +k++;
> +}
> +iov[k].iov_base = (char*)"\001";
> +iov[k].iov_len = 1;
> +k++;
> +iov[k].iov_base = (char*) oldkey;
> +iov[k].iov_len = strlen(oldkey);
> +k++;
> +
> +return apr_pstrcatv(p, iov, k, NULL);
> +}

Um, what's the '\001' for?  If I'm reading this right, it'd be creating a new
hash value that uses the following as the seed:

header name (from stored response) + header value (from req. or "") + 
... repeat ...  + \001 + original key.

I think the 001 is unnecessary here or it hasn't been suitably explained...

BTW, a comment at the top of mod_disk_cache.c that explains this vary file and
the indirection would be really nice, too.  See where the description of the
header format currently is.  There's no need to force people to guess what the
format is.  In fact, as seen below, I think I'm confused as to how far the
indirection goes (your emails aren't clear, either).

BTW, I bet we could (should?) make this return const char *.

> +
> +static int array_alphasort(const void *fn1, const void *fn2)
> +{
> +return strcmp(*(char**)fn1, *(char**)fn2);
> +}
> +
> +static void tokens_to_array(apr_pool_t *p, const char *data,
> +apr_array_header_t *arr)
> +{
> +char *t

Re: [PATCH] mod_disk_cache: Handling of Varied Content

[Justin Erenkrantz]

On Sun, Jun 12, 2005 at 10:06:30AM -0700, Paul Querna wrote:
> I believe so.  Yes, it clutters the code, but my objective is to have a 
> switch where I can easily add logging to the common case code, and leave 
> it in there, instead of deleting it before I commit.  I think in the 
> long run, we need a different log level for 'developer' debugging.  Just 
> look at some of the stuff that the a reverse SSL Proxy logs at LogLevel 
> Debug.

I would really prefer that we stick with placing these log entries at debug
and address a 'developer' log level independently of this.  But, adding
#ifdefs is not a solution.

> ap_get_list_item() returns them lower-cased, avoiding this whole issue.

Relying on lowercased header names is not enough.  In certain cases, the
header *values* are supposed to be rendered case-insensitively as well
(Accept-Encoding, etc.).  Yet, in other cases, the header values are supposed
to be case-sensitive.  =)

This will mean that the cache will not hit when it should.  We *could* just
punt on the values and cache multiple responses that are actually the same...

> >This probably should call apr_file_remove first.
> 
> I disagree.  It should not.  Calling apr_file_remove first creates a 
> wider window when the .vary file does not exist.  If anywhere, it should 
> be called right before _rename(), but why even call it there, when 
> rename will replace it atomically?

Hmm.  I guess APR makes a guarantee that it'll always overwrite the file (even
though many platforms don't do that in their rename call - i.e. Win32).  So,
we should probably yank the other _remove calls then?

> >There is also a degenerate case here: a response wasn't previously using 
> >Vary
> >but now is or vice versa.  We should see about clearing out any datafile 
> >and
> >hdrsfile (or varyfile if no longer present) before continuing on.
> > 
> >
> 
> Case #1:  URL added a Vary Header.
> This should be work fine.  The .header and .data files should be cleaned 
> up by htcacheclean like normal URLs.  If we switched to .header files 
> for the Vary headers, we would need something to cleanup the .data file. 
> (htcacheclean modification?)

I'd prefer that we be clean here and _remove() the .data file then.
htcacheclean won't prune orphan .data files - it needs the .header file now.

> Case #2: URL stops sending the Vary header.
> Problem.  In this case we should remove the .vary file.  However, if we 
> changed the code to store the vary info in the .header file, this case 
> would work fine, since it would overwrite the .header file with Vary 
> info, with one without vary info, and fix this issue.

Correct.  That's a nice side-effect of using the .header file.  =)  -- justin

Re: Removing AddOutputFilterByType

[Justin Erenkrantz]

--On June 19, 2005 5:33:14 PM +0100 Nick Kew <[EMAIL PROTECTED]> wrote:


AddOutputFilterByType has always been problematic.  I see there's another bug
report this month arising from it:

http://issues.apache.org/bugzilla/show_bug.cgi?id=33499

Since the purpose of this directive is now available from mod_filter, does
anyone object if I simply yank AddOutputFilterByType from 2.1?


Why can't AddOutputFilterByType call mod_filter under the hood?

But, from the user's perspective, I think AddOutputFilterByType needs to stay 
and I'm not convinced that mod_filter has a good enough directive interface to 
make it intuitive.  -- justin

Re: REQUEST_CHUNKED_PASS Q for 2.x TraceEnable [on|off|extended]

[Justin Erenkrantz]

On Thu, Jun 23, 2005 at 08:59:12AM -0500, William A. Rowe, Jr. wrote:
> Does anyone see a reason not to resurrect REQUEST_CHUNKED_PASS?

I have no idea what you are talking about (your email is vague on context);
but we should certainly not reintroduce the kludges associated with
REQUEST_CHUNKED_PASS.  The ap_*_client code needs to go away.  -- justin

Re: Proposal: Drop .Z dist file for 2.1/2.2

[Justin Erenkrantz]

--On Friday, June 24, 2005 11:09 AM -0700 Paul Querna 
<[EMAIL PROTECTED]> wrote:



So, .Z is more than twice as big as bz2.  I don't see any reason to keep
including it, since every modern system has gzip, or can get it.


+1.  -- justin

Re: mod_smtpd project planning

[Justin Erenkrantz]

On Wed, Jun 29, 2005 at 02:39:43PM -0700, Roy Fielding wrote:
> The dev list needs some new blood (and new ideas) and these folks
> aren't going to learn anything about real Apache development if
> they are stuck out in the suburbs.

++1.  -- justin

Re: mod_smtpd project planning

[Justin Erenkrantz]

--On June 30, 2005 12:33:24 AM -0500 Jem Berkes <[EMAIL PROTECTED]> wrote:


Hi all, I'm another student working on mod_smtpd


If they haven't already, your mentors should be getting in touch with you 
soon.  (It's their responsibility to inform you of the details.)


In short, you need to fill out our CLAs and fax them to us.  The form is at:



Once you do that, an account can be created for you.  Note that we are 
reserving the right to delete your accounts at the end of the SoC program.


Welcome!  -- justin

Re: svn commit: r208787 - in /httpd/httpd/trunk/modules: http/http_filters.c http/http_protocol.c proxy/mod_proxy.c

[Justin Erenkrantz]

--On July 4, 2005 11:08:18 PM -0500 "William A. Rowe, Jr." 
<[EMAIL PROTECTED]> wrote:

 It still is not 'correct' until REQUEST_CHUNKED_PASS is reimplemented
 and passes some chunk headers, since we aren't echoing the entire
 request.  But it gets me further on testing 1.3 -> 2.0 -> 2.1 -> 2.0 ->
 1.3 proxy behaviors.


As I've said before, REQUEST_CHUNKED_PASS needs to go away in 2.x.  The 
correct 'fix' is to remove the CHUNK input filter and then call 
ap_get_brigade().  But, calling ap_should_client_block, 
ap_get_client_block(), etc. is just flat-out wrong for 2.x.  -- justin

Re: svn commit: r208787 - in /httpd/httpd/trunk/modules: http/http_filters.c http/http_protocol.c proxy/mod_proxy.c

[Justin Erenkrantz]

--On July 6, 2005 4:30:49 PM -0700 Paul Querna <[EMAIL PROTECTED]> wrote:


Then we should remove them from trunk today.  Why leave a 'flat-out
wrong' API available?


If no one has removed them from trunk by the hackathon next weekend, I will 
remove them then.  -- justin

Re: [vote] MODULE_MAGIC_COOKIE

[Justin Erenkrantz]

On Thu, Jul 07, 2005 at 10:51:09PM -0500, William A. Rowe, Jr. wrote:
>   [ ] Revert the cookie to AP20 for httpd-2.1 and httpd-2.2
>   [ ] Leave the cookie at AP21 and bump again to AP22 w/httpd-2.2
>   [X] Get it over with already and bump now to AP22 for httpd-2.1

Re: [VOTE] mod_ftp for HTTP Server Project

[Justin Erenkrantz]

On Thu, Jul 07, 2005 at 02:26:13PM -0400, Jim Jagielski wrote:
> I therefore Call A Vote on whether we should support mod_ftp for
> inclusion into the Incubator and if we should accept mod_ftp upon
> graduation from the Incubator.

+1.  -- justin