Re: svn commit: r692325 - /httpd/httpd/trunk/docs/manual/mod/mpm_common.xml

[Dan Poirier]

William A. Rowe, Jr. wrote:

[EMAIL PROTECTED] wrote:

@@ -443,8 +443,8 @@
 
 

 MaxClients
-Maximum number of child processes that will be created
-to serve requests
+Maximum number of simultaneous requests that will
+be served


perhaps we mean to say

"Maximum number of connections that will be processed simultaneously"

which is, as you note, different than the number served, lingering,
sitting in the accept queue, etc.

also note that clients in this day and age typically consume 2 
connections

so that description was sub-par.
Maybe all we can say these days about MaxClients is "Maximum number of 
worker threads that will be created," since the event MPM could have 
more connections or requests in progress than threads.

MaxKeepAliveRequests meaning

[Dan Poirier]

I've noticed that when MaxKeepAliveRequests is set to N, each
connection handles up to N+1 requests.

To me this seemed surprising, but a co-worker interprets
MaxKeepAliveRequests as the number of additional requests served
due to KeepAlive, which would make this exactly right.

I'm wondering what the consensus is about the intention
here.  Perhaps the doc could be clarified.

-- 
Dan Poirier <[EMAIL PROTECTED]>

More accurate logging of requests per connection

[Dan Poirier]

I recently contributed a new %k log format that is intended to log how 
many requests have been handled on the current connection.  It does this 
by logging the value of conn->keepalives.


However, conn->keepalives isn't, strictly speaking, the number of 
requests the connection has handled.  If keepalives are disabled, it's 
zero.  It's also not incremented when the request count reaches 
MaxKeepAliveRequests, so with e.g. MaxKeepAliveRequests 3, it logs 1,2,3,3.


The attached patch adjusts the value logged, so it's always 1 if 
keepalives are disabled, and if it's the last request (conn->keepalive 
== AP_CONN_CLOSE), we add one so the values logged are 1,2,3,4 instead 
of 1,2,3,3.


There's one remaining problem.  There are some error paths which set 
conn->keepalive to AP_CONN_CLOSE after conn->keepalives has already been 
incremented, and in that case, this logging code will log a value that's 
one too many.  I don't think that's a terrible problem, but I'd welcome 
suggestions on how to catch that case.


Thanks.
Dan Poirier
[EMAIL PROTECTED]



Index: modules/loggers/mod_log_config.c
===
--- modules/loggers/mod_log_config.c(revision 693854)
+++ modules/loggers/mod_log_config.c(working copy)
@@ -697,7 +697,28 @@
 
 static const char *log_requests_on_connection(request_rec *r, char *a)
 {
-return apr_itoa(r->pool, r->connection->keepalives);
+int num_requests;
+if (!r->server->keep_alive) {
+num_requests = 1;
+} else if (r->connection->keepalive == AP_CONN_CLOSE) {
+num_requests = r->connection->keepalives + 1;
+} else {
+num_requests = r->connection->keepalives;
+}
+return apr_itoa(r->pool, num_requests);
 }
 
 /*

HTTP Protocol: How frequently should browser repeat requests after 304 responses?

[Dan Poirier]

I've looked at mod_expires doc and RFC 2616, but can't really
tell what the right behavior is supposed to be in this case.

Using mod_expire to set the expiration time to something like
"access plus 1 hour", we see a browser request a file, get back
a 200 with the expiration, and not request it again for an hour,
as expected.  The next time, it uses an if-modified-since and
gets back a 304, also good.  But from then on, it keeps coming
back to the server every time and getting back 304's.

The 304 responses won't have any cache control information, of
course, per RFC 2616 10.3.5.

Two questions:
1) Is the browser behaving properly, or should it wait another hour
after each 304 before making another request?

2) If the browser behavior is right, is there a way to configure
Apache to tell the browser to only check once an hour instead of
every time?

Thanks.

-- 
Dan Poirier <[EMAIL PROTECTED]>

Re: HTTP Protocol: How frequently should browser repeat requests after 304 responses?

[Dan Poirier]

Ruediger Pluem wrote:


On 09/17/2008 10:00 PM, Dan Poirier wrote:

I've looked at mod_expires doc and RFC 2616, but can't really
tell what the right behavior is supposed to be in this case.

Using mod_expire to set the expiration time to something like
"access plus 1 hour", we see a browser request a file, get back
a 200 with the expiration, and not request it again for an hour,
as expected.  The next time, it uses an if-modified-since and
gets back a 304, also good.  But from then on, it keeps coming
back to the server every time and getting back 304's.

The 304 responses won't have any cache control information, of
course, per RFC 2616 10.3.5.


I don't read from RFC 2616 10.3.5 that there shouldn't be any cache 
control

related headers contained in the response. On the contrary: If the values
for Expires or Cache-Control changed these values MUST be included.
So I guess we have a bug in mod_expires here. I guess this is caused by
the position of the MOD_EXPIRES filter in the filter chain. It is
AP_FTYPE_CONTENT_SET-2 as it needs to run before the cache save filter.
I guess (haven't checked so far) that we do not run through these filters
if a handler simply returns a 304.

Looking at a wire trace, we are actually providing Expires and Cache-control
on the 304 response.  So whatever the browser is doing, that's not the 
cause.


If I keep hitting reload in Firefox, I do see requests more frequently 
than the

max-age - but maybe Firefox reasons that my hitting reload on the same page
means I really do want to go back to the server despite the caching.  Which
might mean the whole test is invalid.  I'm going to need to investigate some
more.

Dan

Re: More accurate logging of requests per connection

[Dan Poirier]

On Fri, September 19, 2008 11:18 am, Jim Jagielski wrote:
> A better interpretation would be, I think, the number of keepalive
> requests on that connection. So a '1' would be the 1st keepalive
> request, 2 would be the 2nd, etc... So a '0' would be the
> initial request, and would be applicable whether keepalives
> are enabled or not. This would make the logic simpler.

That works.  The remaining issue, which is minor, is that since
ap_set_keepalive() doesn't increment connection->keepalives on the
last request, the value logged will be N-1 for both the N-1 and N'th
keepalive request.  But I don't imagine most connections will ever
see the N'th keepalive request.

-- 
Dan Poirier <[EMAIL PROTECTED]>

AuthzMergeRules blocks everything in default configuration

[Dan Poirier]

I hate to re-open this can of worms, but...

Unless I'm missing something, in trunk right now, uncommenting includes 
for the examples like "extra/httpd-manual.conf" does not result in being 
able to serve the documentation pages.


In the main config file:


 Require all denied


blocks all access, and that's inherited by every other  or
 in the configuration, since AuthzMergeRules defaults to On.

To make this work, one would have to add AuthzMergeRules Off to every 
other  or  in the configuration that isn't a subset 
of another one

that already has it.

Doing that makes me wonder what's the point of having it, if we have to 
turn it

off in almost every case to actually serve pages.

Or would it make sense to add AuthzMergeRules Off to ?  
Would that
make the rest of the permissions kind of stand alone?  I guess then 
you'd have

to add AuthzMergeRules On to any of them whose permissions you wanted
inherited by even lower level sections.

I read through some previous discussion of the authz inheritance 
behavior, but
it doesn't seem to have considered the effect of having "Require all 
denied" at
the top level, which is overriding everything else by default even when 
other

sections specify other permissions.

Dan

Re: AuthzMergeRules blocks everything in default configuration

[Dan Poirier]

On Tue, 23 Sep 2008 11:05:45 -0700, "Chris Darroch"
<[EMAIL PROTECTED]> said:
> Dan Poirier wrote:
>... 
> > I read through some previous discussion of the authz inheritance
> > behavior, but it doesn't seem to have considered the effect of having
> > "Require all denied" at the top level, which is overriding everything
> > else by default even when other sections specify other permissions.

[omitted some history]
 
>Let's say you've got two authz configs, A and B, and A is considered
> before B (maybe A is in  and B is in  for
> a given request (e.g., for /foo/bar).
...
>Previously [in trunk] the default merge rule was "OR", i.e., A ||
> B.  Thus in the case where A allowed access and B didn't, you'd be
> let in despite B's local authz denying you access.  That would, I
> thought, have created a lot of unexpected security holes for people
> who upgraded from 2.2, where B's local authz was the only authority.

Agreed.

>Brad Nicholes changed that a few months ago so that the default
> rule should be "AND", i.e., A && B.  Thus in the case above, you'd
> be denied access because of B's config.
> 
>This closes off the worst security problems, I think.  As you note,
> though, it can produce situations where A's config denies you access
> and so B's is just ignored, even if B wants to let you have access.
> You can set AuthzMergeRules Off for B, but also as you note you'd have to
> set it everywhere.  And since it's value isn't "inherited", if
> A, B, and C all apply to the request, then turning it Off for B
> means you'll still get B && C applied, not just C.
> 
>We can obviously change the example configs so as to make
> the  setting have "Require all granted".  However, I
> think we'll find that fair numbers of people get themselves into
> this situation -- by naively upgrading using 2.2 configs, assuming
> things work the 2.2 way (B isn't affected by A), assuming AuthzMergeRules
> values are "inherited", etc.  

I don't think we really want to change  to "Require
all granted", do we?  Wouldn't that open up the entire system by
default?

>Hence my feeling that the default should be Off (to be closer
> to 2.2 behaviour), and when explicitly On, the rule that's applied
> should be AND -- as is now true, thanks to Brad!
> 
>One further thought of mine from ages ago was that On (which
> currently means AND) could be replaced by two non-default options,
> And and Or (or SastifyAll and SastifyOne, respectively).  Then
> the administrator would have full control over the inter-block
> merge rule, similar to the control they have in trunk over the
> intra-block rules via  and .

I like the idea of replacing ON with AND and OR.  It would not
only provide more control, but make it explicit what kind of merging
was going to happen.

I have mixed thoughts about changing the default to OFF.  

Cons: That would mean every container directive would have to specify
some sort of access control (or at least AuthzMergeRules AND) or it'd
be wide open, right?

Pros: On the other hand, that's the behavior in 2.2, and changing it
would surely cause confusion for people trying to migrate to the new
behavior.  As you point out, it's also confusing to put configuration
in B that ought to grant access, and find that access is still denied
due to a setting in A.

One other idea occurs to me.  Would it seem more intuitive if a
context that had no authz directives inherited the settings from its
predecessor, but as soon you added authz directives, it behaved as if
you were starting from scratch?  (As if you'd included AuthzMergeRules
Off.  You could always add AuthzMergeRules AND if you wanted that
behavior.)  Or would it just be more confusing?  I guess you'd still
have to go through the default configuration files adding "Require
all granted" or "AuthzMergeRules Off" everywhere in order to give
access to anything, since otherwise they'd still inherit from the
top-level's not granting any access.

>Thoughts?  I apologize profusely for having so little time to
> devote to actual useful coding at the moment.

I'd like to hear what others think.  Can the current behavior be
improved, and if so, how?

Dan

Re: how to uninstall httpd-2.2.10

[Dan Poirier]

[from the users list]

On Wed, Oct 22, 2008 at 9:02 PM, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote:

[EMAIL PROTECTED] wrote:

Hello All,

I've compile and installed apache-2.2.10 from source and would like to
remove it from my system.  I notice that I cannot use 'make uninstall'
anymore.
Could someone tell me how to go about uninstalling it?


"make uninstall" sounds like a convenient feature.  Does anyone know a 
reason we don't have it, besides that nobody's coded it yet?


Dan

Re: AuthzMergeRules blocks everything in default configuration

[Dan Poirier]

Chris Darroch said the following on 10/29/2008 10:35 PM:


 Require valid-user
 
   Require group alien
   
 Require group hostile
 Require group neutral
 Require group noninterventionist
   
 



I wonder if we have to stick with building up complicated authorization 
expressions by nesting many different directives.


I'd find it much easier to understand if we had fewer directives, and 
just built up the more complicated ideas by writing boolean expressions, 
which most of us already know how to cope with.


I'm picturing something like this:


AuthorizeIf valid-user AND member-of-group human



Maybe with some syntactic sugar to help break things down into 
understandable chunks:



DefineAuthorization friendly NOT (member-of-group hostile OR 
member-of-group neutral OR member-of-group noninterventionist)


DefineAuthorization human NOT (member-of-group alien)


AuthorizeIf valid-user AND (human OR friendly)



We could add a few more pre-defined primitives that could be used in 
expressions, such as


# Inherit parent authorization, and add another condition
AuthorizeIf authorized-in-parent AND member-of-group vips

# Grant access to all
AuthorizeIf true


Granted, this is very different from the syntax in 2.2 configurations, 
so migration would have to be considered.  But it might be worthwhile if 
something like this reduced confusion and questions on the lists.


Dan

Re: httpd win64 "project" sources/makefiles [was:...binaries]

[Dan Poirier]

William A. Rowe, Jr. said the following on 11/03/2008 07:32 AM:

But studio (or eclipse or codewarrior or [name your IDE]) users would
appreciate a perspective into the sources.  The IDE-accessible nature
of the original Win32 port is what made it so easy for me to jump in,
understand and substantially refactor the win32 support.  Without the
IDE view, would I have done that?  Maybe - but at that time in my
development patterns - more likely not.

Lets look to supporting [name your favorite IDE] as a bigger picture
item not specific to windows, and to transition away from .dsp for
the build/ide view support.


I'm still getting my head around how all the parts of Apache work 
together, and would love to be able to dive into it using Eclipse and 
explore.  The advantage of Eclipse would be that it's more 
cross-platform than most of the alternatives that I'm aware of.


Re: windows, aren't there one or more ports of gcc?  Would something 
like that be worth considering as a more stable build solution than MS's 
tools?


Dan

Re: svn commit: r722213 - /httpd/httpd/trunk/modules/http/http_request.c

[Dan Poirier]

I'm having a little trouble understanding how this works.  I'm sure I
must be missing some subtlety.

Before this change, if ap_die was called with AP_FILTER_ERROR, it would
return without writing any response.  If the response hadn't already
been written by some other code, then no response would be written at
all.  Obviously bad.

ap_http_header_filter is where the response status line and response
headers are written, and after that, the filter removes itself from the
filter chain.

So, we can tell if a response has already been written by seeing if the
header filter has already removed itself from the chain. If no response
has been written, the status is changed to 500, and we fall down into
the
rest of ap_die and write a 500 response.

What confuses me is that most of the rest of the code in ap_die() is
concerned with constructing the response, which seems to assume that no
response has been written before calling ap_die; yet the new code seems
to allow for the possibility that a response has already been written
before ap_die was called.

Is it the case that ap_die is always assumed to be called before a
response has been written?  Or could it be called after a response was
written, and if so, is there any reason to carry on with processing
another response that won't be used (or if sent after another response,
is simply wrong)?

Thanks, Dan

[Patch] mod_suexec compile failure

[Dan Poirier]

A couple of missed "ap_"'s.

Index: modules/generators/mod_suexec.c
===
--- modules/generators/mod_suexec.c (revision 722900)
+++ modules/generators/mod_suexec.c (working copy)
@@ -64,7 +64,7 @@
 if (err != NULL) {
 return err;
 }
-if (unixd_config.suexec_enabled) {
+if (ap_unixd_config.suexec_enabled) {
 cfg->ugid.uid = ap_uname2id(uid);
 cfg->ugid.gid = ap_gname2id(gid);
 cfg->ugid.userdir = 0;
@@ -94,7 +94,7 @@
 apr_pool_userdata_get(&reported, SUEXEC_POST_CONFIG_USERDATA,
   s->process->pool);
 
-if ((reported == NULL) && unixd_config.suexec_enabled) {
+if ((reported == NULL) && ap_unixd_config.suexec_enabled) {
 ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, s,
  "suEXEC mechanism enabled (wrapper: %s)", SUEXEC_BIN);
 

Re: SNI test case

[Dan Poirier]

Guenter Knauf  writes:

>> Very useful. Might also make sense in a "perlified" version as part of
>> the test code.
> here's a first "perlified" version; not finished yet; 
> needs some cleanup, and not checked yet the results;
> but before someone else starts on it and doubles work I thought I post what I 
> have so far...
> http://svwe10.itex.at/downloads/httpd-sni/make_sni.pl

Has this ever made it into subversion?  I looked in test but
didn't see it.

-- 
Dan Poirier 

Re: [patch] Add IPV6 'specials' flag to mod_rewrite - try 2

[Dan Poirier]

Paul Querna  writes:

> Ryan Phillips wrote:
>> Thanks for the tips.  IN6_IS_ADDR_V4MAPPED is defined in netinet/in.h on
>> unices and APR defines it on windows.  I've modified the patch to check for
>> APR_HAVE_IPV6 support and check for APR_HAVE_NETINET_IN_H. Also, this patch 
>> will
>> apply to trunk/.
>
> needs docs, but committed to trunk in r737973.

How about (for trunk):


Index: mod_rewrite.xml
===
--- mod_rewrite.xml (revision 738498)
+++ mod_rewrite.xml (working copy)
@@ -836,6 +836,13 @@
   can be safely used regardless of whether or not
   mod_ssl is loaded).
 
+  IPV6
+
+  Will contain the text "on" if the connection is
+  using IPv6, or "off" otherwise.  (This variable can
+  be safely used regardless of whether the server has
+  IPv6 support.)
+
 
 
 

Better error message for SSL connection on non-SSL port

[Dan Poirier]

I've proposed a patch for trunk at

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

that spots an attempted SSL connection on a non-SSL port and gives a
more useful message than "Invalid method in request".  It looks to see
if the first couple of bytes look like an SSL handshake, and logs
instead "Invalid method in request %s - possible attempt to establish
SSL connection on non-SSL port".

I'm interested in comments - does this seem useful?  Is this the best
way to go about it?

Thanks.

-- 
Dan Poirier 

Re: Sending data to client

[Dan Poirier]

BTW, I've found this book to be excellent on how filters work, and
Apache 2 modules in general:

Nick Kew, _The Apache Modules Book_
Prentice Hall, 2007

Re: Documentation request for review

[Dan Poirier]

"Lars Eilebrecht"  writes:

> Vincent Deffontaines wrote on 2009-02-08 13:20:23:
>
>> While reviewing Lucien's french translation for the trunk performance 
>> tuning guide (misc/perf-tuning.xml), it has come to my understanding 
>> that this document contains extremely old, and probably outdated, 
>> information.
>
> Yes, this page is really *really* outdated.
> IMHO we should consider removing it or at least adding a note that
> the stuff is outdated.

If it's badly outdated, we should either fix it or remove it.  The net
has too much outdated information on it already :-)  The outdated page
would still be in subversion if anyone wants to work on it later.

That said, it looks to me as if much of the information there is still
relevant, such as the Run-time Configuration section.  It seems worth
preserving.

-- 
Dan Poirier 

Logging bytes sent

[Dan Poirier]

It appears that %b logging of bytes sent can be wrong if something
happens to the connection during the request processing.

The number logged by mod_log_config is r->bytes_sent, which is computed
in ap_content_length_filter().  If something goes wrong (maybe I pull
Apache's network cable) while sending the response, the connection gets
aborted, but r->bytes_sent isn't changed, and so the access log shows
that the full length of the response was sent.  You can add %X to the
logging to see whether the connection was aborted, but the number of
bytes sent logged is still wrong.

I'm wondering if there's some good reason for this that I'm missing?  Or
maybe it's just an oversight I've noticed since I happen to be looking
at that part of the code right now.

Thanks,
Dan

Serving filenames with wildcards using mod_proxy_ftp

[Dan Poirier]

equent digits ("sss") are optional. <..>
  * Time values are always represented in UTC (GMT)
  */
-rc = proxy_ftp_command(apr_pstrcat(p, "MDTM ", 
ftp_escape_globbingchars(p, path), CRLF, NULL),
+rc = proxy_ftp_command(apr_pstrcat(p, "MDTM ", 
ftp_escape_globbingchars(p, path, dconf), CRLF, NULL),
r, origin, bb, &ftpmessage);
 /* then extract the Last-Modified time from it (MMDDhhmmss or 
MMDDhhmmss.xxx GMT). */
 if (rc == 213) {
@@ -1622,7 +1633,7 @@
 }
 #endif /* USE_MDTM */
 /* FIXME: Handle range requests - send REST */
-buf = apr_pstrcat(p, "RETR ", ftp_escape_globbingchars(p, path), CRLF, 
NULL);
+buf = apr_pstrcat(p, "RETR ", ftp_escape_globbingchars(p, path, 
dconf), CRLF, NULL);
 }
 rc = proxy_ftp_command(buf, r, origin, bb, &ftpmessage);
 /* rc is an intermediate response for the LIST or RETR commands */
@@ -1659,7 +1670,7 @@
 ftp_set_TYPE('A', r, origin, bb, NULL);
 
 rc = proxy_ftp_command(apr_pstrcat(p, "CWD ",
-   ftp_escape_globbingchars(p, path), CRLF, NULL),
+   ftp_escape_globbingchars(p, path, dconf), CRLF, 
NULL),
r, origin, bb, &ftpmessage);
 /* possible results: 250, 421, 500, 501, 502, 530, 550 */
 /* 250 Requested file action okay, completed. */
@@ -1709,9 +1720,6 @@
 
 /* set content-type */
 if (dirlisting) {
-proxy_dir_conf *dconf = ap_get_module_config(r->per_dir_config,
- &proxy_module);
-
 ap_set_content_type(r, apr_pstrcat(p, "text/html;charset=",
dconf->ftp_directory_charset ?
dconf->ftp_directory_charset :
We need to be able to serve files whose names contain wildcards using
mod_proxy_ftp.  Right now, mod_proxy_ftp provides a file listing when
it sees wildcards in the filename requested, which is reasonable for
interactive browsing of an FTP site.  In this case, though, we need to
actually get files with those names, and we don't need the browsing
function (we already know the filenames).  Unfortunately, we don't
control the filenames, or we'd just use filenames without wildcards
and avoid all this.

The solution I'm thinking about is to add a directive to allow
selectively disabling the behavior of doing file listings when
wildcards are seen in the requested filename.  E.g.:

  The ProxyFtpListOnWildcard directive controls whether wildcard
  characters ("*?[{~") in requested filenames cause mod_proxy_ftp to
  return a listing of files instead of downloading a file.  By default
  (value on), they do.  Set to "off" to allow downloading files even
  if they have wildcard characters in their names.  (directory
  context)

I'd appreciate comments on this approach, and my proposed
implementation (patch against trunk is attached).  

Also, in testing that change, I found that mod_proxy_ftp escapes
wildcards in filenames using backslashes when sending them to the FTP
server, which none of the FTP servers I was testing with understood.
To continue with my testing, I added another directive to turn that
behavior off, but I'd like to better understand why that behavior is
there, since it appears to assume a behavior that my FTP servers don't
have, and doesn't seem to be mentioned in RFC 959.


-- 
Dan Poirier 

Re: Serving filenames with wildcards using mod_proxy_ftp

[Dan Poirier]

Sorry, I didn't expect the patch to end up before the explanation.
Here's the message that should have come first.

We need to be able to serve files whose names contain wildcards using
mod_proxy_ftp.  Right now, mod_proxy_ftp provides a file listing when
it sees wildcards in the filename requested, which is reasonable for
interactive browsing of an FTP site.  In this case, though, we need to
actually get files with those names, and we don't need the browsing
function (we already know the filenames).  Unfortunately, we don't
control the filenames, or we'd just use filenames without wildcards
and avoid all this.

The solution I'm thinking about is to add a directive to allow
selectively disabling the behavior of doing file listings when
wildcards are seen in the requested filename.  E.g.:

  The ProxyFtpListOnWildcard directive controls whether wildcard
  characters ("*?[{~") in requested filenames cause mod_proxy_ftp to
  return a listing of files instead of downloading a file.  By default
  (value on), they do.  Set to "off" to allow downloading files even
  if they have wildcard characters in their names.  (directory
  context)

I'd appreciate comments on this approach, and my proposed
implementation (patch against trunk is attached).  

Also, in testing that change, I found that mod_proxy_ftp escapes
wildcards in filenames using backslashes when sending them to the FTP
server, which none of the FTP servers I was testing with understood.
To continue with my testing, I added another directive to turn that
behavior off, but I'd like to better understand why that behavior is
there, since it appears to assume a behavior that my FTP servers don't
have, and doesn't seem to be mentioned in RFC 959.


-- 
Dan Poirier 

Re: Serving filenames with wildcards using mod_proxy_ftp

[Dan Poirier]

Eric Covener  writes:

> On Tue, Feb 24, 2009 at 10:29 AM, Dan Poirier  wrote:
>> Also, in testing that change, I found that mod_proxy_ftp escapes
>> wildcards in filenames using backslashes when sending them to the FTP
>> server, which none of the FTP servers I was testing with understood.
>> To continue with my testing, I added another directive to turn that
>> behavior off, but I'd like to better understand why that behavior is
>> there, since it appears to assume a behavior that my FTP servers don't
>> have, and doesn't seem to be mentioned in RFC 959.
>>
>
> What backends/results did you try?  Does anyone have any anecdotal
> experience about what backends might require escaping of the glob
> characters on a retrieve?

As I recall, I tried vsftpd, proftpd, and Apache FTP server.

> I'm wondering if we add the feature, should we flip the default in trunk?

I would say yes.  I don't think this escaping could ever have been used
before, since any paths with globbing characters in them would have
triggered a file listing, which doesn't do the escaping.  So until now,
apache would never have tried to retrieve any file with globbing
characters in its name.   

There's not really any reason to make this an option, now that I think
about it, unless someone comes up with an example of an ftp server that
needs it.  We might just want to remove the escaping code.

-- 
Dan Poirier 

MaxClients reached message can be premature

[Dan Poirier]

httpd issues the message

"server reached MaxClients setting, consider raising the
MaxClients setting"

as soon as the number of spare threads drops below MinSpareThreads.  In
a pathological case where MinSpareThreads is high, the number of threads 
actually in use might be nowhere near MaxClients.

Here's a possible solution.  It checks whether we're really at
MaxClients,
or just below MinSpareThreads, and issues a more accurate message.  The
new
message will only be issued once.  If we do hit MaxClients for real, the 
original message will still be issued.

The patch is against trunk.

Dan

Index: server/mpm/worker/worker.c
===
--- server/mpm/worker/worker.c  (revision 756126)
+++ server/mpm/worker/worker.c  (working copy)
@@ -1509,15 +1509,27 @@
 /* terminate the free list */
 if (free_length == 0) { /* scoreboard is full, can't fork */
 
-if (active_thread_count >= ap_daemons_limit *
ap_threads_per_child) { 
-static int reported = 0;
-if (!reported) {
-/* only report this condition once */
-ap_log_error(APLOG_MARK, APLOG_ERR, 0,
- ap_server_conf,
- "server reached MaxClients setting,
consider"
- " raising the MaxClients setting");
-reported = 1;
+if (active_thread_count >= ap_daemons_limit *
ap_threads_per_child) {
+/* no threads are "inactive" - starting, stopping, etc.
*/
+/* Are all threads in use? */
+if (0 == idle_thread_count) {
+static int reported = 0;
+if (!reported) {
+/* only report this condition once */
+ap_log_error(APLOG_MARK, APLOG_ERR, 0,
+ ap_server_conf,
+ "server reached MaxClients
setting, consider"
+ " raising the MaxClients
setting");
+reported = 1;
+}
+} else {
+static int reported = 0;
+if (!reported) {
+ap_log_error(APLOG_MARK, APLOG_ERR, 0,
+ ap_server_conf,
+ "server is within MinSpareThreads
of MaxClients, "
+ "consider raising the MaxClients
setting");
+}
 }
 }
 else {

Re: MaxClients reached message can be premature

[Dan Poirier]

Sorry, my email client mangled that patch.  Trying another
one:

Index: server/mpm/worker/worker.c
===
--- server/mpm/worker/worker.c  (revision 756126)
+++ server/mpm/worker/worker.c  (working copy)
@@ -1509,15 +1509,27 @@
 /* terminate the free list */
 if (free_length == 0) { /* scoreboard is full, can't fork */
 
-if (active_thread_count >= ap_daemons_limit * 
ap_threads_per_child) { 
-static int reported = 0;
-if (!reported) {
-/* only report this condition once */
-ap_log_error(APLOG_MARK, APLOG_ERR, 0,
- ap_server_conf,
- "server reached MaxClients setting, consider"
- " raising the MaxClients setting");
-reported = 1;
+if (active_thread_count >= ap_daemons_limit * 
ap_threads_per_child) {
+/* no threads are "inactive" - starting, stopping, etc. */
+/* Are all threads in use? */
+if (0 == idle_thread_count) {
+static int reported = 0;
+if (!reported) {
+/* only report this condition once */
+ap_log_error(APLOG_MARK, APLOG_ERR, 0,
+ ap_server_conf,
+ "server reached MaxClients setting, 
consider"
+ " raising the MaxClients setting");
+reported = 1;
+}
+} else {
+static int reported = 0;
+if (!reported) {
+ap_log_error(APLOG_MARK, APLOG_ERR, 0,
+ ap_server_conf,
+ "server is within MinSpareThreads of 
MaxClients, "
+ "consider raising the MaxClients 
setting");
+}
 }
 }
 else {

Re: MaxClients reached message can be premature

[Dan Poirier]

"Dan Poirier"  writes:

> httpd issues the message
>
> "server reached MaxClients setting, consider raising the
> MaxClients setting"
>
> as soon as the number of spare threads drops below MinSpareThreads.  In
> a pathological case where MinSpareThreads is high, the number of threads 
> actually in use might be nowhere near MaxClients.
>
> Here's a possible solution.  It checks whether we're really at
> MaxClients, or just below MinSpareThreads, and issues a more accurate
> message.  The new message will only be issued once.  If we do hit
> MaxClients for real, the original message will still be issued.

Thanks to Greg Ames who pointed out that I forgot to set the reported
flag after issuing the new message.

Any other comments?

-- 
Dan Poirier 

Re: Ideas for content-aware filter modules for 2.4

[Dan Poirier]

Nick Kew  writes:

> We have handling of certain important encodings:
> SSL and compression (albeit not quite bug-free) as
> standard in current versions.  I'd be interested to
> expand that with some new filter modules.
>
> 1.  Character Encoding.  We have very limited capability
> in mod_charset_lite.  We can expand that to support
> automatic detection of charset, and either setting a request
> field or transforming to a selected charset.
>
> We can also provide an API for modules to configure this,
> in cases where more than one transformation is wanted.
> A real-life use case for this is where users of libxml2-based
> modules such as mod_proxy_html need to use charsets
> other than utf-8, and particularly charsets that are not
> supported by libxml2.

I'd definitely be in favor of some improvements in this area.

mod_charset_lite can translate between character encodings, but there's
no general way for it to know what encoding the content coming down the
chain is using.

Can automatic detection of charsets be done reliably and with low cost?
I'd have guessed not, but would love to be educated to the contrary.

Alternatively, maybe generators and filters could indicate the encoding
of the content they're inserting.

In any case, count me as an interested party willing to help out with
this.

-- 
Dan Poirier 

Re: 2.2.11 & mod_include

[Dan Poirier]

Lars Eilebrecht  writes:

> Torsten Foertsch wrote:
>
> [mod_include DATE_LOCAL bug]
>> Is this a known bug?
>
> It's probably this one:
> https://issues.apache.org/bugzilla/show_bug.cgi?id=39369

I think that's right.  It's a test for Joe's fix for 39369, that has
only been applied to trunk.  It would be nice to backport that fix so
the stable release doesn't fail tests (or else do something with that
test).

-- 
Dan Poirier 

Re: 2.2.11 & mod_include

[Dan Poirier]

Torsten Foertsch  writes:

> On Wed 01 Apr 2009, Dan Poirier wrote:
>> Lars Eilebrecht  writes:
>> > Torsten Foertsch wrote:
>> >
>> > [mod_include DATE_LOCAL bug]
>> >
>> >> Is this a known bug?
>> >
>> > It's probably this one:
>> > https://issues.apache.org/bugzilla/show_bug.cgi?id=39369
>>
>> I think that's right.  It's a test for Joe's fix for 39369, that has
>> only been applied to trunk.  It would be nice to backport that fix so
>> the stable release doesn't fail tests (or else do something with that
>> test).
>
> Here is a patch that works for 2.2.11. The mod_rerwite patch cures the 
> failure in t/modules/rewrite.t:
>
>   https://issues.apache.org/bugzilla/show_bug.cgi?id=46428
>
> in 2.2.11.
...
> Should I attach these patches to the problem reports in bugzilla or is 
> that useless because they wont be backported officially?
>
> Torsten

These are two separate problems that just happen to have been fixed
recently in trunk.  I haven't looked at the rewrite one.

I don't know any reason why these couldn't be backported, but someone
with commit privileges will have to propose them for backporting.

Dan

Documentation started for virtual hosts with SNI

[Dan Poirier]

[Please followup to d...@httpd.apache.org]

I've started a documentation page for using virtual hosts
over SSL with SNI at

http://wiki.apache.org/httpd/NameBasedSSLVHostsWithSNI

Comments are welcome, or make improvements directly on
the wiki.

-- 
Dan Poirier 

State of Apache::Test?

[Dan Poirier]

I'm wondering - what's the state of Apache::Test and httpd/test?  Are
there active developers using them who understand how they work?  Or
have they fallen into disuse?

Background: I wanted to see if I could write some tests for the new SNI
features, so I started looking at Apache::Test.  Unfortunately it's been
some years since I had my head deep into the Perl way of doing things,
and even after looking over the documentation, I think it would take me
a long time to get to the point where I could write new tests there.

I also notice that the mailing list for Apache::Test seems to be
abandoned, and there have been very few commits to httpd/test in a long
time.  Which leads me to wonder if the whole framework is headed toward
abandonment.

Also, has there been any discussion of an alternative test framework for
Apache?  Even if it's still actively supported, Apache::Test seems to
have an awfully steep learning curve.

-- 
Dan Poirier 

Re: State of Apache::Test?

[Dan Poirier]

Torsten Foertsch  writes:

> On Wed 15 Apr 2009, Dan Poirier wrote:
>> I'm wondering - what's the state of Apache::Test and httpd/test?  Are
>> there active developers using them who understand how they work?  Or
>> have they fallen into disuse?
>>
>> Background: I wanted to see if I could write some tests for the new
>> SNI features, so I started looking at Apache::Test.  Unfortunately
>> it's been some years since I had my head deep into the Perl way of
>> doing things, and even after looking over the documentation, I think
>> it would take me a long time to get to the point where I could write
>> new tests there.
>
> The modperl users list modp...@perl.apache.org is for you. I think 
> you'll find answers very shortly.

Thanks, if I decide to use Apache::Test I'll go there.

I'm still wondering if Apache developers in general (outside the modperl
folks) are still actively using Apache::Test?

-- 
Dan Poirier 

Re: svn commit: r774730 - /httpd/httpd/trunk/server/log.c

[Dan Poirier]

rj...@apache.org writes:

> Log:
> Add name of program to spawn to the error
> message, when starting piped loggers fails.

Thanks.  Error messages that keep secret the information you need to fix
the error are so frustrating.

-- 
Dan Poirier 

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

[Dan Poirier]

Suggestion:

Index: docs/manual/mod/mod_cache.xml
===
--- docs/manual/mod/mod_cache.xml   (revision 777086)
+++ docs/manual/mod/mod_cache.xml   (working copy)
@@ -414,6 +414,8 @@
 an entity in the cache, such that cachable resources are not stored 
separately for
 each session.
 
+CacheIgnoreURLSessionIdentifiers None clears the list of 
ignored
+identifiers.  Otherwise, each identifier is added to the list.
 
 Example 1
   CacheIgnoreURLSessionIdentifiers jsessionid



-- 
Dan Poirier 

Re: Some ramblings on httpd config

[Dan Poirier]

Rich Bowen  writes:

> The  directive, along with better documentation, would solve 50% of what
> people use mod_rewrite for. A less painful way to configure dynamic virtual
> hosts would solve another 15%. And a healthy dose of education about all the
> gross misconceptions called "SEO" would solve the rest.
>
> Yes, we need some kind of macro thingy in the configuration. And while I think
> that most of us reading this email thread can handle that thingy being Lua, I
> honestly don't believe that the folks over on the users@ list can, unless it's
> something that can be embedded into existing configurations, rather than being
> the entirety of the configuration.

If we can fix the biggest problems with the current configuration
scheme, I think that's a powerful argument that we don't need to throw
it out and invent a whole new one.  As others have said, there's a huge
amount of knowledge out there based on the current configuration scheme,
and throwing all that out would be a shame.

-- 
Dan Poirier 

Re: Some ramblings on httpd config

[Dan Poirier]

Niklas Edmundsson  writes:

> Those who can't grasp lua probably couldn't grasp our current config
> scheme either.

I strongly disagree.  There's a huge gulf between being able to describe
a configuration with a few directives, and having to write a program to
implement mod_rewrite or a virtual host yourself.

I really don't want to spend the next however-many-years debugging not
only users' configurations, but their programming as well.

-- 
Dan Poirier 

Re: svn commit: r782860 - in /httpd/httpd/trunk/docs/manual: env.xml mod/mod_cache.xml

[Dan Poirier]

cove...@apache.org writes:

> ==
> --- httpd/httpd/trunk/docs/manual/env.xml (original)
> +++ httpd/httpd/trunk/docs/manual/env.xml Tue Jun  9 01:27:43 2009
> @@ -325,6 +325,7 @@
>  
>  
>  no-cache
> +Available in versions after 2.2.11
>  
>  When set, mod_cache will not save an otherwise
>  cacheable response.  This environment variable does not influence
>

Does "versions after 2.2.11" mean starting with 2.2.11, or starting with
2.2.12?  (I guess literally it means only versions later than 2.2.11,
but I think there's some room for confusion stating it that way.)

Dan

Re: A question about ap_content_length_filter()

[Dan Poirier]

Peter Wang  writes:

> so, my question is: when response size <= 8000,
> the request contains a Content-Length field,
> otherwise, it uses chunked.
>
> 8000 may be very specific on my server, i wonder
> where can i tuning such a limit. I have tried to
> change output_buffering in php.ini from 4096 to
> 8192 to tuning that limit, but it seems nothing changes.

Try adjusting APR_BUCKET_BUFF_SIZE in
srclib/apr-util/buckets/apr_buckets.h (no idea if that'll break
anything, but that's where the 8000 is coming from).

-- 
Dan Poirier 

Re: mod_substitute memory problem

[Dan Poirier]

Nick Gearls  writes:

> There seems to be a memory problem when substituing something on very
> long lines (several hundreds KB). this problem is different from bug
> 44948 (I applied this patch).
>
> When using something like "Substitute s/string1/string2/" on a 300 KB
> line with 30 times "string1" on the line, there are 2 problems:
>
> 1. It uses a huge amount of memory, as - I think - it reallocates a
> new buffer (300 KB) before each substitute without freeing the
> previous one

I think you're right - most of the working memory is not released until
the request is over.  This is kind of a worst-case for mod_substitute's
temporary memory usage.

Is this a problem in real cases?  Or just in artificial testing?
If these are real requests, maybe if you tell a little more about them,
we can find a better solution.

> 2. The memory is not freeed at the end of the HTTP request.
> Maybe is it due to Keep-alive?

If by "the memory is not freed", you mean the memory usage for the
http process as reported by the OS doesn't go down, that's right.
But at the end of the request, the memory should be released back
to Apache to re-use.  You can test this by repeating your test several
times - after the first time, the process's memory usage should not keep
going up the way it does the first time.

If that's a problem in your case, you can set MaxMemFree to ask Apache
to release memory by calling free() above a certain threshold.  Even
then, whether the process releases the memory back to the OS depends on
the OS, so it might not gain you anything.

But if this is going to happen very often, you might as well let Apache
hold on to the memory; it's just going to grab it back again the next
time this happens.

> If using the "q" switch to not flatten the buckets, it uses almost no
> memory. Btw, it correctly handles more 60 substitutions on the same
> line (some shorter, some longer) without the q flag !?! When exactly
> is this flag needed?

Don't know about this one.

> In macro SEDSCAT, we have
>   s2 = apr_pstrmemdup(pool, buff, blen);
> s1 = apr_pstrcat(pool, s1, s2, NULL);
> Shouldn't we free the previous s1 buffer?

Pools don't provide a way of freeing individual allocations; you have
to destroy the whole pool.

> Anyway, I do not understand why the memory is not released, as the
> pool is supposed to be destroyed.

See above.

-- 
Dan Poirier 

Re: rotatelogs - Adding timeout for reading from stdin

[Dan Poirier]

Would wait_for_io_or_timeout() be a good candidate for apr?

-- 
Dan Poirier 

Re: Some ramblings on httpd config

[Dan Poirier]

"Akins, Brian"  writes:

> On 6/10/09 8:52 AM, "Rich Bowen"  wrote:
>
>> If we had a standard set of variables (like, say, HOSTNAME or whatever) that
>> could be interpolated into config directives, as well as a standard way to do
>> variable assignment and backreferences, that would solve a whole class of
>> problems that are really pretty difficult right now.
>
> That sounds a whole lot like using the config as a runtime. 

I'm not sure I follow that.  I do like that the config would still look
a lot like it does now, only more flexible.

-- 
Dan Poirier 

State of mod_auth_digest?

[Dan Poirier]

I was looking at mod_auth_digest and bug 16057.  Currently the shared
memory code in that module is disabled, and it turns out that has
effects throughout the module, such as disabling all client tracking,
nonce-count checking, MD5-sess, and probably other things.

I wonder if this does anything useful at all in its current state?

Regardless, I might take a stab at cleaning some of that up, if nobody
else is working on it already.

-- 
Dan Poirier 

Re: DO NOT REPLY [Bug 16142] MUST use strong comparison for Range requests

[Dan Poirier]

Nick Kew  writes:

> On Tue, 16 Jun 2009 11:14:16 -0400
> Jeff Trawick  wrote:
>
>> IMO potential HTTP violations should remain open until somebody can
>> evaluate them and fix it or explain why the report isn't valid.
>> Thoughts?
>
> +1.
>
> Similar applies (less strongly) to other old bugs that haven't been
> evaluated just 'cos noone has found a round tuit.

Makes sense.  I've re-opened these.

> For some bugs like these, a great exercise might be to restructure
> them, perhaps with use of meta-bugs like PR 43454.  That makes
> things easier as and when someone does find time to hack it.

Sounds like a good idea.  Let me see if I have any round tuits lying
around...

-- 
Dan Poirier 

Re: svn commit: r785425 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_dir.c

[Dan Poirier]

"William A. Rowe, Jr."  writes:

> DefaultHandler implies handling all content; not no-match content.

It seems to me that "Default" is right - it implies what should be done
when no more explicit configuration applies.  E.g. DefaultType,
DefaultIcon, etc.

Whether this is a "Handler" I'm not so sure of.

-- 
Dan Poirier 

Re: testing for connections with keep-alive property from modules

[Dan Poirier]

Kevin J Walters  writes:

> What's the recommended way to check for a connection having the
> property of being kept alive?
>
> I was expecting to see tests like c->keepalive == AP_CONN_KEEPALIVE
> but i see that mod_proxy.c is testing c->keepalives which is the
> incrementing counter of requests processed on that connection. Are
> those two tests equivalent and interchangeable?
>
> E.g. from 2.2.10's mod_proxy_http.c
>
> if ((r->proxyreq == PROXYREQ_REVERSE) && (!c->keepalives)
> && (apr_table_get(r->subprocess_env, "proxy-initial-not-pooled"))) {
> backend->close = 1;
> }

c->keepalives will tell you how many requests the connection has
handled, not whether it's going to be kept alive past the current
request.  For that you need to look at c->keepalive.

-- 
Dan Poirier 

Re: mod_noloris: mitigating against slowloris-style attack

[Dan Poirier]

To avoid a ban on 89.0.0.13 also banning 9.0.0.1, we might want to
include the separators in the strstr, as in the attached patch.

--- mod_noloris.c-orig	2009-07-01 08:57:32.0 -0400
+++ mod_noloris.c	2009-07-01 09:15:21.918474373 -0400
@@ -61,7 +61,7 @@
  * accepts a conn_rec.
  */
 struct { int child_num; int thread_num; } *sbh = conn->sbh;
-
+char search_string[18];
 char *shm_rec;
 worker_score *ws;
 if (shm == NULL) {
@@ -70,7 +70,10 @@
 
 /* check the IP is not banned */
 shm_rec = apr_shm_baseaddr_get(shm);
-if (strstr(shm_rec, conn->remote_ip)) {
+search_string[0] = ' ';
+strcpy(search_string+1, conn->remote_ip);
+strcat(search_string, " ");
+if (strstr(shm_rec, search_string)) {
 ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, conn,
   "Dropping connection from banned IP %s", conn->remote_ip);
 return OK;
@@ -155,6 +158,7 @@
 strcpy(shm_rec++, " ");  /* space == separator */
 strcpy(shm_rec, ip);
 shm_rec += strlen(ip);
+strcpy(shm_rec, " ");  /* put separator at end too */
 }
 }
 }

-- 
Dan Poirier 

Re: svn commit: r791454 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS server/core_filters.c

[Dan Poirier]

traw...@apache.org writes:

> Author: trawick
> Date: Mon Jul  6 12:03:20 2009
> New Revision: 791454
>
> URL: http://svn.apache.org/viewvc?rev=791454&view=rev
> Log:
> SECURITY: CVE-2009-1891 (cve.mitre.org)
> Fix a potential Denial-of-Service attack against mod_deflate or other 
> modules, by forcing the server to consume CPU time in compressing a 
> large file after a client disconnects.  [Joe Orton, Ruediger Pluem]
>
> Submitted by: jorton, rpluem
> Reviewed by:  jim, trawick
>
>
> Modified:
> httpd/httpd/branches/2.2.x/CHANGES
> httpd/httpd/branches/2.2.x/STATUS
> httpd/httpd/branches/2.2.x/server/core_filters.c

Would anyone care to backport this to 2.0.x?  The changes appear to
apply trivially to the core_output_filter() in server/core.c.  I'll
attach the patch:

Index: CHANGES
===
--- CHANGES (revision 791478)
+++ CHANGES (working copy)
@@ -1,6 +1,12 @@
  -*- coding: utf-8 -*-
 Changes with Apache 2.0.64
 
+  *) SECURITY: CVE-2009-1891 (cve.mitre.org)
+ Fix a potential Denial-of-Service attack against mod_deflate or other 
+ modules, by forcing the server to consume CPU time in compressing a 
+ large file after a client disconnects.  PR 39605.
+ [Joe Orton, Ruediger Pluem]
+
   *) SECURITY: CVE-2008-2939 (cve.mitre.org)
  mod_proxy_ftp: Prevent XSS attacks when using wildcards in the path of
  the FTP URL. Discovered by Marc Bevand of Rapid7. [Ruediger Pluem]
Index: server/core.c
===
--- server/core.c   (revision 791906)
+++ server/core.c   (working copy)
@@ -3969,6 +3969,12 @@
 apr_read_type_e eblock = APR_NONBLOCK_READ;
 apr_pool_t *input_pool = b->p;
 
+/* Fail quickly if the connection has already been aborted. */
+if (c->aborted) {
+apr_brigade_cleanup(b);
+return APR_ECONNABORTED;
+}
+
 if (ctx == NULL) {
 ctx = apr_pcalloc(c->pool, sizeof(*ctx));
 net->out_ctx = ctx;
@@ -4336,12 +4342,9 @@
 /* No need to check for SUCCESS, we did that above. */
 if (!APR_STATUS_IS_EAGAIN(rv)) {
 c->aborted = 1;
+return APR_ECONNABORTED;
 }
 
-/* The client has aborted, but the request was successful. We
- * will report success, and leave it to the access and error
- * logs to note that the connection was aborted.
- */
 return APR_SUCCESS;
 }
 


-- 
Dan Poirier 

Re: svn commit: r790589 - /httpd/test/framework/trunk/t/security/CVE-2009-1890.t

[Dan Poirier]

jor...@apache.org writes:

> Author: jorton
> Date: Thu Jul  2 13:42:12 2009
> New Revision: 790589
>
> URL: http://svn.apache.org/viewvc?rev=790589&view=rev
> Log:
> - add test case for CVE-2009-1890
>
> Added:
> httpd/test/framework/trunk/t/security/CVE-2009-1890.t   (with props)

I've been looking at this test and I could use some help understanding
it.  

The test doesn't seem to do what the vulnerability description talks
about.  The vulnerability talks about sending additional data after
sending Content-length bytes of request body, where this test sends a
request body of the right length, just in two parts with a pause in
between.

-- 
Dan Poirier 

Re: svn commit: r790589 - /httpd/test/framework/trunk/t/security/CVE-2009-1890.t

[Dan Poirier]

"Plüm, Rüdiger, VF-Group"  writes:
>> -Original Message-----
>> From: Dan Poirier 
>> Sent: Donnerstag, 9. Juli 2009 15:10
>> To: dev@httpd.apache.org
>> Subject: Re: svn commit: r790589 - 
>> /httpd/test/framework/trunk/t/security/CVE-2009-1890.t
>> 
>> The test doesn't seem to do what the vulnerability description talks
>> about.  The vulnerability talks about sending additional data after
>> sending Content-length bytes of request body, where this test sends a
>> request body of the right length, just in two parts with a pause in
>> between.
>
> It adds a leading '0' to the content-length header causing the old code
> to interpret the content-length as being an octal number.
> Interpreting the content-length as octal results in a much lower content 
> length
> as if it was interpreted as a decimal number.

So if the content-length was parsed correctly, but the vulnerability
related to additional data wasn't fixed, this test would still pass?
(Since then we're not sending any more data than expected?)

-- 
Dan Poirier 

Re: State of mod_auth_digest?

[Dan Poirier]

Joe Orton  writes:

> On Thu, Jun 11, 2009 at 09:46:39AM -0400, Dan Poirier wrote:
>> I was looking at mod_auth_digest and bug 16057.  Currently the shared
>> memory code in that module is disabled, and it turns out that has
>> effects throughout the module, such as disabling all client tracking,
>> nonce-count checking, MD5-sess, and probably other things.
>> 
>> I wonder if this does anything useful at all in its current state?
>
> I vaguely recall that it either crashed, or the RMM segment got full up 
> quickly and it got stuck.  I've had a TODO list item to convert the #if 
> 0'ed code to use of ap_socache.h but never found the time.
>
>> Regardless, I might take a stab at cleaning some of that up, if nobody
>> else is working on it already.
>
> Any and all work on the module would certainly be good!

I've added a patch to bug 16057 to replace the use of shared memory with
socache.  I was hoping the changes would be less pervasive, but the
shared memory assumption showed up in a lot of places.

Comments on the proposed changes would be more than welcome.

-- 
Dan Poirier 

Re: mod_deflate DoS using HEAD

[Dan Poirier]

"William A. Rowe, Jr."  writes:

> Joe Orton wrote:
>> 
>> Does 2616 mandate that a resource must always 
>> exactly the same set of content-codings across methods and time?  
>> (AFAICT there is no MUST on that front; it's a SHOULD if anything)
>
> Read through to the end, it breaks all proxied content;
>
> 9.4 HEAD
>
>The HEAD method is identical to GET except that the server MUST NOT
>return a message-body in the response. The metainformation contained
>in the HTTP headers in response to a HEAD request SHOULD be identical
>to the information sent in response to a GET request. This method can
>be used for obtaining metainformation about the entity implied by the
>request without transferring the entity-body itself. This method is
>often used for testing hypertext links for validity, accessibility,
>and recent modification.
>
>The response to a HEAD request MAY be cacheable in the sense that the
>information contained in the response MAY be used to update a
>previously cached entity from that resource. If the new field values
>indicate that the cached entity differs from the current entity (as
>would be indicated by a change in Content-Length, Content-MD5, ETag
>or Last-Modified), then the cache MUST treat the cache entry as
>stale.

Doesn't that last sentence just indicate that the cache entry will be
invalidated?  That would add some possibly unnecessary work fetching the
content again the next time it's requested, but I wouldn't think it
would break anything.

-- 
Dan Poirier 

Re: mod_cache sends 200 code instead of 304

[Dan Poirier]

Nicholas Sherlock  writes:

> If you make a conditional request for a cached document, but the
> document is expired in the cache, mod_cache currently passes on the
> conditional request to the backend. If the backend responds with a
> "304 Not Modified" response that indicates that the cached copy is
> still up to date, mod_cache serves the contents of the cache to the
> client with a 200 code.

This wouldn't surprise me.  There's currently a bug open for the
opposite case, returning a 304 to an unconditional request (45341).

I believe this violates a SHOULD in 14.25 of RFC 2616, which isn't as
strong as a MUST, but certainly would indicate it's worthwhile to try to
fix it.

I'd suggest opening a bug report
(http://httpd.apache.org/bug_report.html), including all the details
from your original message, so this doesn't fall through the cracks
before someone gets to look at it in more depth.

Dan

Re: [us...@httpd] create errorlog folder

[Dan Poirier]

Jordi Moles  writes:

> I'm sorry if this has been discussed before, but I've been looking
> through this list's archive and found nothing related to it.
...
> For some reason (I'm trying to find out), the path where the error_log
> file is kept gets deleted from time to time i some of the domains
> (each one has its own errorlog folder). After that, when i perform a
> configtest, apache doesn't complain, but when i run graceful, it stops
> working as to apache this is a major problem.
>
> So... i just would like to know if i can do something to avoid this
> from happening... may be can i set up apache so that a message like
> "errorlog file does not exist" appears in the configtest and not only
> in the graceful itself? may be can i make apache create that folder if
> it doesn't exist? may be can i make apache ignore that problem and
> consider it like a warning and not error?
>
> I know that apache is able to create a new error_log file if it
> doesn't find one and has the right privileges, but it fails to create
> the folder. I guess this is the normal and expected behaviour, i just
> need to know if i can make it behave as i need.

[Copying to dev@ list and setting reply-to there.]

I thought I'd seen this reported before but can't find a bugzilla entry
for it.  I'd suggest searching yourself, and if there isn't one,
creating an enhancement request in bugzilla
(http://httpd.apache.org/bug_report.html) so this doesn't get forgotten.

It seems worthwhile to me to check in configtest whether it looks as if
Apache will be able to create or write to the log files.  Apache already
tests whether things like ServerRoot and DocumentRoot exist during
configtest.

I don't think it's possible to check this perfectly without actually
trying to open and write to the log file, which we wouldn't want to do
during configtest, but we can at least see if the directory exists, and
if it appears to be writeable.

-- 
Dan Poirier 

Re: svn commit: r799563 - in /httpd/site/trunk: docs/contributors/index.html xdocs/contributors/index.xml

[Dan Poirier]

--On Friday, July 31, 2009 09:56:07 AM + rj...@apache.org wrote:

URL: http://svn.apache.org/viewvc?rev=799563&view=rev
Log:
Adding back Dan Poirier, now also to xdocs.

Dan missed xdocs in r795301 and Paul
accidentily removed his entry in r799152.


Thanks and sorry for the trouble.  I'm still figuring things out.

Dan

Context lines in subversion update messages

[Dan Poirier]

I've noticed that when folks vote and the STATUS file update messages go 
to the commits list, there's typically not enough context in the email 
to tell what was voted on.  Would it make sense to increase the lines of 
context in these messages?


Dan

Re: [VOTE] httpd 2.2.13 candidate

[Dan Poirier]

On 08/06/2009 03:53 AM, William A. Rowe, Jr. wrote:

Tarballs at /dev/dist/; due to the potential security impact on third
party modules, and my own personal schedule, we'll run this vote for
48 hours and see where it stands.  Windows src.zip to follow shortly.

   +/-1
   [ +1 ]  Release httpd-2.2.13 as GA

(nonbinding)

Ubuntu 9.04 i686: md5sum okay, configures and builds clean, no test 
regressions

Mac OS X 10.5.7 Intel: configures and builds clean, no test regressions

--
Dan Poirier 
"We would worry less about what others think of us if we realized how 
seldom they do." -- Ethel Barrett

Re: Add new comfiguration paeametr to httpd.conf

[Dan Poirier]

On 08/06/2009 06:52 AM, h iroshan wrote:
I have developed a new Apache module. I want to put some parameter at 
httpd.conf . How can I add new a parameter for httpd.conf?.


http://www.apachetutor.org/dev/config would be a good start (linked from 
the Developer Info in the Apache doc online).   Even better, buy _The 
Apache Modules Book_ by Nick Kew; if you're writing a module, there's a 
lot of other information in there that you'll find useful.


What are you working on?  Is it something others might find useful?

--
Dan Poirier 

Re: svn commit: r801528 - /httpd/httpd/branches/2.2.x/CHANGES

[Dan Poirier]

wr...@apache.org writes:

> Author: wrowe
> Date: Thu Aug  6 07:33:32 2009
> New Revision: 801528
>
> URL: http://svn.apache.org/viewvc?rev=801528&view=rev
> Log:
> Two notable notes
>
> Modified:
> httpd/httpd/branches/2.2.x/CHANGES
>
> Modified: httpd/httpd/branches/2.2.x/CHANGES
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/CHANGES?rev=801528&r1=801527&r2=801528&view=diff

This commit added some funny characters to the beginning of CHANGES.

-- 
Dan Poirier 

Re: svn commit: r808906 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS

[Dan Poirier]

Maybe it would be helpful to put a template into STATUS.  Or somewhere
else and put a pointer to it there, anyway.

Dan

On Fri, 28 Aug 2009 12:14 -0700, "Roy T. Fielding" 
wrote:
> On Aug 28, 2009, at 7:44 AM, j...@apache.org wrote:
>
> > Submitted by: rpluem Reviewed/backported by: jim
>
> Hi Jim,
>
> Could you please return to the traditional way of noting this, namely
>
>   Submitted by: Ruediger Pluem Reviewed by: jim, minfrin
>
> We already know who backported it (the committer) and it is kind of
> annoying to have a different way of noting the reviewers for each
> person doing the commit.
>
> I sure miss having the rcsinfo template from cvs -- wish I knew why
> subversion never supported it.
>
> Cheers,
>
> Roy

Should server start if module cannot behave as configured?

[Dan Poirier]

mod_auth_digest cannot implement nonce-count checking or the md5-sess 
algorithm if the platform doesn't have shared memory.


Right now, if the admin configures either of these options and the 
platform doesn't have shared memory, the module issues a warning and 
continues without the requested option.


In my opinion, if a security check that the admin requested in the 
configuration cannot be implemented, it should be more than a warning; 
it should be a fatal startup error.


What's the consensus on changing this?

1) What's the right behavior?

2) If it should be changed, what's the best way to do it?  The change 
could break configurations that currently appear to "work", although 
they're not really doing what the admin configured them to do.


Thanks,
Dan

Re: Should server start if module cannot behave as configured?

[Dan Poirier]

On 09/09/2009 10:57 AM, Jeff Trawick wrote:

2) If it should be changed, what's the best way to do it?  The
change could break configurations that currently appear to "work",
although they're not really doing what the admin configured them to do.


how many affected configurations are we talking about?

* did anything that needed shared memory really work before your recent
fixes?


No.


* are either of these unsupported features the default?


No.


* what platforms have no APR support for shared memory?


That I don't know.

But it seems like I probably don't need to worry too much about breaking 
configurations.  The number of users who have turned on these 
unsupported features even though they don't work is probably pretty small.


Dan

Re: DAV Option Patch

[Dan Poirier]

"Brian J. France"  writes:

> I have updated the dav-options patch against head:
>
>   http://www.brianfrance.com/software/apache/dav/dav-options.diff

One suggestion - using "hook" in the names could be confusing since this
isn't using the built-in hook mechanism.  Since it is using the provider
mechanism, maybe names like "dav_options_provider" would be clearer.

-- 
Dan Poirier 

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

[Dan Poirier]

On 09/14/2009 01:25 PM, traw...@apache.org wrote:

- * htcacheclean: 19 ways to fail, 1 error message. Fixed.
-   Trunk Patch: http://svn.apache.org/viewvc?view=rev&revision=814091
-   +1: minfrin, covener, poirier
+   -1: trawick (Memory use for dumping the database is now unbounded; it
+doesn't even look difficult to avoid allocating/copying.)



Can you elaborate?  I don't see how these changes create additional and 
unbounded memory usage.


Dan Poirier

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

[Dan Poirier]

Dan Poirier  writes:

> Can you elaborate?  I don't see how these changes create additional
> and unbounded memory usage.

Sorry, I was confused and thought the comment applied to another, more 
innocuous proposed backport. I'll go look at this again.

-- 
Dan Poirier 

Re: svn commit: r814091 - in /httpd/httpd/trunk: CHANGES support/htcacheclean.c

[Dan Poirier]

On 09/11/2009 07:57 PM, minf...@apache.org wrote:

Modified: httpd/httpd/trunk/support/htcacheclean.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/support/htcacheclean.c?rev=814091&r1=814090&r2=814091&view=diff
==
--- httpd/httpd/trunk/support/htcacheclean.c (original)
+++ httpd/httpd/trunk/support/htcacheclean.c Fri Sep 11 23:57:48 2009
@@ -776,6 +779,7 @@
  intelligent = 0;
  previous = 0; /* avoid compiler warning */
  proxypath = NULL;
+char errmsg[1024];

  if (apr_app_initialize(&argc,&argv, NULL) != APR_SUCCESS) {
  return 1;


This adds a variable declaration in the middle of a block, which I 
believe is invalid in C89.  (Though gcc doesn't seem to care.)


Dan

Logging command line at startup

[Dan Poirier]

I'd like to log the server command line and server root at startup.

The reason is: sometimes when debugging a problem I'm given some logs
and a directory full of various revisions of the server configuration
file, and can only guess which of the configuration files was actually
being used.

Logging the path to the configuration file would be complicated, since
when it's opened, we don't know where the logs are going to go yet, and
we don't save the configuration file path for later use.

But with the command line and the server root, it won't be hard to tell
which configuration file got used.

I've put a first pass at implementing this at
http://people.apache.org/~poirier/log_command_line_at_startup.patch.txt
and welcome comments.

-- 
Dan Poirier 

Who can cast binding votes?

[Dan Poirier]

While we're discussing voting, I've been looking at
http://httpd.apache.org/dev/guidelines.html

Under "Apache HTTP Server Committers" it says "these volunteers may cast
binding votes on any technical discussion".

Under "Voting", it says "the only binding votes are those cast by active
members of the Apache Group", which sounds like PMC members, but that
would imply that not all committers can cast binding votes,
contradicting what was said earlier.

Can someone clarify that for me?

Thanks.

-- 
Dan Poirier 

mod_ftp - start directory?

[Dan Poirier]

Looking over the mod_ftp documentation, I don't see a way to do this,
but maybe I'm just missing it.

Can I arrange for anyone logging into the mod_ftp server to start out in
some subdirectory of the document root?  I want a fixed directory, not
something that depends on their user name.

For extra credit, can I keep them restricted to that directory and
subdirectories of it?

Concrete example:


FTP On
DocumentRoot /var/share/ftpdocs


When Alice or Bob log in, I want them to be looking at the files in
/var/share/ftpdocs/interesting/directory, and 'pwd' should return
'/interesting/directory'.  

They should be able to access anything under
there, but not anything outside that subtree.

-- 
Dan Poirier 

Re: DAV Provider Patch

[Dan Poirier]

"Brian J. France"  writes:

> On Sep 21, 2009, at 10:15 AM, Graham Leggett wrote:
>
>> Brian J. France wrote:
>>
>>> I believe this is the first patch that will break binary
>>> compatibility
>>> because it adds a function pointer to the middle of the struct.  I
>>> believe binary compatibility could be retained if we add the function
>>> pointers to the end of the struct instead of in the middle.  Moving
>>> the
>>> function could be part of the back porting patch to 2.2, but leave
>>> it as
>>> is in 2.3+.
>>
>> Would it be possible to move the function pointers to the end of the
>> struct for httpd-trunk as well? Breaking binary compatibility is to be
>> avoided if it can be, even on trunk.
>
>
> Sure.  Which method would be preferred?  Having two hooks or just one
> and use the request_rec to get the filename?

As long as the filename can be gotten trivially from the request rec,
I'd say keep things simple and just add the one hook.

-- 
Dan Poirier 

Re: DO NOT REPLY [Bug 15866] cache MUST treat incomplete cached response as partial

[Dan Poirier]

Nick Kew  writes:

> On 23 Sep 2009, at 14:40, bugzi...@apache.org wrote:
>
>> https://issues.apache.org/bugzilla/show_bug.cgi?id=15866
>
> Hey, you're a committer now!  Any reason you didn't just commit?
>
> A patch will generally get more review in /trunk/ than in bugzilla
> if that's your concern.  Or if you have particular issues you need
> to raise, a post to the dev list is in order.

Yes, that was my concern.  Committed now.

-- 
Dan Poirier 

Re: svn commit: r818492 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_cache.c modules/cache/mod_cache.h

[Dan Poirier]

Ruediger Pluem  writes:

> On 09/24/2009 04:25 PM, poir...@apache.org wrote:
>> +   If there's some other cache provider that has to read the whole
>> +   cached body to fill in the brigade, though, that would make
>> +   this rather expensive.
>
> Exactly for this reason I don't like this approach. Currently we have only
> mod_disk_cache but this might change in the future and the provider API is
> open. So there might be third party modules affected by this.
>
> Why don't we just count the bytes as we store them? My first thought would be
> to write a wrapper function for cache->provider->store_body() that could look
> like the following and would be called instead of 
> cache->provider->store_body():

What I'd been thinking about, but haven't implemented yet, was extending
the cache provider API (in trunk only) so we can ask the cache provider
for the size.

Your idea of a wrapper function sounds good for 2.2 though, since it
doesn't require an API change.  How about if I give that a try in trunk
so we can see how it works before considering a backport?

-- 
Dan Poirier 

Re: [vote] release httpd-2.2.14?

[Dan Poirier]

Looks good on Mac OS 10.6.1, with a *
- gpg signature checks on tar.bz2 download
- builds with CC="gcc -arch i386"*
- perl test framework passes

* Without CC="gcc -arch i386" apr failed some tests (testfmt, one
other), so I played it safe and built everything with -arch i386.

-- 
Dan Poirier 

Logging or not logging 408's

[Dan Poirier]

Apache 1.3 logged a 408 in the access log if a connection was received
but no request was received before the timeout.

Apache 2.x does not.  If a partial request is received, a 400 is logged,
but if nothing is received, it silently closes the connection when it
times out.

Logging of 408's might be useful to notice if someone is making
malicious requestless connections to attack the server, and identifying
the IP address of the client.  

Is there some good reason not to log the 408's in this case?

(This is also mentioned in PR39785,
https://issues.apache.org/bugzilla/show_bug.cgi?id=39785)

Thanks,

-- 
Dan Poirier 

Cacheability and s-maxage

[Dan Poirier]

I believe a couple of changes are needed in mod_cache to correctly
consider s-maxage in responses.  I'd like to get the list's opinion
about it.

First, currently if a GET or HEAD request with a query in the URI has a
response with Cache-control: s-maxage=NN, that response is not cached.
>From RFC 2616 13.9, 2nd paragraph, s-maxage specifies an explicit
expiration and so the response should be cached despite the query in the
URI.

Second, currently a successful response with no Last-Modified, Etag, or
Expires headers is not cached.  If it has Cache-control: max-age or
Cache-control: s-maxage, that specifies an expiration, and per RFC
2616 13.4 we would expect it to be cached.

I'm attaching a trunk patch that would implement these changes.

-- 
Dan Poirier 

svn status for directory ~/apache/httpd/httpd/trunk/modules/cache/
0 file(s) marked

  821762 821552 minfrin .
  821762 106103 nd  .indent.pro
  821762 106103 nd  Makefile.in
  821762 521264 fuankg  NWGNUdsk_cach
  821762 711470 wrowe   NWGNUmakefile
  821762 521264 fuankg  NWGNUmod_cach
  821762 646182 fuankg  NWGNUsocachdbm
  821762 646177 fuankg  NWGNUsocachshmcb
  821762 664145 wrowe   cache_cache.c
  821762 664145 wrowe   cache_cache.h
  821762 664145 wrowe   cache_hash.c
  821762 664145 wrowe   cache_hash.h
  821762 420983 fieldingcache_pqueue.c
  821762 420983 fieldingcache_pqueue.h
  821762 808212 minfrin cache_storage.c
  821762 821539 minfrin cache_util.c
  821762 711576 rpluem  config.m4
  ? ?   mod_cache.c.orig
  M   821763 821763 poirier mod_cache.c
  821762 664145 wrowe   mod_cache.dsp
  821762 821202 minfrin mod_cache.h
  821762 659643 bnicholes   mod_cache.imp
  821763 821763 poirier mod_disk_cache.c
  821762 495126 wrowe   mod_disk_cache.dsp
  821762 502365 minfrin mod_disk_cache.h
  821762 743837 fieldingmod_file_cache.c
  821762 495126 wrowe   mod_file_cache.dsp
  821762 106103 nd  mod_file_cache.exp
  821762 757396 jorton  mod_socache_dbm.c
  821762 664240 wrowe   mod_socache_dbm.dsp
  821762 726059 jorton  mod_socache_dc.c
  821762 664240 wrowe   mod_socache_dc.dsp
  821762 757396 jorton  mod_socache_memcache.c
  821762 664240 wrowe   mod_socache_memcache.dsp
  821762 757396 jorton  mod_socache_shmcb.c
  821762 664240 wrowe   mod_socache_shmcb.dsp

Re: Cacheability and s-maxage

[Dan Poirier]

And here's the right patch file, sorry.

-- 
Dan Poirier 

Index: mod_cache.c
===
--- mod_cache.c (revision 821763)
+++ mod_cache.c (working copy)
@@ -767,7 +767,8 @@
 reason = "Expires header already expired, not cacheable";
 }
 else if (!conf->ignorequerystring && r->parsed_uri.query && exps == NULL &&
- !ap_cache_liststr(NULL, cc_out, "max-age", NULL)) {
+ !ap_cache_liststr(NULL, cc_out, "max-age", NULL) &&
+ !ap_cache_liststr(NULL, cc_out, "s-maxage", NULL)) {
 /* if a query string is present but no explicit expiration time,
  * don't cache it (RFC 2616/13.9 & 13.2.1)
  */
@@ -781,14 +782,16 @@
 reason = "HTTP Status 304 Not Modified";
 }
 else if (r->status == HTTP_OK && lastmods == NULL && etag == NULL
- && (exps == NULL) && (conf->no_last_mod_ignore ==0)) {
+ && (exps == NULL) && (conf->no_last_mod_ignore ==0) &&
+ !ap_cache_liststr(NULL, cc_out, "max-age", NULL) &&
+ !ap_cache_liststr(NULL, cc_out, "s-maxage", NULL)) {
 /* 200 OK response from HTTP/1.0 and up without Last-Modified,
  * Etag, or Expires headers.
  */
 /* Note: mod-include clears last_modified/expires/etags - this
  * is why we have an optional function for a key-gen ;-)
  */
-reason = "No Last-Modified, Etag, or Expires headers";
+reason = "No Last-Modified, Etag, Expires, Cache-Control:max-age or 
Cache-Control:s-maxage headers";
 }
 else if (r->header_only && !cache->stale_handle) {
 /* Forbid HEAD requests unless we have it cached already */

Re: svn commit: r724717 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_private.h

[Dan Poirier]

jor...@apache.org writes:

>   (ssl_hook_Fixup): Use modssl_var_extract_dns to insert the
>   SSL_*_DN_ variables efficiently and accurately, handling
>   certs with duplicate RDN tags correctly.

I first read that function name as "extract DNS".  Would changing it to
something like "mod_ssl_extract_dn_vars" be better?

-- 
Dan Poirier 

Re: svn commit: r829619 - in /httpd/httpd/trunk: ./ modules/ssl/

[Dan Poirier]

jor...@apache.org writes:

> Author: jorton
> Date: Sun Oct 25 17:21:10 2009
> New Revision: 829619
...
> +const char *ssl_cmd_SSLStaplingResponseTimeSkew(cmd_parms *cmd, void *dcfg,
> +const char *arg)
> +{
> +SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
> +sc->server->stapling_resptime_skew = atoi(arg);
> +if (sc->server->stapling_resptime_skew < 0) {
> +return "SSLstapling_resptime_skew: invalid argument";
> +}
> +return NULL;
> +}
> +
> +const char *ssl_cmd_SSLStaplingResponseMaxAge(cmd_parms *cmd, void *dcfg,
> +const char *arg)
> +{
> +SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
> +sc->server->stapling_resp_maxage = atoi(arg);
> +if (sc->server->stapling_resp_maxage < 0) {
> +return "SSLstapling_resp_maxage: invalid argument";
> +}
> +return NULL;
> +}
> +
> +const char *ssl_cmd_SSLStaplingStandardCacheTimeout(cmd_parms *cmd, void 
> *dcfg,
> +const char *arg)
> +{
> +SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
> +sc->server->stapling_cache_timeout = atoi(arg);
> +if (sc->server->stapling_cache_timeout < 0) {
> +return "SSLstapling_cache_timeout: invalid argument";
> +}
> +return NULL;
> +}
> +
> +const char *ssl_cmd_SSLStaplingErrorCacheTimeout(cmd_parms *cmd, void *dcfg,
> + const char *arg)
> +{
> +SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
> +sc->server->stapling_errcache_timeout = atoi(arg);
> +if (sc->server->stapling_errcache_timeout < 0) {
> +return "SSLstapling_errcache_timeout: invalid argument";
> +}
> +return NULL;
> +}
...
> +const char *ssl_cmd_SSLStaplingResponderTimeout(cmd_parms *cmd, void *dcfg,
> +const char *arg)
> +{
> +SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
> +sc->server->stapling_responder_timeout = atoi(arg);
> +sc->server->stapling_responder_timeout *= APR_USEC_PER_SEC;
> +if (sc->server->stapling_responder_timeout < 0) {
> +return "SSLstapling_responder_timeout: invalid argument";
> +}
> +return NULL;
> +}

Shouldn't we check these arguments for validity before using them,
rather than after?

Dan

Re: svn commit: r829619 - in /httpd/httpd/trunk: ./ modules/ssl/

[Dan Poirier]

Ruediger Pluem  writes:

> On 10/26/2009 01:37 PM, Dan Poirier wrote:
>> jor...@apache.org writes:
>> 
>>> Author: jorton
>>> Date: Sun Oct 25 17:21:10 2009
>>> New Revision: 829619
>> ...
>>> +const char *ssl_cmd_SSLStaplingResponseTimeSkew(cmd_parms *cmd, void *dcfg,
>>> +const char *arg)
>>> +{
>>> +SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
>>> +sc->server->stapling_resptime_skew = atoi(arg);
>>> +if (sc->server->stapling_resptime_skew < 0) {
>>> +return "SSLstapling_resptime_skew: invalid argument";
>>> +}
>>> +return NULL;
>>> +}
>> 
>> Shouldn't we check these arguments for validity before using them,
>> rather than after?
>
> Where do we use them so far?
> The are the functions to process the directives.

I meant that we assign a new value to the configuration before we know
whether that new value is valid.

It now occurs to me that while the code in isolation looks suspicious,
returning an error means the server won't start, so the point is moot.
Never mind.

Dan

Re: svn commit: r831031 - in /httpd/httpd/branches/2.2.x: ./ CHANGES STATUS docs/manual/caching.xml modules/cache/mod_cache.c

[Dan Poirier]

Jeff Trawick  writes:

> On Thu, Oct 29, 2009 at 1:06 PM,   wrote:
>> Author: poirier
>> Date: Thu Oct 29 17:06:15 2009
>> New Revision: 831031
>>
>> URL: http://svn.apache.org/viewvc?rev=831031&view=rev
>> Log:
>> Merge r823536, r823563 from trunk:
...
>> Reviewed by: poirier
>
> The value for "Reviewed by" should be the two or more people in this
> list other than the author:
>
> +1: poirier, sf, covener

Makes sense; fixed.

I used http://people.apache.org/~jorton/svn.merge to do this backport
merge, which is where the "Reviewed by: poirier" came from.  Is there an
updated version of that or another script for doing the routine work
of a backport?

Dan

Re: svn commit: r885606 - /httpd/httpd/trunk/build/rpm/httpd.init

[Dan Poirier]

minf...@apache.org writes:

> Author: minfrin
> Date: Mon Nov 30 22:53:43 2009
> New Revision: 885606
>
> URL: http://svn.apache.org/viewvc?rev=885606&view=rev
> Log:
...
> Remove the use of the apachectl script, as a script calling
> another script makes no sense. 

I wonder if this is a good idea?

apachectl does some things that httpd.init does not.  For example, an
admin might reasonably expect to be able to control Apache's environment
by editing bin/envvars, but this will now silently fail when Apache is
installed using RPM.

Of course httpd.init could also source bin/envvars, but then we start
down the road of having to keep httpd.init in sync with apachectl.

I think we should just use the documented, recommended way of
controlling apache.  The extra cost seems minimal, especially compared
to the long-term maintenance costs of not doing so.

Dan

Re: svn commit: r885606 - /httpd/httpd/trunk/build/rpm/httpd.init

[Dan Poirier]

Graham Leggett  writes:

> Redhat's init scripts don't work anything like the apachectl script, and
> trying to call one from the other causes the exact problem you mention -
> loss of environment variables, and all round weirdness.

Maybe I'm just slow getting back up to speed after the holiday, but I'm
not seeing how it would cause a problem.  Could you please expand on
that?

-- 
Dan Poirier 

Re: svn commit: r885606 - /httpd/httpd/trunk/build/rpm/httpd.init

[Dan Poirier]

Graham Leggett  writes:

> Dan Poirier wrote:
>
>>> Redhat's init scripts don't work anything like the apachectl script, and
>>> trying to call one from the other causes the exact problem you mention -
>>> loss of environment variables, and all round weirdness.
>> 
>> Maybe I'm just slow getting back up to speed after the holiday, but I'm
>> not seeing how it would cause a problem.  Could you please expand on
>> that?
>
> On a Redhat machine (Fedora/RHEL/Centos), search
> /etc/rc.d/init.d/functions for functions called "daemon", "status" and
> "killproc". These functions provide similar but incompatible
> functionality to that provided by apachectl, and only exist on Redhat
> derived machines.

Well, apachectl doesn't provide any of that functionality.  About all it
does is source bin/envvars, if it exists, and then call httpd and pass
along any command line arguments.  httpd does all the start/stop/restart
etc. logic internally.  

So I'm still not seeing the problem with setting httpd=apachectl in
httpd.init, which would make bin/envvars apply, and not really change
the behavior otherwise.

> The startup script is far too trivial to justify jumping through hoops
> to try and make apachectl work like Redhat's init.

That's not what I was suggesting, and I don't think it would be
necessary.

> It's caused us enough grief already, thus the fix.

What grief is that?  I don't actually know what problem we're trying to
fix here.

Dan

Re: requests possibly not reaching the log phase

[Dan Poirier]

John ORourke  writes:

> The Authorize.net system makes HTTP POST requests to our server, and
> about 1 in every 500 transactions, the Authorize.net system reports a
> timeout and there's no trace of the request in our logs.  Authorize.net
> won't investigate in any detail because their system is reporting that
> the request simply timed out.

This could happen in Apache 2 if the web server timed out before reading
the complete request.  (Apache 1.3 logs a 408; Apache 2 doesn't; Apache
trunk will log a 4xx of some sort depending on how much of the request
was read.)

If that's what's happening, it would indicate a problem outside of
Apache.  You could confirm it by backporting the fix from
https://issues.apache.org/bugzilla/show_bug.cgi?id=39785 so logging
would occur.  Then you might need to resort to packet tracing or
something like that to figure out exactly what's going on.

Dan

Re: apache module's privileges

[Dan Poirier]

Jordi Prats  writes:

> If you start apache with root as usual, you realize that every module
> is able to run code with root privileges:
...
> Why is coded this way? Shouldn't run with lower privileges? 

No.  That's not the purpose of apache modules.

Re: apache module's privileges

[Dan Poirier]

Graham Dumpleton  writes:

> 2009/12/16 Dan Poirier :
>> Jordi Prats  writes:
>>
>>> If you start apache with root as usual, you realize that every module
>>> is able to run code with root privileges:
>> ...
>>> Why is coded this way? Shouldn't run with lower privileges?
>>
>> No. That's not the purpose of apache modules.
>
> There is a lot more to it than that.

Well, yeah, but the main misconception seemed to be that the purpose of
Apache modules was to limit the privileges available to modules.
("Shouldn't [sic] run with lower privileges?")  In reality if you run
Apache as root and load a module, that module can do anything it wants
as root, and that's by design, not an inherent flaw in Apache.

Re: Per module LogLevel configuration

[Dan Poirier]

"William A. Rowe Jr."  writes:

> One thing we should refactor is 'debug' logging.  Proper debug
> logging is log early and often, but there is overhead involved
> in preparing the args and submitting the log request, only to have
> it fall on deaf ears.
>
> If we are doing any significant 2.0 refactoring, toggling the
> truly-debug log level processing at compile time would be a big win.

Do we have some measurements of how much overhead this adds?

Being able to turn on debug logging by just changing the configuration
is much more convenient than requiring a recompile, and I'd hate to give
that up without knowing we were gaining significantly in performance.

Re: Per module LogLevel configuration

[Dan Poirier]

"William A. Rowe Jr."  writes:

> Dan Poirier wrote:
>> "William A. Rowe Jr."  writes:
>> 
>>> One thing we should refactor is 'debug' logging.  Proper debug
>>> logging is log early and often, but there is overhead involved
>>> in preparing the args and submitting the log request, only to have
>>> it fall on deaf ears.
>>>
>>> If we are doing any significant 2.0 refactoring, toggling the
>>> truly-debug log level processing at compile time would be a big win.
>> 
>> Do we have some measurements of how much overhead this adds?
>
> Of course not, since it varies wildly by what is being logged.  But just
> the preparation can consume double buffers/invoke a copy with the simple
> presence of an LF character.

Looking at log_error_core(), it appears that if the logging level is set
to disallow a particular message from being logged, that
log_error_core() returns before doing any argument processing.  So the
overhead would only consist of a few function calls, and my sense would
be that saving that wouldn't make a big difference.

Of course common sense is notoriously unreliable in making judgments
about performance :-), but this does make me even more interested in
seeing some evidence of significant improvement before we make major
changes in this area at the expense of making problem determination more
difficult.

Dan

Re: [VOTE] Formal deprecation of 1.3.x branch

[Dan Poirier]

Colm MacCárthaigh  writes:

> Because ... stealing an idea from wrowe@ ... how about we formally
> deprecate the 1.3.x branch? Make one more release, but attach a notice
> to the effect that it will be the final release, and that in future
> we'll be distributing security updates by other means :-)

(non-binding) +1

While we're at it, how about issuing a statement regarding how much
longer 2.0.x will be supported?  It doesn't seem to get much maintenance
attention these days either.

Dan

Re: svn commit: r896392 - in /httpd/httpd/branches/1.3.x: README src/CHANGES

[Dan Poirier]

c...@apache.org writes:

> --- httpd/httpd/branches/1.3.x/README (original)
> +++ httpd/httpd/branches/1.3.x/README Wed Jan  6 11:13:11 2010
> @@ -14,8 +14,17 @@
>The Latest Version
>--
>  
> -  Details of the latest version can be found on the Apache HTTP
> -  server project page under http://httpd.apache.org/.
> +  As of January 2010, version 1.3 of Apache has reached its end
> +  of life. Future security updates will be available as patches, 
> +  via;
> +  
> +http://www.apache.org/dist/httpd/patches/

It might be worth emphasizing that there's no commitment to produce any
future updates, even for security issues, and even if patches were
provided for something, they wouldn't be official Apache code releases.

Dan

Re: svn commit: r896392 - in /httpd/httpd/branches/1.3.x: README src/CHANGES

[Dan Poirier]

Ruediger Pluem  writes:

> IMHO patches in the patches directory are as official as previous patches.

Oh, sorry, I didn't realize that.  I don't think I've ever seen a vote
to release a patch.

mod_reqtimeout backport (was: svn commit: r897527 - /httpd/httpd/branches/2.2.x/STATUS)

[Dan Poirier]

Some comments based on
:

Code:

- The units are confusing in the computation and use of the rate_factor
  values.  rate_factor is computed as
 
apr_time_from_sec(1)/min_rate

  where min_rate's units are bytes/second, so the units for the
  rate_factor are seconds^2/(10^6 * bytes).  Then in extend_timeout(),
  that gets multipled by a length in bytes, so we end up with units of
  seconds^2/10^6 where we really want microseconds.

  I think it would be clearer to write the computation of the
  rate_factor as

 rate_factor = APR_USEC_PER_SEC / min_rate

  so the units work out as usec/byte and the timeout ends up
  in usec.

  (Yes, you end up with the same numbers, but it took me quite a while
  to prove that to myself, given that the units weren't right.)

- I'd feel better if most of the AP_DEBUG_ASSERTs were replaced by code
  that would fail the request.  In production this could result in
  crashes at runtime if any of these bad return values actually happen.

- If headermax precedes headerinit on the RequestTimeout directive line,
  there's no check that headerinit <= headermax.  Similarly for bodymax
  and bodyinit.

Documentation:

- Should the module status still be "Experimental" if it's backported?

- Compatibility should show this available in 2.2.something if it's
  backported.

- s/timout/timeout/g

- Maybe an explanatory paragraph should be added, about xxxinit and
  xxxminrate and xxxmax and how they relate to each other and affect the
  timeout of requests.  That would resolve some questions I had as I was
  reading through this, like "why is this called headerinit instead of
  headerread?".


Dan

Re: mod_reqtimeout backport

[Dan Poirier]

Stefan Fritsch  writes:

> I am not particularly happy about the syntax, yet. I just had the idea 
> to have one keyword xxx that can optionally accept a range, instead of 
> two keywords xxxinit and xxxmax:
>
>   Header=30
>   Body=5-50 BodyMinRate=500
>
> or 
>
> Header=5-20,minrate=500
> Body=5-50,minrate=500
>
> but the combination
>
> Header=5,minrate=1000
>
> is less clear. Maybe make the '-' mandatory in this case:
>
> Header=5-,minrate=1000
>
> Any opinions or better ideas?
>
> One could also rename RequestTimeout into RequestReadTimeout if that 
> makes it easier to understand.

We might simplify the model by not exposing the internal extending of
the timeout.  Just let the admin specify an overall max time, a minimum
rate, or both:

  HeaderTimeout: Maximum seconds to read the entire header.

  HeaderMinRate: Minimum rate (bytes/second) allowed when reading the
  request header.  If the rate drops below this, the request is timed
  out.

We'd enforce both if specified.  In that case HeaderTimeout would act
like headermax.  Internally we'd probably implement HeaderMinRate by
gradually extending a timeout, but we wouldn't be tied to that.

Dan

Re: mod_reqtimeout backport

[Dan Poirier]

"Plüm, Rüdiger, VF-Group"  writes:

>> -Original Message-
>> From: news [mailto:n...@ger.gmane.org] On Behalf Of Dan Poirier

>> We might simplify the model by not exposing the internal extending of
>> the timeout.  Just let the admin specify an overall max time, 
>> a minimum
>> rate, or both:
>> 
>>   HeaderTimeout: Maximum seconds to read the entire header.
>> 
>>   HeaderMinRate: Minimum rate (bytes/second) allowed when reading the
>>   request header.  If the rate drops below this, the request is timed
>>   out.
>> 
>> We'd enforce both if specified.  In that case HeaderTimeout would act
>> like headermax.  Internally we'd probably implement HeaderMinRate by
>> gradually extending a timeout, but we wouldn't be tied to that.
>
> But that would result in different behaviour, wouldn't it?
>
> e.g. with init timeout set to 10 max timeout set to 30 and minrate set 500
> the client can wait for 10 seconds before sending data at a rate of 500 
> bytes/sec.
> If I understand your model correctly we would cancel the request anytime if 
> the client
> falls below 500 bytes/sec. So if it does start only with 200 bytes/sec we 
> would cancel
> it immediately.

Yes, my proposal probably simplifies things too much.  We could allow
some time for things to get going.  Maybe not start enforcing the
minimum rate until after some number of seconds, with a reasonable
default but it could be configured:

  HeaderStartupTime: time in seconds before the specified HeaderMinRate
  starts being enforced.  Default = 10 seconds.

I'm not thrilled with that though, it's inelegant.

Front page - ApacheCon US 2009

[Dan Poirier]

I noticed that http://httpd.apache.org still invites folks to attend
ApacheCon US 2009, which was last November.  Maybe it's time to change
that to link to the video archives, and announce whatever the next
ApacheCon will be.

Dan

Re: mod_reqtimeout backport

[Dan Poirier]

Stefan Fritsch  writes:

> In any case, we need at least three values to completely define the
> behaviour. IIRC I chose the initial timeout/maximum timeout over the
> startup time/maximum timeout approach because it was easier to
> implement. I still think it's ok, given that for normal
> configurations, there is not much difference. But the "headerinit"
> keyword is just a bit too cryptic for my taste.

> Do you agree that using RequestReadTimeout instead of RequestTimeout
> and using a single keyword with a timeout range is more descriptive
> than the current syntax?

Yes, I like that better too.

Dan

Re: svn commit: r904768 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/core.xml server/core.c

[Dan Poirier]

I'm just curious, why use Define with ! rather than creating a new
Undefine directive?

Dan

Re: svn commit: r906039 - in /httpd/httpd/trunk/modules/ssl: mod_ssl.c ssl_engine_config.c ssl_engine_init.c ssl_engine_kernel.c ssl_private.h

[Dan Poirier]

How about logging a dire warning during startup if insecure
renegotiation has been enabled?

Dan

Re: [PATCH] report "reached MaxClients" more than once

[Dan Poirier]

Nice.  Maybe change "count" to "messages_skipped" but that's just a
quibble.

I wonder where else this would be handy?

Dan

Re: svn commit: r906779 - /httpd/httpd/trunk/docs/manual/configuring.xml

[Dan Poirier]

[Moving discussion from dev@ to d...@httpd.apache.org]

On Fri, Feb  5, 2010, at 06:57:58 AM, Jeff Trawick  wrote:

> On Thu, Feb 4, 2010 at 9:48 PM,   wrote:
>> +    Only environment variables defined before the server is started
>> +    can be used in expansions.  Variables defined in the
>> +    configuration file itself, for example with > +    module="mod_env">SetEnv, take effect too late to be
>> +    used for expansions in the configuration file.
>
> I think the "take effect too late..." wording supports the common
> confusion that OS-level environment variables and those server
> variables that are set in the environment of sub-processes are
> essentially the same thing.
>
> I don't know what the magic distinguishing words are.  Perhaps "OS
> environment variable," with a link to a new glossary entry, should be
> used in the appropriate places?  The glossary entry for Environment
> variable describes the difference but doesn't introduce unique
> terminology for the two types of variables
> (http://httpd.apache.org/docs/2.2/glossary.html#environmentvariable.

I agree that it's still ambiguous or even misleading. I played with
trying to expand on that, but couldn't come up with wording I liked last
night.

It would definitely help to have unique terminology.  Maybe "operating
system environment variables" for the ones set outside the server in the
OS, and "server environment variables" for the ones set inside the
server?  There's still some possible confusion because the "server"
variables become operating system environment variables for CGI scripts,
though.

If we can find consensus on terminology, I'll update the glossary and
the env page , and try to
find and disambiguate as many references as I can find and link back to
the glossary.

Dan

Re: [PATCH] LogLevel refactoring part 1

[Dan Poirier]

On Wed, Feb  3, 2010, at 03:22:21 AM, Stefan Fritsch  wrote:

> ap_log_error_wrapper.diff:
> On C99 compilers, avoid argument setup and function call overhead if 
> the log message will be discarded anyway. Also allow to disable higher 
> loglevels at compile time by defining APLOG_MAX_LOGLEVEL.
> On pre-C99 compilers, it should just work like before.

This seems like a reasonable thing to do.  I can't comment on the
correctness, not being up on C99.  

Is there some server coding convention calling for trailing _ and __ on
the macro and function names?  If not, my personal preference would be
something more meaningful when reading the code.

I'd love to know difference this makes in processor usage under load,
between running with loglevel debug and something lower.  Saving a
function call for every logging line on the main path could be a big
win.

> loglevel_trace.diff:
> Introduce additional log levels trace1 ... trace8 above the debug 
> level.

If we're thinking about expanding control of debug messages, my
inclination would be to work toward allowing independent control of
messages from different parts of the code or about different things,
rather than a strict series of increasing levels of logging.  E.g. maybe
today I'd like to see all debug trace from authentication, but tomorrow
just see SSL stuff.

Dan

What goes in CHANGES and svn log comments?

[Dan Poirier]

Do we have any formal conventions (i.e. written down) on what kinds of
changes require entries in CHANGES and what doesn't, and what
information we include in CHANGES and svn log entries?

The most logical place for something like that seems to be near the
bottom of http://httpd.apache.org/dev/guidelines.html, but I'm not
seeing anything there.

If we don't have any documentation of our conventions, I'll volunteer to
write something (if there seems to be consensus on what the conventions
are).

Dan