[PATCH] Prevent possible segv
Jeff,
Does this resolve the issue you added the comment for?
Sander
Index: modules/mappers/mod_negotiation.c
===
RCS file: /home/cvs/httpd-2.0/modules/mappers/mod_negotiation.c,v
retrieving revision 1.96
diff -u -r1.96 mod_negotiation.c
--- modules/mappers/mod_negotiation.c 12 Mar 2002 11:48:32 - 1.96
+++ modules/mappers/mod_negotiation.c 12 Mar 2002 12:20:01 -
-794,8 +794,12
{
char *endbody;
int bodylen;
+int taglen;
apr_off_t pos;
+taglen = strlen(tag);
+*len -= taglen;
+
/* We are at the first character following a body:tag\n entry
* Suck in the body, then backspace to the first char after the
* closing tag entry. If we fail to read, find the tag or back
-803,13 +807,11
*/
if (apr_file_read(map, buffer, len) != APR_SUCCESS) {
return -1;
-}
-/* XXX next line can go beyond allocated storage and segfault,
- * or worse yet go beyond data read but not beyond allocated
- * storage and think it found the tag
- */
+}
+
+strncpy(buffer + *len, tag, taglen);
endbody = strstr(buffer, tag);
-if (!endbody) {
+if (!endbody || endbody == buffer + *len) {
return -1;
}
bodylen = endbody - buffer;
RE: [PATCH] Prevent possible segv
From: Sander Striker [mailto:[EMAIL PROTECTED]]
Sent: 12 March 2002 13:36
Jeff,
Does this resolve the issue you added the comment for?
Sander
Index: modules/mappers/mod_negotiation.c
===
RCS file: /home/cvs/httpd-2.0/modules/mappers/mod_negotiation.c,v
retrieving revision 1.96
diff -u -r1.96 mod_negotiation.c
--- modules/mappers/mod_negotiation.c 12 Mar 2002 11:48:32 - 1.96
+++ modules/mappers/mod_negotiation.c 12 Mar 2002 12:20:01 -
@@ -794,8 +794,12 @@
{
char *endbody;
int bodylen;
+int taglen;
apr_off_t pos;
+taglen = strlen(tag);
+*len -= taglen;
+
/* We are at the first character following a body:tag\n entry
* Suck in the body, then backspace to the first char after the
* closing tag entry. If we fail to read, find the tag or back
@@ -803,13 +807,11 @@
*/
if (apr_file_read(map, buffer, len) != APR_SUCCESS) {
return -1;
-}
-/* XXX next line can go beyond allocated storage and segfault,
- * or worse yet go beyond data read but not beyond allocated
- * storage and think it found the tag
- */
+}
+
+strncpy(buffer + *len, tag, taglen);
endbody = strstr(buffer, tag);
-if (!endbody) {
+if (!endbody || endbody == buffer + *len) {
Ahum, just:
if (endbody == buffer + *len) {
would do the trick. ;)
return -1;
}
bodylen = endbody - buffer;
Sander
RE: [BUG] Limit test 10 is failing
From: Doug MacEachern [mailto:[EMAIL PROTECTED]] Sent: 12 March 2002 17:36 On Tue, 12 Mar 2002, Sander Striker wrote: #User-Agent: libwww-perl/5.53 could be a bug in the client. try 5.64 you can also grab: http://httpd.apache.org/~dougm/httpd-test-bundle-0.02.tar.gz unpack and run: % echo | perl Makefile.PL make install (the 'echo |' trick makes all prompts use the default) ignore warnings about prerequisites, they are probably complaining missing modules which are about to be installed. That fixes the problem, but I still find it weird that the response code can in this case be: 500 (Internal Server Error) short write Where the correct response code is logged. Sander
RE: dumb questions on a couple of the current 2.0 showstoppers
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]]On Behalf Of Jeff Trawick Sent: 13 March 2002 00:00 Cliff Woolley [EMAIL PROTECTED] writes: The pool allocator change is done AFAIK -- Sander, are there any other changes that need to be made above the ones in the patch you sent me? Changes. I'm still not sold on the way to handle the allocator ownership. And allocator locking. I posted them to the list aswell, btw. If not, please go ahead and commit. If noone else sees problems with it (other than the docstrings missing from the apr_allocator.h file), I'll commit it in the morning (GMT +1 morning that is). The bucket freelist change was kind of waiting on the pool allocator change, but the API change part can go ahead and get committed as is without the pool change--it would just be a wrapper around malloc until the pool change is done, at which point the freelist allocator can be dropped in. Cool. When I get back to Charlottesville on Thursday I'll finish up and commit whatever part of the buckets change I can depending on the pool patch status. Dang, that's exactly what I wanted to hear :) :) Sander
Code questions (server/mpm_common.c)
Hi,
server/mpm_common.c:363
#if defined(QNX) || defined(MPE) || defined(BEOS) || defined(_OSD_POSIX) ||
defined(TPF) || defined(__TANDEM) || defined(OS2) ||
defined(WIN32) || defined(NETWARE)
Can I break this line into smaller chunks? If so, how?
I seem a bit rusty on how the various preprocessors handle
multiline #ifs (if they even do).
server/mpm_common.c:442
rv = apr_file_close(pod-pod_in);
if (rv != APR_SUCCESS) {
return rv;
}
return rv;
}
If we are going to waste the if, we might aswell return
APR_SUCCESS, no?
Sander
Minor(?) style questions
Hi,
Just checking if people have given this some thought before.
And, maybe if there was something decided on this matter. For
instance, that this is free when it comes to style rules.
As some of you have noticed I am doing style reviews of/
corrections on the current httpd codebase, which has the nice
sideeffect of getting us some basic code review aswell. With
everything in a consistent style, review by other parties will
be easier aswell. Not to mention that one can probably read
the code quicker...
So, just a few simple questions:
1) Can we decide on a standard style when it comes to using
++ or --?
Example:
lines++;
vs.
++lines;
We are currently mixing this in the current code base.
I personally favor the first. And unless we are testing
the variable in an expression, pre- or postfix doesn't
matter for the resulting binary, only for readability.
2) How should a do {} while look like?
do {
...
} while (...);
or
do {
...
}
while (...);
3) What is the preferred method of doing an infinite loop?
while (1) {
...
}
or
for (;;) {
...
}
Thanks,
Sander
RE: cvs commit: httpd-2.0/docs/manual/developer filters.html index.html
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]] Sent: 11 March 2002 13:04 To: [EMAIL PROTECTED] Subject: cvs commit: httpd-2.0/docs/manual/developer filters.html index.html striker 02/03/11 04:03:44 Modified:docs/manual/developer index.html Added: docs/manual/developer filters.html Log: Add the How filters work section. Cut 'n paste job from Ryans email 022501c1c529$f63a9550$7f0a@KOJ, needs formatting. Submitted by: Ryan Bloom I just dropped it in, because I didn't want to see it get 'lost' in the archives. Sander
Code questions (server/protocol.c)
Hi,
server/protocol.c:136
if (ap_strcasestr(type, charset=) != NULL) {
/* already has parameter, do nothing */
/* XXX we don't check the validity */
;
}
Validity checking seems like a good idea, someone
want to grab this one?
server/protocol.c:658
#if 0
/* XXX If we want to keep track of the Method, the protocol module should do
* it. That support isn't in the scoreboard yet. Hopefully next week
* sometime. rbb */
ap_update_connection_status(AP_CHILD_THREAD_FROM_ID(conn-id), Method,
r-method);
#endif
Can this block go? 'next week' has been over 6 months now :)
server/protocol.c:823
r-request_config = ap_create_request_config(r-pool);
/* Must be set before we run create request hook */
r-proto_output_filters = conn-output_filters;
r-output_filters = r-proto_output_filters;
r-proto_input_filters = conn-input_filters;
r-input_filters = r-proto_input_filters;
ap_run_create_request(r);
To what code does the comment refer? The line above it, or
the block under it?
server/protocol.c:1133
/* Humm, is this check the best it can be?
* - protocol = HTTP/1.1 implies support for chunking
* - non-keepalive implies the end of byte stream will be signaled
*by a connection close
* In both cases, we can send bytes to the client w/o needing to
* compute content-length.
* Todo:
* We should be able to force connection close from this filter
* when we see we are buffering too much.
*/
The comment says it all.
server/protocol.c:1290
AP_DECLARE(size_t) ap_send_mmap(apr_mmap_t *mm, request_rec *r, size_t offset,
size_t length)
I reckon the size_t's are left here intentional, weren't forgotten when
switching to apr_size_t?
server/protocol.c:1338
/* future optimization: record some flags in the request_rec to
* say whether we've added our filter, and whether it is first.
*/
Still valid?
server/protocol.c:1501
/* ### TODO: if the total output is large, put all the strings
* ### into a single brigade, rather than flushing each time we
* ### fill the buffer
*/
And that's another one for our performance freaks ;)
Sander
RE: Code questions (server/mpm_common.c)
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED]]On Behalf Of Jeff Trawick
Sent: 11 March 2002 15:17
Sander Striker [EMAIL PROTECTED] writes:
Hi,
server/mpm_common.c:363
#if defined(QNX) || defined(MPE) || defined(BEOS) || defined(_OSD_POSIX) ||
defined(TPF) || defined(__TANDEM) || defined(OS2) ||
defined(WIN32) || defined(NETWARE)
Can I break this line into smaller chunks? If so, how?
I seem a bit rusty on how the various preprocessors handle
multiline #ifs (if they even do).
Look in ap_config.h for the way to do it (look for SUNOS4)
Why not move this OS-specific stuff to ap_config.h and define
AP_HAVE_SUPLEMENTARY_GROUPS if we aren't on one of those platforms,
and then use AP_HAVE_SUPPLEMENTARY_GROUPS to see what to do?
(I'm assuming that the presence of setgrent() and friends isn't good
enough to make the decision.)
Takers?
server/mpm_common.c:442
rv = apr_file_close(pod-pod_in);
if (rv != APR_SUCCESS) {
return rv;
}
return rv;
}
If we are going to waste the if, we might aswell return
APR_SUCCESS, no?
plenty of ways to skin a cat, all of them good
As a cat owner I sincerely how this is a figure of speech... :)
as for this code, go for the APR_SUCCESS (you're left with a trade-off
between compactness vs. using a style that can be extended if more
work is added later)
Yah. Will do. Just raising this for anyone else coding in this kind
of style.
Sander
RE: Copyright year bumping
From: Justin Erenkrantz [mailto:[EMAIL PROTECTED]]
Sent: 09 March 2002 08:57
On Sat, Mar 09, 2002 at 02:54:01AM -0500, Dave Jones wrote:
On Sat, Mar 09, 2002 at 12:20:23PM +0800, Stas Bekman wrote:
Sander Striker wrote:
Hi,
Should we bump the copyright year on all the files?
Anyone have a script handy?
find . -type f -exec perl -pi -e 's|2000-2001|2000-2002|' {} \;
It always seems to me that if you are going to put a copyright date on each
individual source file, you shouldn't change it unless you actually make a
change to that file.
There's no way each developer has changed the copyright dates before
committing any modifications. We're lucky if it compiles before
it gets checked in - never mind altering a date. =( -- justin
*grin*
Ok, people, give me some +1's and I'll move forward and change all
the copyright dates in one swoop.
Sander
More code questions...
server/main.c:578 /* This is a hack until we finish the code so that it only reads * the config file once and just operates on the tree already in * memory. rbb */ What's the status on this? Sander
RE: Config path brokenness
From: Brian Havard [mailto:[EMAIL PROTECTED]]
Sent: 07 March 2002 14:20
With the current HEAD (configured using --prefix=/Apps/apache2), I get the
following in my ap_config_auto.h:
/* Location of the config file, relative to the Apache root directory */
#define SERVER_CONFIG_FILE /Apps/apache2/conf/httpd.conf
whereas in, say, 2.0.32 it reads:
/* Location of the config file, relative to the Apache root directory */
#define SERVER_CONFIG_FILE conf/httpd.conf
What this means is that I can no longer run the server with an alternate
server root using the -d switch, something I commonly do to test running in
the build tree without having to make install. It's also commonly used by
users of binary builds if they want to install to a directory other than
what the binary builder specified.
I know this stuff's been hacked at recently, I just want to let people know
it's not finished yet :)
configure.in, lines 446-448:
APR_EXPAND_VAR(ap_sysconfdir, $sysconfdir)
AC_DEFINE_UNQUOTED(SERVER_CONFIG_FILE, ${ap_sysconfdir}/${progname}.conf,
[Location of the config file, relative to the Apache root directory])
Apparently $ap_sysconfdir isn't relative. We could opt for stripping
of the ap_prefix part of ap_sysconfdir if it is present.
Sander
[SEGV] mod_deflate
Hi,
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1024 (LWP 3503)]
0x402e24b3 in strncmp (s1=0x0, s2=0x403fc7c8 text/html, n=9) at
../sysdeps/generic/strncmp.c:42
42c1 = (unsigned char) *s1++;
(gdb) bt
#0 0x402e24b3 in strncmp (s1=0x0, s2=0x403fc7c8 text/html, n=9) at
../sysdeps/generic/strncmp.c:42
#1 0x403fbfdb in deflate_out_filter (f=0x8154b18, bb=0x8154ec8) at mod_deflate.c:244
#2 0x8085a11 in ap_pass_brigade (next=0x8154b18, bb=0x8154ec8) at util_filter.c:534
#3 0x808743d in end_output_stream (r=0x81535d0) at protocol.c:941
#4 0x808748b in ap_finalize_request_protocol (r=0x81535d0) at protocol.c:961
#5 0x806b805 in ap_die (type=-2, r=0x81535d0) at http_request.c:110
#6 0x806ba8a in ap_process_request (r=0x81535d0) at http_request.c:273
#7 0x80680ec in ap_process_http_connection (c=0x8141d48) at http_core.c:287
#8 0x8083f49 in ap_run_process_connection (c=0x8141d48) at connection.c:85
#9 0x80841ef in ap_process_connection (c=0x8141d48, csd=0x8141c78) at connection.c:229
#10 0x807a466 in child_main (child_num_arg=0) at prefork.c:671
#11 0x807a51c in make_child (s=0x813fa18, slot=0) at prefork.c:707
#12 0x807a611 in startup_children (number_to_start=5) at prefork.c:784
#13 0x807a923 in ap_mpm_run (_pconf=0x80b2838, plog=0x80ea918, s=0x813fa18) at
prefork.c:1002
#14 0x807f8ad in main (argc=2, argv=0xbdac) at main.c:499
#15 0x4028826a in __libc_start_main (main=0x807f1f0 main, argc=2, ubp_av=0xbdac,
init=0x805e5b4 _init, fini=0x80957f4
_fini,
rtld_fini=0x4000daa4 _dl_fini, stack_end=0xbd9c) at
../sysdeps/generic/libc-start.c:129
(gdb) frame 1
#1 0x403fbfdb in deflate_out_filter (f=0x8154b18, bb=0x8154ec8) at mod_deflate.c:244
244 if (strncmp(r-content_type, text/html, 9)
(gdb) list
239
240 /* Some browsers might have problems with content types
241 * other than text/html, so set gzip-only-text/html
242 * (with browsermatch) for them
243 */
244 if (strncmp(r-content_type, text/html, 9)
245 apr_table_get(r-subprocess_env, gzip-only-text/html)) {
246 return ap_pass_brigade(f-next, bb);
247 }
248
Can someone tell me if r-content_type is always supposed to be pointing to something?
Sander
RE: [SEGV] mod_deflate
From: Sander Striker [mailto:[EMAIL PROTECTED]] Sent: 07 March 2002 13:56 Hi, [...] Can someone tell me if r-content_type is always supposed to be pointing to something? Nevermind, I figured it out by looking at the other modules who test r-content_type. Fix committed. Thanks, Sander
RE: Code questions
From: Ryan Bloom [mailto:[EMAIL PROTECTED]]
Sent: 07 March 2002 19:58
server/config.c:396
return !!(cmd-limited (AP_METHOD_BIT methnum));
^^
Is that a typo or intentional?
It's intentional. This line always sparks a VERY large debate.
Then why didn't any one leave a nice comment behind so that this
doesn't happen again? ;)
The reason for it is that it is the only way to ensure that you have a 1 or
0 result. By negating twice, the first negation always takes the result
to 0 or 1, and second always gives the opposite result.
It's not exactly the only way. There are two more:
1)return (cmd-limited (AP_METHOD_BIT methnum)) ? 1 : 0;
2)return (cmd-limited (AP_METHOD_BIT methnum)) != 0;
And method 3, this entire block:
/*
* A method number either hardcoded into apache or
* added by a module and registered.
*/
if (methnum != M_INVALID) {
return (cmd-limited (AP_METHOD_BIT methnum)) ? 1 : 0;
}
return 0; /* not found */
can be written as:
/*
* A method number either hardcoded into apache or
* added by a module and registered.
*/
return (methnum != M_INVALID
(cmd-limited (AP_METHOD_BIT methnum)));
If noone steps up I'll change it to method 1, since that is
most clear to read for everyone I think.
server/core.c:661
AP_DECLARE(const char *) ap_document_root(request_rec *r) /* Don't use this! */
If we shouldn't use it, why is it still here?
Because people are lazy and most people didn't realize that comment
existed. If nobody is using that function, remove it.
Okay, thanks for the heads up.
server/core.c:691
/* Should probably just get rid of this... the only code that cares is
* part of the core anyway (and in fact, it isn't publicised to other
* modules).
*/
Read the comment.
Check to make sure nobody uses it, and remove it if nobody does.
Ok.
server/listen.c:320
/*free(lr);*/
Can this go? Was there a future purpose to this call,
or was it just old code commented out?
It most likely highlights a memory leak more than anything else. These
structures use to be malloc'ed, and free'd at the correct times. Now we
use palloc to allocate them. I would bet that the free was left so that
somebody would remember to look for the leak.
Consider the line torched.
Ryan
Thanks,
Sander
Copyright year bumping
Hi, Should we bump the copyright year on all the files? Anyone have a script handy? Sander
Torching ap_document_root, WAS: RE: Code questions
From: Sander Striker [mailto:[EMAIL PROTECTED]]
Sent: 07 March 2002 20:48
server/core.c:661
AP_DECLARE(const char *) ap_document_root(request_rec *r) /* Don't use this! */
If we shouldn't use it, why is it still here?
Because people are lazy and most people didn't realize that comment
existed. If nobody is using that function, remove it.
Okay, thanks for the heads up.
modules/ssl/ssl_engine_vars.c:158:result = (char *)ap_document_root(r);
modules/mappers/mod_rewrite.c:1255:if ((ccp = ap_document_root(r)) !=
NULL) {
modules/mappers/mod_rewrite.c:1552:if ((ccp = ap_document_root(r)) !=
NULL) {
modules/mappers/mod_rewrite.c:3492:result = ap_document_root(r);
server/util_script.c:278:apr_table_addn(e, DOCUMENT_ROOT, ap_document_root(r));
/* Apache */
Ofcourse there are always places where such a function is used...
Question is now, are they legit? Should they be changed?
Sander
Torching ap_response_code_string, WAS: RE: Code questions
From: Sander Striker [mailto:[EMAIL PROTECTED]]
Sent: 07 March 2002 20:48
server/core.c:691
/* Should probably just get rid of this... the only code that cares is
* part of the core anyway (and in fact, it isn't publicised to other
* modules).
*/
Read the comment.
Check to make sure nobody uses it, and remove it if nobody does.
Ok.
modules/http/http_protocol.c:1923:if ((custom_response =
ap_response_code_string(r, idx))) {
modules/http/http_request.c:102:char *custom_response = ap_response_code_string(r,
error_index);
Two places where it is used. Suggestions?
Sander
RE: Torching ap_document_root, WAS: RE: Code questions
From: Ryan Bloom [mailto:[EMAIL PROTECTED]]
Sent: 07 March 2002 20:49
server/core.c:661
AP_DECLARE(const char *) ap_document_root(request_rec *r) /*
Don't
use this! */
If we shouldn't use it, why is it still here?
Because people are lazy and most people didn't realize that comment
existed. If nobody is using that function, remove it.
Okay, thanks for the heads up.
modules/ssl/ssl_engine_vars.c:158:result = (char
*)ap_document_root(r);
modules/mappers/mod_rewrite.c:1255:if ((ccp =
ap_document_root(r)) != NULL) {
modules/mappers/mod_rewrite.c:1552:if ((ccp =
ap_document_root(r)) != NULL) {
modules/mappers/mod_rewrite.c:3492:result =
ap_document_root(r);
server/util_script.c:278:apr_table_addn(e, DOCUMENT_ROOT,
ap_document_root(r)); /* Apache */
Ofcourse there are always places where such a function is used...
Question is now, are they legit? Should they be changed?
Having looked at the code now. MO is, yes they are legit. The code
reaches into a core private structure to grab the conf-document_root
variable. I don't want modules doing that themselves.
So the /* don't use this! */ comment should go?
Ryan
Sander
RE: cvs commit: httpd-2.0/include ap_release.h
From: Brian Pane [mailto:[EMAIL PROTECTED]] Sent: 06 March 2002 19:35 Justin Erenkrantz wrote: On Wed, Mar 06, 2002 at 09:48:38AM -0800, Ryan Bloom wrote: Yes, I have tagged 2.0.33. I won't roll the release until Aaron commits the path problem fix. I'll announce when the roll is done. Let me just go on record saying that I don't think we're in a position to release another version. IMHO, we need the API changes in before doing another release. You obviously don't seem to agree with me (per our usual custom). Whatever. I believe this isn't a good time to be tagging. We've just had several major changes and we still haven't thoroughly tested their impact yet. I agree with Justin on both parts: I'd prefer to get the API changes in place (pool allocators and buckets are the ones that I know of) and do some more testing on the latest round of filter rewrites (are they running on daedalus) before relasing 2.0.33. Yes, I have to agree with this too. I didn't see a tag comming at this point. And with the little amount of time we had to test the new code, I don't have the confidence in 2.0.33 as I had in 2.0.32. I'd ideally like to see 2.0.33 become the API freeze release, where we're able to tell 3rd party module maintainers that the APIs are now stable, with 2.0.34 following it as the performance tuning and bug fixes release (and GA candidate). --Brian Sander
[PATCH] Style police work on server/config.c
Hi, Patch is attached to prevent line wrapping/munging. I encountered this: Line 396: return !!(cmd-limited (AP_METHOD_BIT methnum)); ^^ Is that a typo or intentional? Sander config.patch Description: Binary data
RE: [PATCH] Style police work on server/config.c
From: Sander Striker [mailto:[EMAIL PROTECTED]] Sent: 05 March 2002 11:42 Hi, Patch is attached to prevent line wrapping/munging. I encountered this: Line 396: return !!(cmd-limited (AP_METHOD_BIT methnum)); ^^ Is that a typo or intentional? I also forgot to mention that there are some FIXME:s left in the code. Anyone care to take a look? Sander
[PATCH] server/connection.c detab
Hi, More formatting/style/readability stuff. Patch attached yadda yadda yadda. Sander connection.patch Description: Binary data
[PATCH] server/error_bucket.c detab
Hi, More detab. Patch attached Sander error_bucket.patch Description: Binary data
[PATCH] server/gen_test_char.c detab
Hi, Yet more detab. Patch attached. Sander gen_test_char.patch Description: Binary data
[PATCH] server/listen.c detab
Hi, More detab, etc. Can this go? Was there a future purpose to this call, or was it old code commented out? Line 320: /*free(lr);*/ Patch attached. Sander listen.patch Description: Binary data
[PATCH] server/log.c detab
Hi, More detab, etc. Patch attached. Sander log.patch Description: Binary data
RE: [PATCH] server/core.c detab
The mail with the attachement bounced because it was 100k. If anyone wants it, please holler. It wasn't all simple find-replace, so better not wait until conflicts arise ;) Sander -Original Message- From: Sander Striker [mailto:[EMAIL PROTECTED]] Sent: 05 March 2002 14:00 To: [EMAIL PROTECTED] Subject: [PATCH] server/core.c detab Hi, More detab and other style stuff. This time for server/core.c. I encountered this in there: Line 661: AP_DECLARE(const char *) ap_document_root(request_rec *r) /* Don't use this! */ If we shouldn't use it, why is it still here? And: Line 691: /* Should probably just get rid of this... the only code that cares is * part of the core anyway (and in fact, it isn't publicised to other * modules). */ Patch attached yadda yadda yadda. Sander
[PATCH] Pass one of mod_status style cleanup
Hi, Still more issues left in the form of lots of long lines. Maybe the html output could use some cleanup aswell. Anyhow, this is the first pass at getting mod_status.c more into the style we all know. Patch attached to prevent line wrapping/munging. Sander mod_status.patch Description: Binary data
RE: [BUG] Location /dir doesn't work as expected when there is a /dir in the DocumentRoot
Hi, I've reproduced the problem reported by Vlad Skvortsov locally and I'll describe it once more together with some feedback I got. From the config: DocumentRoot /htdocs Location /repos DAV svn SVNPath /htdocs/repos /Location In the /htdocs path, there is a directory called repos. The directory seems to win over the Location (badness!). The OPTIONS request to http://example.com/repos will indicate a non-dav-enabled resource. On irc OtherBill stated that mod_dav chose not to override map_to_storage*, but Location should still win over Dir. His guess was that mod_davs OPTIONS intercept could be borked. Sander *) I tried to find out how to implement a map_to_storage hook for mod_dav, but failed to grasp how it worked (couldn't find any docs or good examples).
RE: [PATCH] fix alignment on shared memory
[mailto:[EMAIL PROTECTED]]On Behalf Of Jeff Trawick Sent: 01 March 2002 20:08 Cliff Woolley [EMAIL PROTECTED] writes: On Fri, 1 Mar 2002, Aaron Bannert wrote: Can we put that alignment macro in a common place in APR, since it is not useful to apps and internals? but where :) apr_lib.h? (duck) apr_general.h would be my first guess. Sander
RE: daemontools/foreground support in 1.3.*
From: Justin Erenkrantz [mailto:[EMAIL PROTECTED]] Sent: 28 February 2002 10:30 To: Michael Handler Cc: [EMAIL PROTECTED] Subject: Re: daemontools/foreground support in 1.3.* On Thu, Feb 28, 2002 at 04:00:34AM -0500, Michael Handler wrote: I completely understand the desire to not to introduce substantial changes into 1.3.* at this point, as well as encouraging people to test the stability and correctness of 2.0. However: I have added it to our contrib section for now: http://www.apache.org/dist/httpd/contrib/patches/1.3/daemontools.patch This should give you an official place to download it. I have also added a note in 1.3's STATUS. And, BTW, I would appreciate it if people would stop spreading FUD like 2.0 arguably won't be [ready] for some time. It's in production usage right now. *You* may not judge it production quality - most of us do. The only thing stopping a GA is some API changes (Cliff!) *caugh* and *caugh* me *caugh* I need to change the pools api one more time. The allocators can be abstracted out and that best be done before GA IMO. To be changed:apr_pool_create_ex To be introduced: apr_allocator_create, apr_allocator_destroy, apr_allocator_alloc, apr_allocator_free, ... and small bugfixes (fast_redirect, worker robustness). We also need to wait to give the 3rd-party providers one last chance to get their modules working (php). -- justin Sander
[PATCH] Fix configure.in for autoconf 2.52 WAS: RE: cvs commit: httpd-2.0 configure.in acinclude.m4
Hi, Reposted with a clearer subject line. This fixes the problem for me (although I didn't apply the patch, but moved AC_PREFIX_DEFAULT manually). Sander * [EMAIL PROTECTED] ([EMAIL PROTECTED]) wrote : aaron 02/02/27 17:38:11 Modified:.configure.in acinclude.m4 Log: Fix a typo in the default cgidir variable. Set a couple more defaults if they haven't already been set, just as a precaution. Sadly this breaks under autoconf 2.52 - AC_PREFIX_DEFAULT needs to be run after the apr m4 files are included, not before. (buildconf chokes like this otherwise: Creating configure ... rebuilding srclib/pcre/configure rebuilding include/ap_config_auto.h.in configure.in:8: error: m4_defn: undefined macro: _m4_divert_diversion acgeneral.m4:616: AC_PREFIX_DEFAULT is expanded from... configure.in:8: the top level autoconf: tracing failed rebuilding configure configure.in:8: error: m4_defn: undefined macro: _m4_divert_diversion acgeneral.m4:616: AC_PREFIX_DEFAULT is expanded from... configure.in:8: the top level Sander Striker spotted this one. Patch attached to move it back, and add the warning back in. Cheers, -Thom Index: configure.in === RCS file: /home/cvspublic/httpd-2.0/configure.in,v retrieving revision 1.204 diff -u -u -r1.204 configure.in --- configure.in 28 Feb 2002 02:56:15 - 1.204 +++ configure.in 28 Feb 2002 13:18:32 - @@ -5,7 +5,6 @@ dnl AC_PREREQ(2.13) -AC_PREFIX_DEFAULT(/usr/local/apache2) AC_INIT(ABOUT_APACHE) AC_CONFIG_HEADER(include/ap_config_auto.h) @@ -18,6 +17,11 @@ sinclude(srclib/apr/build/apr_network.m4) sinclude(srclib/apr/build/apr_threads.m4) sinclude(acinclude.m4) + +dnl XXX we can't just use AC_PREFIX_DEFAULT because that isn't subbed in +dnl by configure until it is too late. Is that how it should be or not? +dnl Something seems broken here. +AC_PREFIX_DEFAULT(/usr/local/apache2) dnl Get the layout here, so we can pass the required variables to apr dnl APACHE_ENABLE_LAYOUT
RE: [BUG] Location /dir doesn't work as expected when there is a /dir in the DocumentRoot
From: Joshua Slive [mailto:[EMAIL PROTECTED]] Sent: 28 February 2002 16:54 To: [EMAIL PROTECTED] Subject: RE: [BUG] Location /dir doesn't work as expected when there is a /dir in the DocumentRoot From: Sander Striker [mailto:[EMAIL PROTECTED]] Today Vlad Skvortsov [EMAIL PROTECTED] stumbled over a, IMO, showstopper. He was trying out subversion and had a config where (simplified): DocumentRoot /home/svn Location /home/svn/repos DAV svn SVNPath /home/svn/repos /Location In the /home/svn path, there is a directory repos. The directory seems to win over the Location (badness!). I may be missing something obvious, but let me make sure that you aren't: That location section only matches http://example.com/home/svn/repos/ so it seems perfectly resonable that if you request http://example.com/repos/ it will fall through to the actual directory. Ick, my bad: The location _is_: Location /repos I think what you want is Location /repos or Directory /home/svn/repos Joshua. I'll try to reproduce it myself in an hour. Sander
[PATCH] mod_deflate, style cleanup
Hi, This is a style cleanup only patch. It removes trailing spaces. Corrects some identation. Minor style nits, no code changes. Patch is attached, because the mailer can't cope with trailing spaces very well, so it seems. Sander deflate.patch Description: Binary data
RE: [PATCH] mod_deflate
From: Zvi Har'El [mailto:[EMAIL PROTECTED]] Sent: 16 February 2002 08:37 On Fri, 15 Feb 2002 09:44:19 -0800, Ian Holsman wrote about Re: [PATCH] mod_deflate: I'm still not very happy about compressing EVERYTHING and excluding certain browsers as you would have to exclude IE Netscape. so this is a -1 for this patch. in order to change this checks need to be there with a directive to ignore them (default:off) IMHO, deflating everything is a waste of the computer resources. HTML files really compress well. But most of the image files currently supported, e.g., PNG, GIF, JPEG are already compressed, and deflating them will really do nothing -- just spend your CPU. I think that compressing text/html for browsers who send Accept-Encoding: gzip is the right approach. A possible enhancement is to have a directive (DeflateAddMimeType) which will enable deflating more mime types, e.g., text/plain, but these are really rare! Another type which is worth compressing is application/postscript, since many viewers (I am not an expert which - at least those decendents of GhostScript) are capable of viewing gzipped postscript files. The problem with that is that this is not a function of the browser, which cannot handle such files, but a function of the viewer, so the required Accept-Encoding: gzip doesn't prove anything about the ability of the external viewer! To summerize, I suggest to deflate only types which can be handled by the browser itself, and which are not already compressed, which amounts to text/html or more generally text/* (text/css for instance). It is not a question of browsers. Subversion is also an HTTP client (for details on how subversion works: http://subversion.tigris.org), actually a DAV client, but that's not the point. We want to compress a lot more than what the common browser can handle. And from some preliminary testing, we've seen a big win with compression (traffic size reduced to 10% of the original. Disclaimer: this number is not acurate). I refuse to rule this out by saying: the common browser doesn't support this. I'd much rather decide on browser and content type, which could be done through configuration. Your DeflateAddMimeType could be an option, but I'd rather see AddOutputFilter take a content type argument: AddOutputFilter filter (content type | extention). Anyhow, it is more about configuration now than to the codes default. Best, Zvi. Sander
RE: [PATCH] mod_deflate
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]]On Behalf Of Jeff Trawick Ian Holsman [EMAIL PROTECTED] writes: In my mod_deflate module (for Apache 1.3.x) I'd enabled by default text/html only. You can add or remove another type with DeflateTypes directive. Here are some recomendations: This is EXACTLY what we should be doing IMHO. if a person wants to run a SVN server they can add a directive to deflate everything, otherwise it should just do text/html out of the box. we want to make it easy for the 'general' person to use. +1 application/x-javascript NN4 does not understand it compressed. text/css the same. text/plain Macromedia FlashPlayer 4.x-5.x does not understand it compressed when get it with loadVariables() function via browser. text/xml Macromedia FlashPlayer 5.x does not understand it compressed when get it with XML.load() function via browser. application/x-shockwave-flash FlashPlayer plugin for NN4 for Windows does not understand it compressed. Although plugin for LinuxNN4 work correctly. text/rtf MSIE 4.x-6.x understand correctly them application/msword when compressed. NN and Opera does not. application/vnd.ms-excel application/vnd.ms-powerpoint Igor Sysoev Wow! Obviously the code/default config need to be extremely conservative! Yes. But browsers change (evolve to better things we hope), so config has my preference. Hardcoding in default rules is badness IMHO. But maybe that's just me. Sander
[PATCH] mod_deflate
Hi,
This patch gets us working with subversion ;)
This removes some checks that go against the spec. If
we have broken browsers out there, we can BrowserMatch
for them. But by default we want to get everything
through this filter.
Justin identified a problem with the Content-Length
header being preserved.
Ofcourse mod_deflate still needs some work, but this
is a (small) step in the right direction.
Sander
Index: modules/experimental/mod_deflate.c
===
RCS file: /home/cvspublic/httpd-2.0/modules/experimental/mod_deflate.c,v
retrieving revision 1.2
diff -u -r1.2 mod_deflate.c
--- modules/experimental/mod_deflate.c 8 Dec 2001 13:05:56 - 1.2
+++ modules/experimental/mod_deflate.c 15 Feb 2002 09:30:51 -
@@ -235,16 +235,6 @@
return ap_pass_brigade(f-next, bb);
}
-/* GETs only (for the moment) */
-if (r-method_number != M_GET) {
-return ap_pass_brigade(f-next, bb);
-}
-
-/* only compress text/html files */
-if (strncmp(r-content_type, text/html, 9)) {
-return ap_pass_brigade(f-next, bb);
-}
-
/* some browsers might have problems, so set no-gzip
* (with browsermatch) for them */
if (apr_table_get(r-subprocess_env, no-gzip)) {
@@ -297,6 +287,7 @@
apr_table_setn(r-headers_out, Content-Encoding, gzip);
apr_table_setn(r-headers_out, Vary, Accept-Encoding);
+apr_table_unset(r-headers_out, Content-Length);
}
APR_BRIGADE_FOREACH(e, bb) {
mod_deflate.patch
Description: Binary data
RE: [PATCH] mod_deflate
Sander Striker wrote:
@@ -297,6 +287,7 @@
apr_table_setn(r-headers_out, Content-Encoding, gzip);
apr_table_setn(r-headers_out, Vary, Accept-Encoding);
+apr_table_unset(r-headers_out, Content-Length);
}
APR_BRIGADE_FOREACH(e, bb) {
Do you mean that till now, mod_deflate preserved the original CL, of
the file before being deflated ???
yes
Wow...
Maybe I am confused, or miss something, but how has mod_deflate
succeeded to work till now?
Or are gzip-supporting-browsers smart enough to ignore the CL of
compressed responses?
Apparently so*. It is in modules/experimental for a reason, although
I'm sure we'd all love to get it out. It will require some work though.
*) Or we were just lucky. Subversion only failed a long way into
the process of checking out a source tree. It had pulled down
quite an amount of files prior to failing (due to the server
closing the connection I might add).
Eli Marmor
Sander
Deflate question for our spec specialists
Hi, Can we compress requests aswell? Is that part of the deflate spec? Can someone give me some details on how this would work (I could ofcourse read the spec...)? Thanks, Sander
RE: [PATCH] mod_deflate
From: Ian Holsman [mailto:[EMAIL PROTECTED]]
Sent: 15 February 2002 18:44
Hi,
I'm still not very happy about compressing EVERYTHING and excluding
certain browsers as you would have to exclude IE Netscape.
so this is a -1 for this patch.
in order to change this checks need to be there with a directive to
ignore them (default:off)
Hopefully this patch resolves your veto. Without that patch, deflate
would be useless for subversion (and any other client that can
handle compressed responses other than text/html).
Sander
Index: modules/experimental/mod_deflate.c
===
RCS file: /home/cvspublic/httpd-2.0/modules/experimental/mod_deflate.c,v
retrieving revision 1.3
diff -u -r1.3 mod_deflate.c
--- modules/experimental/mod_deflate.c 15 Feb 2002 16:33:33 - 1.3
+++ modules/experimental/mod_deflate.c 15 Feb 2002 17:50:56 -
@@ -235,6 +235,15 @@
return ap_pass_brigade(f-next, bb);
}
+/* Some browsers might have problems with content types
+ * other than text/html, so set gzip-only-text/html
+ * (with browsermatch) for them
+ */
+if (strncmp(r-content_type, text/html, 9)
+ apr_table_get(r-subprocess_env, gzip-only-text/html)) {
+return ap_pass_brigade(f-next, bb);
+}
+
/* some browsers might have problems, so set no-gzip
* (with browsermatch) for them */
if (apr_table_get(r-subprocess_env, no-gzip)) {
RE: prefork segfaults under load
From: Adam Sussman [mailto:[EMAIL PROTECTED]] Sent: 12 February 2002 03:36 I agree that disabling threads is covering up a problem, but I suspect that the problem is in glibc and not in Apache. Some rather lame debug suggestions: 1) make sure you have the latest glibc... maybe the problem got fixed Upgrading to the latest glibc does not seem to help. 2) make sure you aren't running out of memory We're not. 3) grab the sources for the level of glibc you have and try to get some idea of why __pthread_reset_main_thread() might segfault There seem to be a number of ways that this could dump core and so far we aren't having any luck tracking this down. The best we can come up with is that there is some stack corruption happening somewhere. The latest CVS snapshot seems even more unstable, by the way. Any other ideas we can chase after? Are you using APR HEAD? We fixed a bug in pools, which was basically writing too much in too little space. -adam Sander
Windows is able to listen on the same port with multiple servers ?! WAS: RE: cvs commit: httpd-2.0/server listen.c
Hi, Stupid question time: how does windows hand of something to the right server? I thought that the port/ip combination was supposed to be unique. [EMAIL PROTECTED] writes: rbb 02/02/04 22:16:04 Modified:.STATUS server listen.c Log: This change keeps the server from allowing multiple instances to bind to the same port. Previously, this was necessary, because the Windows MPM was binding to the socket in both the parent and child. Today's code passes the attached socket to the child from the parent, so we don't need to re-attach in the child. what the *%#... you broke everybody else (and perhaps Windows in some scenarios) -1 This change also prevents a single instance (i.e., normal scenario) to bind to the desired port when an old connection is in TIME_WAIT or some other TCP state. I can't see any SO_REUSEADDR being done on Unix now and meanwhile I have a continual stream of regression test failures saying: (98)Address already in use: make_sock: could not bind to address 0.0.0.0:8099 no listening sockets available, shutting down Very nice. Great for restarts... Not. Sander
RE: Members of dev@httpd.apache.org
From: Gunter Knauf [mailto:[EMAIL PROTECTED]] Hi Gunter, if you don't get feedback, ask why and if nobody answers or somebody says the change isn't appropriate make sure you understand why (but sometimes there is no good reason... collectively we're not perfect communicators I'm afraid) *grin* --- HIER BEGINNT DIE WEITERGELEITETE NACHRICHT -- Von: [EMAIL PROTECTED] (Guenter Knauf) Datum: 14.01.2002, 13:26:35 Betreff: mark Documentroot in httpd.conf Hi, some others and I would like to have the DocumentRoot marked: - DocumentRoot @@ServerRoot@@/htdocs + DocumentRoot @@DocumentRoot@@ and: - Directory @@ServerRoot@@/htdocs + Directory @@DocumentRoot@@ so that we can replace the complete path to the document root by script. can we change this? --- ENDE DER WEITERGELEITETEN NACHRICHT-- according to your suggestions I ask now: why I get no feedback? Not even a 'NO - not accepted'; simply nothing... This is mainly because everyone is also extremely busy. And, on top of that, everyone works in their own area of interest. When a patch doesn't get feedback within a reasonable amount of time, simply repost it, stating that it is a repost meant as a simple reminder that it is there. To give you some feedback: personally I think there might be a point in your suggestion to moving to @@DocumentRoot@@. Guenter. Sander
RE: mod_cgid pipe leak
-Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]]On Behalf Of Jeff Trawick Sent: 24 January 2002 13:44 To: [EMAIL PROTECTED] Cc: Aaron Bannert; Bill Stoddard; [EMAIL PROTECTED] Subject: Re: mod_cgid pipe leak Jeff Trawick [EMAIL PROTECTED] writes: I'll try to follow up with an strace of the cgid daemon across a CGI... gotta rebuild (I always seem to have the wrong config for what I want to do next... maybe some cron jobs need to keep a couple of build flavors current :) ) This is current HEAD on Linux (with some apr_pools.c code ifdef-ed out so it woud compile... whats up with that?). Damn, damn, damn. Please tell me what you #ifdef-ed out, so I can fix it. Sander
RE: mod_cgid pipe leak
-Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]]On Behalf Of Jeff Trawick Sent: 24 January 2002 13:44 To: [EMAIL PROTECTED] Cc: Aaron Bannert; Bill Stoddard; [EMAIL PROTECTED] Subject: Re: mod_cgid pipe leak Jeff Trawick [EMAIL PROTECTED] writes: I'll try to follow up with an strace of the cgid daemon across a CGI... gotta rebuild (I always seem to have the wrong config for what I want to do next... maybe some cron jobs need to keep a couple of build flavors current :) ) This is current HEAD on Linux (with some apr_pools.c code ifdef-ed out so it woud compile... whats up with that?). Damn, damn, damn. Please tell me what you #ifdef-ed out, so I can fix it. Ok, I think I see the problem. Sorry about that. Sander
RE: Current CVS on Win32
From: Sebastian Bergmann [mailto:[EMAIL PROTECTED]] Sent: 24 January 2002 14:21 To: [EMAIL PROTECTED] Subject: Current CVS on Win32 ... does not build: apr_pools.c c:\home\apache\httpd-2.0\srclib\apr\memory\unix\apr_pools.c(73): fatal error C1083: include file not found: 'unistd.h': No such file or directory Ahum, my bad, so it seems. Where is getpid() located in win32? There seems to be something amiss here, since it tries to build a *NIX specific part of the library, I suppose. And, hang on for the next commit to the pools code in about 5 minutes. Sorry for the inconvenience. Sander
RE: PHP Apache2Filter
Ian Holsman wrote: no changes to apxs for a long time. Never mind, it turned out to be a probelm solved by 'make clean'. Sorry for bothering with this one. Now I'm getting sapi_apache2.c: In function `php_input_filter': sapi_apache2.c:252: incompatible type for argument 4 of `ap_get_brigade' sapi_apache2.c:252: too few arguments to function `ap_get_brigade' sapi_apache2.c:259: incompatible type for argument 4 of `ap_get_brigade' sapi_apache2.c:259: too few arguments to function `ap_get_brigade' sapi_apache2.c: In function `php_register_hook': sapi_apache2.c:474: warning: passing arg 2 of `ap_register_input_filter' from incompatible pointer type This looks like an API change. What should be done here? It is. Look at the recent commits by Justin. It should be fairly apparent what needs changing. Otherwise, I'm sure Justin can be helpfull here. Sander
APR_POOL_DEBUG is functioning again WAS: RE: Don't use APR_POOL_DEBUG
Hi, As of a recent commit the apr pools debug code is functioning again. I tested it with httpd-2.0 HEAD and it works (both tried worker and prefork to see if everything goes to the log ok). Some hints (that probably change when we move to a numbered system for selecting the debug mode): If you want a debug build: $ CPPFLAGS=-DAPR_POOL_DEBUG ./configure ... $ make If you want a verbose debug build: $ CPPFLAGS=-DAPR_POOL_DEBUG_VERBOSE ./configure ... $ make When switching between verbose and regular, only recompile apr_pools.c and relink: $ rm srclib/apr/memory/unix/apr_pools.o $ CPPFLAGS=-DAPR_POOL_DEBUG_VERBOSE make A tip on starting the server (when verbose debug is on): # cd where httpd is installed # bin/httpd 2logs/error_log This will put all of the debug output in the log, even the start of the server (at which time the log isn't opened, but pools are being created). Sander
RE: HEAD dumps core with APR_POOL_DEBUG
Hi, I've fixed the integrity check now. If you are still seeing aborts, then we have a serious lifetime problem*. If this is the case, try running with APR_POOL_DEBUG_VERBOSE. When you hit an abort, it will print: INVALID address You can then scan back through the output for the address to see where it was DESTROYed (and CREATEd). Sander *) Or a bug in the debug code...
RE: HEAD dumps core with APR_POOL_DEBUG
Jeff, I'll look into this straight away.
Seems that the addition of simple stats are breaking things.
Will take a few minutes. Expect a commit soon.
Sander
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED]]On Behalf Of Jeff Trawick
Sent: 14 January 2002 12:03
To: [EMAIL PROTECTED]
Subject: Re: HEAD dumps core with APR_POOL_DEBUG
Sander Striker [EMAIL PROTECTED] writes:
Hi,
I've fixed the integrity check now.
If you are still seeing aborts, then we have a serious lifetime
problem*. If this is the case, try running with APR_POOL_DEBUG_VERBOSE.
When you hit an abort, it will print:
INVALID address
You can then scan back through the output for the address to
see where it was DESTROYed (and CREATEd).
Sander
*) Or a bug in the debug code...
Okay, cvs HEAD, fresh build starting with extraclean, just plain
APR_POOL_DEBUG:
#0 0xff34c70c in pool_is_child_of (pool=0x1b3fb0, parent=0x6174652c,
mutex=0x0) at apr_pools.c:900
900 if (parent-mutex parent-mutex != mutex)
(gdb) where
#0 0xff34c70c in pool_is_child_of (pool=0x1b3fb0, parent=0x6174652c,
mutex=0x0) at apr_pools.c:900
#1 0xff34c794 in pool_is_child_of (pool=0x1b3fb0, parent=0x1d2ca0,
mutex=0x0) at apr_pools.c:906
#2 0xff34c794 in pool_is_child_of (pool=0x1b3fb0, parent=0x14a528,
mutex=0x117aa8) at apr_pools.c:906
#3 0xff34c794 in pool_is_child_of (pool=0x1b3fb0, parent=0x117a60,
mutex=0x0) at apr_pools.c:906
#4 0xff34c8dc in check_integrity (pool=0x1b3fb0) at apr_pools.c:950
#5 0xff34c910 in apr_palloc (pool=0x1b3fb0, size=19) at
apr_pools.c:977
#6 0xff32bb10 in apr_pstrndup (a=0x1b3fb0, s=0xfb1077f5
index.html.tw.Big5\n, n=18)
at apr_strings.c:96
#7 0xb60b8 in ap_get_token (p=0x1b3fb0, accept_line=0xfb10778c,
accept_white=0) at util.c:1380
#8 0x876e8 in read_type_map (map=0xfb109888, neg=0x1ece78,
rr=0x167750) at mod_negotiation.c:931
#9 0x8bc1c in handle_map_file (r=0x167750) at mod_negotiation.c:2781
#10 0xa6250 in ap_run_handler (r=0x167750) at config.c:185
#11 0xa6ec8 in ap_invoke_handler (r=0x167750) at config.c:359
#12 0x533c0 in ap_process_request (r=0x167750) at http_request.c:292
#13 0x4b1c0 in ap_process_http_connection (c=0x1b46b0) at
http_core.c:280
#14 0xbab14 in ap_run_process_connection (c=0x1b46b0) at
connection.c:84
#15 0xbb0cc in ap_process_connection (c=0x1b46b0) at connection.c:230
#16 0xa0b70 in process_socket (p=0x177110, sock=0x177290,
my_child_num=0, my_thread_num=182)
at worker.c:562
#17 0xa14b8 in worker_thread (thd=0x15ef40, dummy=0x15e230) at
worker.c:777
#18 0xff340c48 in dummy_worker (opaque=0x15ef40) at thread.c:122
(gdb) p *parent
Cannot access memory at address 0x6174652c
(gdb)
Changing it to APR_POOL_DEBUG_VERBOSE make clean make make install:
(oh, I had to apply this patch first in order to use the
normally-commented-out define of APR_POOL_DEBUG_VERBOSE)
Index: memory/unix/apr_pools.c
===
RCS file: /home/cvs/apr/memory/unix/apr_pools.c,v
retrieving revision 1.133
diff -u -r1.133 apr_pools.c
--- memory/unix/apr_pools.c 14 Jan 2002 09:31:57 - 1.133
+++ memory/unix/apr_pools.c 14 Jan 2002 10:52:27 -
@@ -849,7 +849,7 @@
apr_pools_initialized = 1;
-#if APR_POOL_DEBUG_VERBOSE
+#ifdef APR_POOL_DEBUG_VERBOSE
apr_file_open_stderr(file_stderr, global_pool);
if (file_stderr) {
apr_file_printf(file_stderr,
@@ -876,7 +876,7 @@
apr_pool_destroy(global_pool); /* This will also destroy the mutex */
global_pool = NULL;
-#if APR_POOL_DEBUG_VERBOSE
+#ifdef APR_POOL_DEBUG_VERBOSE
file_stderr = NULL;
#endif
}
Here is the coredump I get (almost immediately after starting
pounding):
#0 0xff34d994 in pool_num_bytes (pool=0x14a3c8) at apr_pools.c:1381
1381for (index = 0; index node-index; index++) {
(gdb) where
#0 0xff34d994 in pool_num_bytes (pool=0x14a3c8) at apr_pools.c:1381
#1 0xff34da44 in apr_pool_num_bytes (pool=0x14a3c8, recurse=1) at
apr_pools.c:1395
#2 0xff34da8c in apr_pool_num_bytes (pool=0x14a3c8, recurse=1) at
apr_pools.c:1401
#3 0xff34da8c in apr_pool_num_bytes (pool=0x190968, recurse=1) at
apr_pools.c:1401
#4 0xff34cf7c in apr_pool_destroy_debug (pool=0x160d08,
file_line=0xff34ed98 thread.c:182)
at apr_pools.c:1091
#5 0xff340f2c in apr_thread_exit (thd=0x163f28, retval=0) at
thread.c:182
#6 0xa15bc in worker_thread (thd=0x163f28, dummy=0x1bb120) at
worker.c:788
#7 0xff340cc0 in dummy_worker (opaque=0x163f28) at thread.c:122
(gdb) p *node
Cannot access memory at address 0x208
(gdb) p *pool
$1 = {parent = 0x0, child = 0x0, sibling = 0x2077a8, ref = 0x19096c,
cleanups = 0x1cb810,
subprocesses = 0x0, abort_fn = 0, user_data = 0x0, tag = 0xf7240
protocol.c:575, nodes = 0x0,
file_line = 0xf7240 protocol.c:575, stat_alloc = 0,
stat_total_alloc = 373
RE: HEAD dumps core with APR_POOL_DEBUG
Okay, cvs HEAD, fresh build starting with extraclean, just plain
APR_POOL_DEBUG:
#0 0xff34c70c in pool_is_child_of (pool=0x1b3fb0, parent=0x6174652c,
mutex=0x0) at apr_pools.c:900
^
So, the parent of 'parent' is a pool which doesn't need a lock. It was
created by an apr_pool_create_ex call with APR_POOL_NEWALLOCATOR passed in.
It's a direct descendant of the global_pool, which means NULL is passed
in as the parent argument.
I'm wondering how the pool hierarchy got currupted.
900 if (parent-mutex parent-mutex != mutex)
(gdb) where
#0 0xff34c70c in pool_is_child_of (pool=0x1b3fb0, parent=0x6174652c,
mutex=0x0) at apr_pools.c:900
#1 0xff34c794 in pool_is_child_of (pool=0x1b3fb0, parent=0x1d2ca0,
mutex=0x0) at apr_pools.c:906
#2 0xff34c794 in pool_is_child_of (pool=0x1b3fb0, parent=0x14a528,
mutex=0x117aa8) at apr_pools.c:906
#3 0xff34c794 in pool_is_child_of (pool=0x1b3fb0, parent=0x117a60,
mutex=0x0) at apr_pools.c:906
#4 0xff34c8dc in check_integrity (pool=0x1b3fb0) at apr_pools.c:950
#5 0xff34c910 in apr_palloc (pool=0x1b3fb0, size=19) at
apr_pools.c:977
#6 0xff32bb10 in apr_pstrndup (a=0x1b3fb0, s=0xfb1077f5
index.html.tw.Big5\n, n=18)
at apr_strings.c:96
[...]
(gdb) p *parent
Cannot access memory at address 0x6174652c
(gdb)
Which proves the corruption. The pool hierarchy is traversed, top down,
while locking if needed (like told by the app), checking if the pool
passed into the function is in the hierarchy. If not, it aborts. If
it segfaults, the hierarchy must be corrupted.
Am my missing something here?
[...]
Here is the coredump I get (almost immediately after starting
pounding):
#0 0xff34d994 in pool_num_bytes (pool=0x14a3c8) at apr_pools.c:1381
1381for (index = 0; index node-index; index++) {
(gdb) where
#0 0xff34d994 in pool_num_bytes (pool=0x14a3c8) at apr_pools.c:1381
#1 0xff34da44 in apr_pool_num_bytes (pool=0x14a3c8, recurse=1) at
apr_pools.c:1395
#2 0xff34da8c in apr_pool_num_bytes (pool=0x14a3c8, recurse=1) at
apr_pools.c:1401
#3 0xff34da8c in apr_pool_num_bytes (pool=0x190968, recurse=1) at
apr_pools.c:1401
This could be the same locking problem as with the integrity check,
which I will fix. What I am wondering about is if pool=0x190968
is the global pool or the pool being destroyed. Do you have that
information?
#4 0xff34cf7c in apr_pool_destroy_debug (pool=0x160d08,
file_line=0xff34ed98 thread.c:182)
at apr_pools.c:1091
#5 0xff340f2c in apr_thread_exit (thd=0x163f28, retval=0) at
thread.c:182
#6 0xa15bc in worker_thread (thd=0x163f28, dummy=0x1bb120) at
worker.c:788
#7 0xff340cc0 in dummy_worker (opaque=0x163f28) at thread.c:122
(gdb) p *node
Cannot access memory at address 0x208
(gdb) p *pool
$1 = {parent = 0x0, child = 0x0, sibling = 0x2077a8, ref = 0x19096c,
cleanups = 0x1cb810,
subprocesses = 0x0, abort_fn = 0, user_data = 0x0, tag = 0xf7240
protocol.c:575, nodes = 0x0,
file_line = 0xf7240 protocol.c:575, stat_alloc = 0,
stat_total_alloc = 373, stat_clear = 1,
mutex = 0x0}
(gdb)
The only POOL DEBUG messages I get in the error_log are CLEAR (just
two, at initialization), CREATE, and DESTROY. I'd expect some other
flavor if the pool code noticed a problem, right?
Could you send me the first bit of the log (up to ~1MB) in private?
Thanks,
Jeff
Sander
Don't use APR_POOL_DEBUG
Hi, Please don't use APR_POOL_DEBUG today. There is a logic error* in there which I will fix tomorrow. Thanks for your patience. Sander *) Rendering it totally useless since it also aborts on correct usage.
RE: pool problems with HEAD??
Jeff, I'm going over the pools code right now. I'm wondering if something slipped in when I checked in the APR_POOL_DEBUG code (*grunt*) Could you do me a favor and try with one rev earlier to see if it has the same problem? In the mean time I'll try and track it down. Sander -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]]On Behalf Of Jeff Trawick Sent: 10 January 2002 17:29 To: [EMAIL PROTECTED]; [EMAIL PROTECTED] Subject: pool problems with HEAD?? CVS HEAD, Solaris, worker MPM, one process with many threads IfModule worker.c ServerLimit 1 ThreadLimit 500 StartServers 1 MaxClients 500 MinSpareThreads 25 MaxSpareThreads 75 ThreadsPerChild 500 MaxRequestsPerChild 0 /IfModule I had a client with many concurrent connections continually doing the same request over and over. After starting another client to get another 200 or so concurrent connections, I quickly got a segfault in mod_log_config because supposedly apr_palloc returned some bogus addresses (0 and 0x38). #0 0x3fd84 in config_log_transaction (r=Cannot access memory at address 0xed5018f4 ) at mod_log_config.c:823 823 strs[i] = process_item(r, orig, items[i]); (gdb) p strs $1 = (char **) 0x0 (gdb) p format-nelts $2 = 14 (gdb) p strl $4 = (int *) 0x38 (gdb) p i $5 = 0 (gdb) (gdb) where #0 0x3fd84 in config_log_transaction (r=0x6a62c8, cls=0x1add38, default_format=0x15cde8) at mod_log_config.c:823 #1 0x3ffd4 in multi_log_transaction (r=0x6a2618) at mod_log_config.c:881 #2 0xc3f98 in ap_run_log_transaction (r=0x6a2618) at protocol.c:1297 #3 0x53228 in ap_process_request (r=0x6a2618) at http_request.c:315 #4 0x4b150 in ap_process_http_connection (c=0x6a0758) at http_core.c:280 #5 0xba72c in ap_run_process_connection (c=0x6a0758) at connection.c:84 #6 0xbace4 in ap_process_connection (c=0x6a0758) at connection.c:230 #7 0xa08bc in process_socket (p=0x6a0638, sock=0x6a0670, my_child_num=0, my_thread_num=187) at worker.c:562 #8 0xa11f4 in worker_thread (thd=0x2bef88, dummy=0x1163b0) at worker.c:777 #9 0xff340990 in ?? () The funkiness about r changing in the traceback is (I suspect) because in multi_log_transaction we walked through a list of r's using the parameter as the variable. Both r's look healthy. Running a level of code from about a day before 2.0.30 on AIX this morning, running about the same worker MPM config, I got a segfault down in apr_palloc() because some heap structures were messed up. Has anybody pounded on an Apache configured like that (one process, many threads)? -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
RE: pool problems with HEAD??
[...]
(2) revert plog/pconf patch and see what happens
I applied this patch:
Index: server/core.c
===
RCS file: /cvs/apache/httpd-2.0/server/core.c,v
retrieving revision 1.128
diff -u -r1.128 core.c
--- core.c 2002/01/08 17:07:19 1.128
+++ core.c 2002/01/10 17:54:08
@@ -3361,7 +3361,8 @@
static int core_open_logs(apr_pool_t *pconf, apr_pool_t *plog,
apr_pool_t *ptemp, server_rec *s)
{
-ap_open_logs(s, plog);
+/* XXX */
+ap_open_logs(s, pconf);
return OK;
}
Here is a backtrace from the coredump I got:
#0 0xff34c3d0 in apr_palloc (pool=0x7b98e0, size=24) at apr_pools.c:430
430 endp = active-first_avail + size;
(gdb) where
#0 0xff34c3d0 in apr_palloc (pool=0x7b98e0, size=24) at apr_pools.c:430
#1 0xff330ff8 in apr_table_make (p=0x7b98e0, nelts=5) at apr_tables.c:343
#2 0xcdb18 in core_create_conn (ptrans=0x7b98e0, server=0x1b2ea0, csd=0x7b7910,
conn_id=433, sbh=0x7b9880) at core.c:3474
#3 0xba9d0 in ap_run_create_connection (p=0x7b98e0, server=0x1b2ea0, csd=0x7b7910,
conn_id=433, sbh=0x7b9880) at connection.c:86
#4 0xa089c in process_socket (p=0x7b98e0, sock=0x7b7910, my_child_num=0,
my_thread_num=433) at worker.c:560
#5 0xa11f4 in worker_thread (thd=0x469330, dummy=0x4e2d68) at worker.c:777
#6 0xff340990 in dummy_worker (opaque=0x469330) at thread.c:122
(gdb) p *active
Cannot access memory at address 0x0
(gdb)
Hmmm, this is weird. This means pool-active == NULL. AFAIK this can only
happen when the pool is not really valid anymore (it got destroyed somewhere).
The reason I suspect this, is because that is the time the pool struct is
recycled (the node containing the struct is put on the freelist). So I think
this is a lifetime problem.
Guess what? We really need lifetime checking in debug mode.
Alternatively you could try running with APR_POOL_DEBUG turned on
and a third party tool like efence or purify.
(3) revert pool debug stuff and see what happens
I'll report back separately on this.
--
Jeff Trawick
Thanks,
Sander
RE: pool problems with HEAD??
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]]On Behalf Of Jeff Trawick Sent: 10 January 2002 19:57 To: Ian Holsman Cc: [EMAIL PROTECTED] Subject: Re: pool problems with HEAD?? Ian Holsman [EMAIL PROTECTED] writes: can you run it with efence/purify on ? this should catch the problem. I wish I had purify. I've tried efence on Linux but I'm not sure what is supposed to happen. It is supposed to segfault at the first point where a bad memory pointer is used. IOW, run it in gdb. Which might be a problem with your repro recipe (500 threads/1 process)... Or isn't it? It segfaults all over the place when I bang on it, but I don't get coredumps when threads segfault on my level of kernel :( Maybe I can get efence installed on AIX or Solaris where I know I can get coredumps. Sander
RE: cvs commit: httpd-2.0/modules/loggers mod_log_config.c
-Original Message- From: Brian Pane [mailto:[EMAIL PROTECTED]] Sent: 06 January 2002 21:49 To: [EMAIL PROTECTED] Subject: Re: cvs commit: httpd-2.0/modules/loggers mod_log_config.c Ben Laurie wrote: [EMAIL PROTECTED] wrote: brianp 02/01/06 00:01:34 Modified:modules/loggers mod_log_config.c Log: Bypass a strdup and an 8KB local variable in the common case where the logger is using the default time format Does that really stop the stack space from being allocated? It seems unlikely to me (I haven't checked). You're right. I just checked, and it didn't stop the space from being allocated. I'll move that branch of the code to separate function so that it really works. (The alternative would be to alloc that buffer from a pool, allocating 8KB from a pool would cause its own set of problems, because a typical pool block doesn't have enough free space to handle an alloc that large, so we'd often be allocating a new, odd-sized block.) Not odd-sized. It would be the next multiple of 4k. This is a property of the new pools code. --Brian Sander
[PATCH] Allow DocumentRoot within Location blocks
Hi,
I've tried to tackle this issue in STATUS:
* Allow the DocumentRoot directive within Location scopes? This
allows the beloved (crusty) Alias /foo/ /somepath/foo/ followed
by a Directory /somepath/foo to become simply
Location /foo/ DocumentRoot /somefile/foo (IMHO a bit more legible
and in-your-face.) DocumentRoot unset would be accepted [and would
not permit content to be served, only virtual resources such as
server-info or server-status.
This proposed change would _not_ depricate Alias.
My patch probably is incomplete (and most likely I broke something), but it
is a first cut I'd like some feedback on. For starters, document_root is
moved for core_server_conf to core_dir_conf. core_dir_conf got an extra
field telling you what type of dir conf it is. Wrowe suggested that we might
want a registration API for this, but I decided to go with an enum for
starters. Registration can come in a later iteration.
Sander
Index: include/http_core.h
===
RCS file: /home/cvspublic/httpd-2.0/include/http_core.h,v
retrieving revision 1.58
diff -u -r1.58 http_core.h
--- include/http_core.h 2 Jan 2002 07:56:24 - 1.58
+++ include/http_core.h 5 Jan 2002 14:14:31 -
@@ -390,11 +390,20 @@
} server_signature_e;
typedef struct {
+enum {
+DIR_CONF_DIRECTORY = 0,
+DIR_CONF_LOCATION,
+DIR_CONF_FILES,
+DIR_CONF_SERVER
+} dir_conf_type;
+
/* path of the directory/regex/etc. see also d_is_fnmatch/absolute below */
char *d;
/* the number of slashes in d */
unsigned d_components;
+const char *document_root;
+
/* If (opts OPT_UNSET) then no absolute assignment to options has
* been made.
* invariant: (opts_add opts_remove) == 0
@@ -499,12 +508,6 @@
char *gprof_dir;
#endif
-/* Name translations --- we want the core to be able to do *something*
- * so it's at least a minimally functional web server on its own (and
- * can be tested that way). But let's keep it to the bare minimum:
- */
-const char *ap_document_root;
-
/* Access control */
char *access_name;
Index: server/core.c
===
RCS file: /home/cvspublic/httpd-2.0/server/core.c,v
retrieving revision 1.126
diff -u -r1.126 core.c
--- server/core.c 2 Jan 2002 07:56:25 - 1.126
+++ server/core.c 5 Jan 2002 14:15:28 -
@@ -124,6 +124,8 @@
core_dir_config *conf;
conf = (core_dir_config *)apr_pcalloc(a, sizeof(core_dir_config));
+
+conf-dir_conf_type = DIR_CONF_SERVER;
/* conf-r and conf-d[_*] are initialized by dirsection() or left NULL */
@@ -182,10 +184,15 @@
conf = (core_dir_config *)apr_palloc(a, sizeof(core_dir_config));
memcpy(conf, base, sizeof(core_dir_config));
+conf-dir_conf_type = new-dir_conf_type;
+
conf-d = new-d;
conf-d_is_fnmatch = new-d_is_fnmatch;
conf-d_components = new-d_components;
conf-r = new-r;
+
+if (new-document_root)
+conf-document_root = new-document_root;
if (new-opts OPT_UNSET) {
/* there was no explicit setting of new-opts, so we merge
@@ -343,7 +350,6 @@
conf-gprof_dir = NULL;
#endif
conf-access_name = is_virtual ? NULL : DEFAULT_ACCESS_FNAME;
-conf-ap_document_root = is_virtual ? NULL : DOCUMENT_LOCATION;
conf-sec_dir = apr_array_make(a, 40, sizeof(ap_conf_vector_t *));
conf-sec_url = apr_array_make(a, 40, sizeof(ap_conf_vector_t *));
@@ -361,9 +367,7 @@
if (!conf-access_name) {
conf-access_name = base-access_name;
}
-if (!conf-ap_document_root) {
-conf-ap_document_root = base-ap_document_root;
-}
+
conf-sec_dir = apr_array_append(p, base-sec_dir, virt-sec_dir);
conf-sec_url = apr_array_append(p, base-sec_url, virt-sec_url);
@@ -530,11 +534,11 @@
AP_DECLARE(const char *) ap_document_root(request_rec *r) /* Don't use this! */
{
-core_server_config *conf;
+core_dir_config *conf;
-conf = (core_server_config *)ap_get_module_config(r-server-module_config,
- core_module);
-return conf-ap_document_root;
+conf = (core_dir_config *)ap_get_module_config(r-per_dir_config,
+ core_module);
+return conf-document_root;
}
AP_DECLARE(const apr_array_header_t *) ap_requires(request_rec *r)
@@ -905,28 +909,29 @@
return NULL;
}
+static const char *unset_document_root = ;
+
static const char *set_document_root(cmd_parms *cmd, void *dummy,
const char *arg)
{
-void *sconf = cmd-server-module_config;
-core_server_config *conf = ap_get_module_config(sconf, core_module);
-
+core_dir_config *dconf = dummy;
+
const char *err =
[PATCH] Don't register methods twice
Hi,
It is not uncommon for modules to register the same method.
Therefor it is better to return the already registered
method number, instead of allowing the registration twice.
Sander
PS. I split this out of the mod_dav patch I sent in earlier,
because it affects more than just mod_dav.
Index: modules/http/http_protocol.c
===
RCS file: /home/cvspublic/httpd-2.0/modules/http/http_protocol.c,v
retrieving revision 1.382
diff -u -r1.382 http_protocol.c
--- modules//http/http_protocol.c 2001/12/06 02:57:19 1.382
+++ modules//http/http_protocol.c 2001/12/22 21:28:49
@@ -337,7 +337,7 @@
AP_DECLARE(int) ap_method_register(apr_pool_t *p, const char *methname)
{
-int *newmethnum;
+int *methnum;
if (methods_registry == NULL) {
ap_method_registry_init(p);
@@ -346,7 +346,15 @@
if (methname == NULL) {
return M_INVALID;
}
-
+
+/* Check if the method was previously registered. If it was
+ * return the associated method number.
+ */
+methnum = (int *)apr_hash_get(methods_registry, methname,
+ APR_HASH_KEY_STRING);
+if (methnum != NULL)
+return *methnum;
+
if (cur_method_number METHOD_NUMBER_LAST) {
/* The method registry has run out of dynamically
* assignable method numbers. Log this and return M_INVALID.
@@ -358,11 +366,11 @@
return M_INVALID;
}
-newmethnum = (int*)apr_palloc(p, sizeof(int));
-*newmethnum = cur_method_number++;
-apr_hash_set(methods_registry, methname, APR_HASH_KEY_STRING, newmethnum);
+methnum = (int *)apr_palloc(p, sizeof(int));
+*methnum = cur_method_number++;
+apr_hash_set(methods_registry, methname, APR_HASH_KEY_STRING, methnum);
-return *newmethnum;
+return *methnum;
}
/* Get the method number associated with the given string, assumed to
Bitfields in core_dir_config
Hi,
Why are we using bitfields in core_dir_config?
Do we really care about a few bits?
Not to mention it is inconsistent:
/* Hostname resolution etc */
#define HOSTNAME_LOOKUP_OFF 0
#define HOSTNAME_LOOKUP_ON 1
#define HOSTNAME_LOOKUP_DOUBLE 2
#define HOSTNAME_LOOKUP_UNSET 3
unsigned int hostname_lookups : 4;
Here, we have a bitfield of 4 bits wide. So,
range is 0-15.
signed int do_rfc1413 : 2; /* See if client is advertising a username? */
signed int content_md5 : 2; /* calculate Content-MD5? */
Why two bit wide flag fields (if one would do)?
#define USE_CANONICAL_NAME_OFF (0)
#define USE_CANONICAL_NAME_ON(1)
#define USE_CANONICAL_NAME_DNS (2)
#define USE_CANONICAL_NAME_UNSET (3)
unsigned use_canonical_name : 2;
Here we have a 2 bit wide bitfield, range is 0-3.
What's so different between hostname_lookups and
use_canonical_name?
Personally I would much rather see:
/* Hostname resolution etc */
enum {
HOSTNAME_LOOKUP_OFF = 0,
HOSTNAME_LOOKUP_ON,
HOSTNAME_LOOKUP_DOUBLE,
HOSTNAME_LOOKUP_UNSET
} hostname_lookups;
apr_bool_t do_rfc1413; /* See if client is advertising a username? */
apr_bool_t content_md5; /* calculate Content-MD5? */
enum {
USE_CANONICAL_NAME_OFF = 0,
USE_CANONICAL_NAME_ON,
USE_CANONICAL_NAME_DNS,
USE_CANONICAL_NAME_UNSET
} use_canonical_name;
Ofcourse we would need to introduce apr_bool_t for that one
(apr just screams for a boolean type :). But even if we don't,
is there any reason not to use enums instead of bitfields?
Sander
RE: Benchmark: 29 vs 30
From: David Reid [mailto:[EMAIL PROTECTED]] Sent: 22 December 2001 13:57 The transaction stats were what jumped out at me - 7% increase in failed connections doesn't sound good to me :( But, then maybe I'm reading that wrong? Which is what I saw first too. But when I talked to Ian over irc snippet: [00:22] IanHolsman hey.. did you see the benchmark.. [00:22] IanHolsman could you parse it [00:22] sander Yes, saw it. [00:22] sander No, could not parse. [00:22] IanHolsman aah [00:23] IanHolsman http://webperf.org/a2/caw/29/Current Total HTTP and TCP Errors vs Load 21-Dec-2001 1215.gif is probably the best image [00:24] sander Under a higher load we get more errors with v30? [00:24] IanHolsman if you look on v29 errors start happening around 500 users sessions. with v30 they happen at 700 [00:24] IanHolsman no... under a higher load you get less. [00:24] sander Ah And this is why I thought this morning: There must be more people having trouble parsing these results david Sander
[PATCH] Get mod_dav to dynamically register its methods
This patch gets mod_dav to register its methods dynamically.
Instead of relying on the fact that a method isn't registered
and comparing method strings, it now registers and compares
method numbers.
Sander
Index: modules/dav/main/mod_dav.c
===
RCS file: /home/cvspublic/httpd-2.0/modules/dav/main/mod_dav.c,v
retrieving revision 1.65
diff -u -r1.65 mod_dav.c
--- modules/dav/main/mod_dav.c 2001/11/23 16:35:21 1.65
+++ modules/dav/main/mod_dav.c 2001/12/22 17:51:52
@@ -132,12 +132,51 @@
/* forward-declare for use in configuration lookup */
extern module DAV_DECLARE_DATA dav_module;
+/* DAV methods */
+#define DAV_M_VERSION_CONTROL 0
+#define DAV_M_CHECKOUT 1
+#define DAV_M_UNCHECKOUT 2
+#define DAV_M_CHECKIN 3
+#define DAV_M_UPDATE 4
+#define DAV_M_LABEL5
+#define DAV_M_REPORT 6
+#define DAV_M_MKWORKSPACE 7
+#define DAV_M_MKACTIVITY 8
+#define DAV_M_BASELINE_CONTROL 9
+#define DAV_M_MERGE10
+#define DAV_M_BIND 11
+
+static int dav_methods[12];
+
+static APR_INLINE void dav_register_method(apr_pool_t *p, int method_index,
+ const char *method)
+{
+dav_methods[method_index] = ap_method_number_of(method);
+if (dav_methods[method_index] == M_INVALID)
+dav_methods[method_index] = ap_method_register(p, method);
+}
+
static int dav_init_handler(apr_pool_t *p, apr_pool_t *plog, apr_pool_t *ptemp,
server_rec *s)
{
/* DBG0(dav_init_handler); */
+/* Register DAV methods */
+dav_register_method(p, DAV_M_VERSION_CONTROL, VERSION-CONTROL);
+dav_register_method(p, DAV_M_CHECKOUT, CHECKOUT);
+dav_register_method(p, DAV_M_UNCHECKOUT, UNCHECKOUT);
+dav_register_method(p, DAV_M_CHECKIN, CHECKIN);
+dav_register_method(p, DAV_M_UPDATE, UPDATE);
+dav_register_method(p, DAV_M_LABEL, LABEL);
+dav_register_method(p, DAV_M_REPORT, REPORT);
+dav_register_method(p, DAV_M_MKWORKSPACE, MKWORKSPACE);
+dav_register_method(p, DAV_M_MKACTIVITY, MKACTIVITY);
+dav_register_method(p, DAV_M_BASELINE_CONTROL, BASELINE-CONTROL);
+dav_register_method(p, DAV_M_MERGE, MERGE);
+dav_register_method(p, DAV_M_BIND, BIND);
+
ap_add_version_component(p, DAV/2);
+
return OK;
}
@@ -4478,61 +4517,52 @@
if (r-method_number == M_UNLOCK) {
return dav_method_unlock(r);
}
-
-/*
- * NOTE: When Apache moves creates defines for the add'l DAV methods,
- * then it will no longer use M_INVALID. This code must be
- * updated each time Apache adds method defines.
- */
-if (r-method_number != M_INVALID) {
- return DECLINED;
-}
-if (!strcmp(r-method, VERSION-CONTROL)) {
+if (r-method_number == dav_methods[DAV_M_VERSION_CONTROL]) {
return dav_method_vsn_control(r);
}
-if (!strcmp(r-method, CHECKOUT)) {
+if (r-method_number == dav_methods[DAV_M_CHECKOUT]) {
return dav_method_checkout(r);
}
-if (!strcmp(r-method, UNCHECKOUT)) {
+if (r-method_number == dav_methods[DAV_M_UNCHECKOUT]) {
return dav_method_uncheckout(r);
}
-if (!strcmp(r-method, CHECKIN)) {
+if (r-method_number == dav_methods[DAV_M_CHECKIN]) {
return dav_method_checkin(r);
}
-if (!strcmp(r-method, UPDATE)) {
+if (r-method_number == dav_methods[DAV_M_UPDATE]) {
return dav_method_update(r);
}
-if (!strcmp(r-method, LABEL)) {
+if (r-method_number == dav_methods[DAV_M_LABEL]) {
return dav_method_label(r);
}
-if (!strcmp(r-method, REPORT)) {
+if (r-method_number == dav_methods[DAV_M_REPORT]) {
return dav_method_report(r);
}
-if (!strcmp(r-method, MKWORKSPACE)) {
+if (r-method_number == dav_methods[DAV_M_MKWORKSPACE]) {
return dav_method_make_workspace(r);
}
-if (!strcmp(r-method, MKACTIVITY)) {
+if (r-method_number == dav_methods[DAV_M_MKACTIVITY]) {
return dav_method_make_activity(r);
}
-if (!strcmp(r-method, BASELINE-CONTROL)) {
+if (r-method_number == dav_methods[DAV_M_BASELINE_CONTROL]) {
return dav_method_baseline_control(r);
}
-if (!strcmp(r-method, MERGE)) {
+if (r-method_number == dav_methods[DAV_M_MERGE]) {
return dav_method_merge(r);
}
-if (!strcmp(r-method, BIND)) {
+if (r-method_number == dav_methods[DAV_M_BIND]) {
return dav_method_bind(r);
}
[PATCH] Get mod_dav to dynamically register its methods [2]
Hi,
Like Greg announced previously: here is a new patch.
I changed a variable name in ap_method_register, because
it is more appropiate with the patch in.
Sander
Index: modules/dav/main/mod_dav.c
===
RCS file: /home/cvspublic/httpd-2.0/modules/dav/main/mod_dav.c,v
retrieving revision 1.65
diff -u -r1.65 mod_dav.c
--- modules//dav/main/mod_dav.c 2001/11/23 16:35:21 1.65
+++ modules//dav/main/mod_dav.c 2001/12/22 21:28:35
@@ -132,12 +132,46 @@
/* forward-declare for use in configuration lookup */
extern module DAV_DECLARE_DATA dav_module;
+/* DAV methods */
+enum {
+DAV_M_VERSION_CONTROL = 0,
+DAV_M_CHECKOUT,
+DAV_M_UNCHECKOUT,
+DAV_M_CHECKIN,
+DAV_M_UPDATE,
+DAV_M_LABEL,
+DAV_M_REPORT,
+DAV_M_MKWORKSPACE,
+DAV_M_MKACTIVITY,
+DAV_M_BASELINE_CONTROL,
+DAV_M_MERGE,
+DAV_M_BIND,
+DAV_M_LAST
+};
+
+static int dav_methods[DAV_M_LAST];
+
static int dav_init_handler(apr_pool_t *p, apr_pool_t *plog, apr_pool_t *ptemp,
server_rec *s)
{
/* DBG0(dav_init_handler); */
+/* Register DAV methods */
+dav_methods[DAV_M_VERSION_CONTROL] = ap_method_register(p, VERSION-CONTROL);
+dav_methods[DAV_M_CHECKOUT] = ap_method_register(p, CHECKOUT);
+dav_methods[DAV_M_UNCHECKOUT] = ap_method_register(p, UNCHECKOUT);
+dav_methods[DAV_M_CHECKIN] = ap_method_register(p, CHECKIN);
+dav_methods[DAV_M_UPDATE] = ap_method_register(p, UPDATE);
+dav_methods[DAV_M_LABEL] = ap_method_register(p, LABEL);
+dav_methods[DAV_M_REPORT] = ap_method_register(p, REPORT);
+dav_methods[DAV_M_MKWORKSPACE] = ap_method_register(p, MKWORKSPACE);
+dav_methods[DAV_M_MKACTIVITY] = ap_method_register(p, MKACTIVITY);
+dav_methods[DAV_M_BASELINE_CONTROL] = ap_method_register(p, BASELINE-CONTROL);
+dav_methods[DAV_M_MERGE] = ap_method_register(p, MERGE);
+dav_methods[DAV_M_BIND] = ap_method_register(p, BIND);
+
ap_add_version_component(p, DAV/2);
+
return OK;
}
@@ -4478,61 +4512,52 @@
if (r-method_number == M_UNLOCK) {
return dav_method_unlock(r);
}
-
-/*
- * NOTE: When Apache moves creates defines for the add'l DAV methods,
- * then it will no longer use M_INVALID. This code must be
- * updated each time Apache adds method defines.
- */
-if (r-method_number != M_INVALID) {
- return DECLINED;
-}
-if (!strcmp(r-method, VERSION-CONTROL)) {
+if (r-method_number == dav_methods[DAV_M_VERSION_CONTROL]) {
return dav_method_vsn_control(r);
}
-if (!strcmp(r-method, CHECKOUT)) {
+if (r-method_number == dav_methods[DAV_M_CHECKOUT]) {
return dav_method_checkout(r);
}
-if (!strcmp(r-method, UNCHECKOUT)) {
+if (r-method_number == dav_methods[DAV_M_UNCHECKOUT]) {
return dav_method_uncheckout(r);
}
-if (!strcmp(r-method, CHECKIN)) {
+if (r-method_number == dav_methods[DAV_M_CHECKIN]) {
return dav_method_checkin(r);
}
-if (!strcmp(r-method, UPDATE)) {
+if (r-method_number == dav_methods[DAV_M_UPDATE]) {
return dav_method_update(r);
}
-if (!strcmp(r-method, LABEL)) {
+if (r-method_number == dav_methods[DAV_M_LABEL]) {
return dav_method_label(r);
}
-if (!strcmp(r-method, REPORT)) {
+if (r-method_number == dav_methods[DAV_M_REPORT]) {
return dav_method_report(r);
}
-if (!strcmp(r-method, MKWORKSPACE)) {
+if (r-method_number == dav_methods[DAV_M_MKWORKSPACE]) {
return dav_method_make_workspace(r);
}
-if (!strcmp(r-method, MKACTIVITY)) {
+if (r-method_number == dav_methods[DAV_M_MKACTIVITY]) {
return dav_method_make_activity(r);
}
-if (!strcmp(r-method, BASELINE-CONTROL)) {
+if (r-method_number == dav_methods[DAV_M_BASELINE_CONTROL]) {
return dav_method_baseline_control(r);
}
-if (!strcmp(r-method, MERGE)) {
+if (r-method_number == dav_methods[DAV_M_MERGE]) {
return dav_method_merge(r);
}
-if (!strcmp(r-method, BIND)) {
+if (r-method_number == dav_methods[DAV_M_BIND]) {
return dav_method_bind(r);
}
Index: modules/http/http_protocol.c
===
RCS file: /home/cvspublic/httpd-2.0/modules/http/http_protocol.c,v
retrieving revision 1.382
diff -u -r1.382 http_protocol.c
--- modules//http/http_protocol.c 2001/12/06 02:57:19 1.382
+++ modules//http/http_protocol.c 2001/12/22 21:28:49
@@ -337,7 +337,7 @@
AP_DECLARE(int) ap_method_register(apr_pool_t *p, const char *methname)
{
-int *newmethnum;
+int *methnum;
if (methods_registry == NULL) {
ap_method_registry_init(p);
@@ -346,7 +346,15 @@
if (methname == NULL) {
return M_INVALID;
}
-
+
+/* Check if the method was previously registered. If it was
+
RE: Apache2Filter crashes on Windows
From: Sebastian Bergmann [mailto:[EMAIL PROTECTED]] Sent: 17 December 2001 11:10 Daniel Stone wrote: Please provide a full gdb backtrace of the problem; just the last call on the stack won't help the developers much. How do I generate a backtrace with MSVC? :-/ Look at the call stack: Alt 7. Sander
[PATCH] Make the worker mpm use the new pools code
[sent to the wrong list the first time, sorry] Hi, This patch will make worker use the new features of the pools code. It also adds a convenient line for debugging purposes (resolves to a NOOP if APR_POOL_DEBUG is not defined). Sander Index: server/mpm/worker/worker.c === RCS file: /home/cvspublic/httpd-2.0/server/mpm/worker/worker.c,v retrieving revision 1.46 diff -r1.46 worker.c 657c657,658 apr_pool_create(ptrans, tpool); --- apr_pool_create_ex(ptrans, tpool, NULL, APR_POOL_FNEW_ALLOCATOR); apr_pool_tag(ptrans, transaction);
RE: cvs commit: httpd-2.0/build instdso.sh
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]] Sent: 14 December 2001 13:30 trawick 01/12/14 04:29:37 Modified:buildinstdso.sh Log: take over DSO installation from libtool on all platforms, for both make install and apxs -i since we don't link with Apache DSOs we don't need the .la files since we load Apache DSOs with explicit path information we don't need any other system-specific magic to be performed Revision ChangesPath 1.2 +6 -7 httpd-2.0/build/instdso.sh Index: instdso.sh === RCS file: /home/cvs/httpd-2.0/build/instdso.sh,v retrieving revision 1.1 retrieving revision 1.2 [...] Wouldn't it be better to try and get libtool to behave on AIX (one of the platforms it doesn't work on)? Currently, someone is trying to get subversion to build, link and install on AIX. Subversion has a lot of library inter dependencies. Lets see where that leads. Sander
RE: cvs commit: httpd-2.0/build instdso.sh
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]]On Behalf Of Jeff Trawick Sent: 14 December 2001 15:23 Sander Striker [EMAIL PROTECTED] writes: [...] Wouldn't it be better to try and get libtool to behave on AIX (one of the platforms it doesn't work on)? Currently, someone is trying to get subversion to build, link and install on AIX. Subversion has a lot of library inter dependencies. Lets see where that leads. I don't follow your logic. On *EVERY* platform, libtool --install on an Apache DSO has unintended side-effects. Ah. Wasn't aware of that. On AIX, HP-UX, and Tru64 (and maybe others that we don't know yet), the side-effects are bad enough that our documentation for LoadModule and our logic to create LoadModule statements no longer works if we stick with libtool --install. In that case: I totally agree. Consistency is good. Sander
RE: cvs commit: httpd-2.0 ROADMAP
From: Ryan Bloom [mailto:[EMAIL PROTECTED]] Sent: 07 December 2001 17:22 To: [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED] Subject: Re: cvs commit: httpd-2.0 ROADMAP +* Add a string class that combines a char* with a length + and a reference count. This will help reduce the number + of strlen and strdup operations during request processing. This doesn't belong in Apache, if anything it is an APR class. BTW, this has come up multiple times, and everybody seems to be in favor or if. Ryan In subversion svn_string_t was introduced, because it wasn't in APR. It does (AFAIK) exactly what brian describes. Sander
RE: cvs commit: httpd-2.0 ROADMAP
From: Karl Fogel [mailto:[EMAIL PROTECTED]] Sent: 07 December 2001 19:42 Sander Striker [EMAIL PROTECTED] writes: +* Add a string class that combines a char* with a length + and a reference count. This will help reduce the number + of strlen and strdup operations during request processing. This doesn't belong in Apache, if anything it is an APR class. BTW, this has come up multiple times, and everybody seems to be in favor or if. Ryan In subversion svn_string_t was introduced, because it wasn't in APR. It does (AFAIK) exactly what brian describes. Subversion's `svn_string_t' and `svn_stringbuf_t' don't have reference counts... -Karl True, and having a refcount would mean they would need to come out of some global 'pool'. The pool passed in for creation/duplication of the string merely bumps the refcount and registers a 'string refcount decrease [delete when zero]' cleanup. At least, that is what pops in my mind at first thought. Refcounts and pools are hairy... :) Caching the strlen is a good start though. Sander
RE: Some Benchmark Numbers
-Original Message- From: Ian Holsman [mailto:[EMAIL PROTECTED]] Sent: 26 November 2001 23:44 To: [EMAIL PROTECTED] Subject: Some Benchmark Numbers Here are some benchmark numbers showing the performance of 3 pages. (included in the results) 1. the performance of 2.0 against 1.3 2. the performance of 2.0 beta 28, against the current HEAD as well as some possible patches which have appeared recently on the list. http://webperf.org/a2/v29/ Thanks to AAron Brian for the patches. Summary: 2.0 HEAD is approaching (and in some cases exceeeding) the performance of Apache 1.3, but there still is some work needed to reduce the CPU utilization, and locking of the pools. Noted. I am on it. I have been able to reserve the day after tomorrow for it. Will post the (documented) revised pools code at the end of that day for review (and hacking). ...Ian Sander PS. Thx for all your patience.
RE: [PATCH] Remove the Port directive - affects SSL vhost config
From: MATHIHALLI,MADHUSUDAN (HP-Cupertino,ex1) Sent: 04 October 2001 22:40 The patch just affects SSL configurations also.. The ServerName directive specified within a Virtual Host configuration HAS to be accompanied by the SSL port, or else, the server assumes the port 80 by default :-(.. -Madhu How hard is it to return the port that was connected to? That seems to be a logical solution. Thoughts? Sander
RE: warnings on sol 2.6
From: Justin Erenkrantz [mailto:[EMAIL PROTECTED]]
Sent: 30 September 2001 06:30
On Sun, Sep 30, 2001 at 12:24:08AM -0400, Cliff Woolley wrote:
A few warnings that have shown up on Solaris 2.6 sometime
within the last few weeks:
thread.c: In function `apr_thread_once_init':
thread.c:261: warning: missing braces around initializer
thread.c:261: warning: (near initialization for
`once_init.__pthread_once_pad')
I've seen this on Solaris 8 as well. I was tempted to commit the {}s,
and I know that it compiles fine on Linux 2.4 with the {}. Anyone
else care to comment?
Linux defines PTHREAD_ONCE as 0, but Solaris defines it as a
structure and hence wants the {}. Is there a side effect if we
place too many {}s? I forget if they are fluff or not.
That should work. Doing this:
int i = { 0 };
works just fine, because the initializer list matches the 'number
of fields and the depth'* to initialize. In this case, it should
be ok. Note that it isn't simply fluff, this will give you a warning:
int i = {{ 0 }};
*) This isn't a quote, this is me trying to find words to describe
it.
thread_rwlock.c:224: warning: no previous prototype for
`apr_thread_rwlock_lock'
This should be easy to fix.
The proto simply isn't in the header file, but I can't tell if
this is intentional and apr_thread_rwlock_lock was just a copy
'n paste too many in the c file, or that it should actually be
in the header. Aaron?
I haven't seen this, but I may have missed it.
proxy_ftp.c: In function `ap_proxy_ftp_handler':
proxy_ftp.c:800: warning: subscript has type `char'
I looked at this... I must be missing something, but I swear I don't see
what the problem is. Anyone?
550:char buffer[MAX_STRING_LEN];
555:int i = 0, j, len, rc;
800:for (i = 0; buffer[i] !isdigit(buffer[i]); i++);
Hmmm, this is very weird...
I'm actually compiling proxy for the first time. If I see anything,
I'll comment. -- justin
Sander
RE: proxy - a return.
From: Greg Stein [mailto:[EMAIL PROTECTED]] Sent: 26 September 2001 09:35 On Tue, Sep 25, 2001 at 05:45:40PM -0700, Ryan Bloom wrote: On Tuesday 25 September 2001 04:13 pm, Graham Leggett wrote: ... Right now, what is the best way of returning mod_proxy to the tree? Is it a) checking in the latest copy of proxy, relying on the old httpd-proxy tree for history. b) moving the ,v files across so that history is carried forward into httpd-2.0 tree? B. That history is incredibly important since proxy is basically a complete re-write for 2.0. Of course, Greg is likely to disagree with me (this is one of our long-standing disagreements), so wait for him to respond too, please. Thanks for the consideration, Ryan... Well, an import is a bit different than moving files around in the tree. I tend to advocate cvs add for moving files, rather than mucking with ,v files. Moving or copying ,v files means that files can appear when you check out old copies (by tag if you don't remove them, but a checkout-by-date will always produce spurious files). [ Greg explains ... ] Are we having fun yet? :-) Cheers, -g Well, I don't know for sure, but there might be another option that doesn't involve too much work. In CVSROOT/modules do this: httpd-2.0 httpd-2.0 httpd-2.0/modules/proxy httpd-2.0/modules/proxy -d modules/proxy httpd-proxy/module-2.0 But then again, this might just be a bit to little. Oh well, just a thought, Sander
[PROPOSAL] apr-client
Hi all, I wish to propose a new library: apr-client. It is basically a http client library. I see a direct use for at least three projects: - mod_proxy (which has most of the code in it), - flood (to do more flexible testing, for example with authentication, or even ssl client auth), - subversion (which is currently using neon). The library is going to be based on apr and apr-util, and will have an optional dependency on openssl (through a --with-ssl[=path] switch). Features: - sessions - request building - response parsing - pipelining - authentication support - filters - like ssl, gz, chunk Callbacks will be used to drive events (like responses, or the need for auth information). The library should make heavy use of buckets/brigades, I dare even say the request building is a thin wrapper around those. Requests supported should be all HTTP requests and the DAV extensions. The filters are a nice area for code reuse. The logic for the client is ofcourse reversed, so we can have a chunk filter on the request side and a dechunk on the response side. This is just an example, replace chunk/dechunk with gzip/unzip and you have another. Same goes for ssl. This way the same code gets used in multiple places which results in more extensive testing of this code (which is a nice side effect). You might ask yourself why the world would need yet another http client library. Well, there seem to be only two good ones: curl and neon. Curl is bloated (has ftp, gopher, etc support and is more a general network client library). Neon is good, but LGPL. Also, it doesn't tie in nicely with apr (example is malloc/free usage in it, which requires the user to malloc a chunk of mem, fill it, pass it to neon which then frees it). Well, this mail isn't the extensive description I wanted to give*, but surely enough to get some feedback. Sander *) Am a bit distracted for some reason.
RE: [PATCH] fix cleanups in cleanups
From: Greg Stein [mailto:[EMAIL PROTECTED]] Sent: 21 September 2001 09:35 On Thu, Sep 20, 2001 at 07:54:22PM -0700, Ryan Bloom wrote: On Thursday 20 September 2001 05:48 pm, Greg Stein wrote: Calling pop_cleanup() on every iteration is a bit much. Consider the following patch: Why is it a bit much? I just took a quick look at it, it is an if, and three assignments. Heh. It is a bit much when you consider you consider a thread in October 1999 discussing using NULL rather than ap_null_cleanup: http://www.humanfactor.com/cgi-bin/cgi-delegate/apache-ML/nh/1999/ Oct/0189.html In that case, an if was considered too much :-) I always wondered about that. It seems so silly to have to pass in apr_pool_cleanup_null allmost all the time as second argument. It isn't very clear for first time users and certainly not intuitive. Oh well, Manoj pointed this out aswell in that thread and still we have apr_pool_cleanup_null... Sander
RE: Optimizing dir_merge() AND RE: [BUG] mod_ssl broken
From: William A. Rowe, Jr. [mailto:[EMAIL PROTECTED]] Sent: 21 September 2001 08:38 From: Sander Striker [EMAIL PROTECTED] Sent: Thursday, September 13, 2001 7:30 AM Ok, now I have a repro recipe that doesn't require mod_dav and mod_dav_svn. The last commit should have fixed the problem (and does with your mod_ssl example.) Could you go back and check mod_dav with mod_dav_svn to assure I've licked it? Bill I just tried it. Works like a charm. Thanks Bill. Now I'll go back hunting for more segfaults :) [I saw another one the other day, but am not sure it is related or not, will investigate] Sander
[BUG] mod_ssl
Hi, Sorry to bring this up, but I tripped over a segfault in mod_ssl while trying to add client authentication to subversion. I can't reproduce this with openssl s_client, which makes the issue harder. There probably is a bug somewhere in svn or neon (or my usage of that), but that doesn't really matter, segfaults should never happen. I'll try to come up with a simple repro recipe, but right now, there isn't one without installing subversion and doing mods to that. Here's the stack trace: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 1024 (LWP 30066)] 0x400cb318 in SSL_ctrl () from /opt/ssl/lib/libssl.so.0.9.6 (gdb) bt #0 0x400cb318 in SSL_ctrl () from /opt/ssl/lib/libssl.so.0.9.6 #1 0x80de62c in __DTOR_END__ () at eval.c:88 #2 0x80af76c in ap_pass_brigade (next=0x81a4fd4, bb=0x815a204) at util_filter.c:276 #3 0x807f236 in ap_http_header_filter (f=0x815584c, b=0x8159f24) at http_protocol.c:1322 #4 0x80af76c in ap_pass_brigade (next=0x815584c, bb=0x8159f24) at util_filter.c:276 #5 0x80b18d1 in ap_content_length_filter (f=0x8155824, b=0x8159f24) at protocol.c:976 #6 0x80af76c in ap_pass_brigade (next=0x8155824, bb=0x8159f24) at util_filter.c:276 #7 0x80811a7 in ap_byterange_filter (f=0x81557fc, bb=0x8159f24) at http_protocol.c:2523 #8 0x80af76c in ap_pass_brigade (next=0x81557fc, bb=0x8159f24) at util_filter.c:276 #9 0x806ac51 in send_parsed_content (bb=0xb948, r=0x81540ac, f=0x815577c) at mod_include.c:2934 #10 0x806b174 in includes_filter (f=0x815577c, b=0x8159f24) at mod_include.c:3094 #11 0x80af76c in ap_pass_brigade (next=0x815577c, bb=0x8159f24) at util_filter.c:276 #12 0x80b1169 in end_output_stream (r=0x81540ac) at protocol.c:729 #13 0x80b11db in ap_finalize_request_protocol (r=0x814fe64) at protocol.c:749 #14 0x808059b in ap_send_error_response (r=0x814fe64, recursive_error=400) at http_protocol.c:2033 #15 0x80817b6 in ap_die (type=400, r=0x81540ac) at http_request.c:227 #16 0x8081d1f in ap_internal_redirect (new_uri=0x814ce14 /error/HTTP_FORBIDDEN.html.var, r=0x814fe64) at http_request.c:446 #17 0x8081770 in ap_die (type=403, r=0x814fe64) at http_request.c:212 #18 0x80818ba in ap_process_request (r=0x814fe64) at http_request.c:297 #19 0x807d19a in ap_process_http_connection (c=0x814df24) at http_core.c:287 #20 0x80ada5b in ap_run_process_connection (c=0x814df24) at connection.c:82 #21 0x80adc30 in ap_process_connection (c=0x814df24) at connection.c:219 #22 0x80a2512 in child_main (child_num_arg=0) at prefork.c:830 #23 0x80a266a in make_child (s=0x80e9f44, slot=0) at prefork.c:917 #24 0x80a26ec in startup_children (number_to_start=1) at prefork.c:940 #25 0x80a2ae5 in ap_mpm_run (_pconf=0x80e88ec, plog=0x8120aac, s=0x80e9f44) at prefork.c:1156 #26 0x80a8493 in main (argc=1, argv=0xbdac) at main.c:431 #27 0x401f126a in __libc_start_main (main=0x80a7f08 main, argc=1, ubp_av=0xbdac, init=0x8062f94 _init, fini=0x80bfb24 _fini, rtld_fini=0x4000daa4 _dl_fini, stack_end=0xbd9c) at ../sysdeps/generic/libc-start.c:129 Maybe a seasoned mod_ssl developer can see something obvious in here... If someone is interested in doing a debug session, please contact me, so I can setup an account on my box or send you full details on how to reproduce. Oh, some details. If I switch of client authentication (ie, no SSLVerifyClient require) all works well. Error log: [Fri Sep 21 11:36:34 2001] [notice] Apache/2.0.26-dev (Unix) mod_ssl/3.0a0 OpenSSL/0.9.6b DAV/2 SVN/M3 configured -- resuming normal operations [Fri Sep 21 11:36:34 2001] [info] Server built: Sep 21 2001 08:58:09 [Fri Sep 21 11:36:52 2001] [error] mod_ssl: Re-negotiation handshake failed: Not accepted by client!? [Fri Sep 21 11:36:52 2001] [error] mod_ssl: SSL handshake failed (server striker.xs4all.nl:443, client 192.168.0.1) (OpenSSL library error follows) [Fri Sep 21 11:36:52 2001] [error] OpenSSL: error:140940F5:SSL routines:SSL3_READ_BYTES:unexpected record [Fri Sep 21 11:36:52 2001] [error] mod_ssl: Re-negotiation handshake failed: Not accepted by client!? [Fri Sep 21 11:36:52 2001] [error] mod_ssl: SSL handshake failed (server striker.xs4all.nl:443, client 192.168.0.1) (OpenSSL library error follows) [Fri Sep 21 11:36:52 2001] [error] OpenSSL: error:140940F5:SSL routines:SSL3_READ_BYTES:unexpected record [Fri Sep 21 11:36:52 2001] [error] mod_ssl: Re-negotiation handshake failed: Not accepted by client!? [Fri Sep 21 11:36:53 2001] [notice] child pid 32521 exit signal Segmentation fault (11) [Fri Sep 21 11:36:53 2001] [notice] child pid 32519 exit signal Segmentation fault (11) Sander
RE: Fix broken cleanups model
From: William A. Rowe, Jr. [mailto:[EMAIL PROTECTED]] Sent: 21 September 2001 16:42 - Original Message - From: Ben Hyde [EMAIL PROTECTED] To: [EMAIL PROTECTED] Sent: Friday, September 21, 2001 9:12 AM I also think it's a long standing mistake that the subpools aren't unwound via the same cleanup stack as everything else but have their one one off to the side; that's a royal pain when you want to allocate a node into it's own subpool while doing things like the above. Agreed. Here are my thoughts. We introduce an apr_cleanup_t. This is a _single_ cleanup holder for mutiple, extendable types of cleanups (at this moment, pool cleanup and cleanup for exec.) We tried to change that in SMS. You might want to take a look at that (concept wise). Sander
RE: UUIDs in 2.0
From: dean gaudet [mailto:[EMAIL PROTECTED]] Sent: 14 September 2001 12:45 On Fri, 14 Sep 2001, Sander Striker wrote: Isn't there an apr_get_current_time() that is thread safe? apr_time_now()? that's not the point. i'm guessing you haven't read the code i'm referring to. -dean Did now :) Ok, it seems like most of the reference implementation ideas have been duplicated. The get_current_time function is indeed not thread safe... Hmmm, wrapping those statics in mutexes is a major slowdown... Ok, dean, suggestions? :) Sander
RE: Optimizing dir_merge() AND RE: [BUG] mod_ssl broken
Hi, I can reproduce the problem easily now with openssl s_client. If anyone is interested to hunt this bug down (I am personally not familiar enough with the location_walk code to find it (without spending more time on it than I have at the moment)), I can give an account on my box to observe the problem first hand. Once again, it is not in the optimization code, I reverted to request.c 1.47 (with patch 1.49 applied). If someone has time, please help hunt this down so I can focus on working on the ssl side of subversion ;) Sander
RE: Optimizing dir_merge() AND RE: [BUG] mod_ssl broken
From: William A. Rowe, Jr. [mailto:[EMAIL PROTECTED]]
My location_walk optimization (which suffers a potential bug, per
our svn friends) takes an entirely different tact, which renders
that whole idea DOA.
Ok, to rule out the possibility it is in the optimization code
I reverted to request.c 1.47. The bug remains:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1024 (LWP 21805)]
0x806dd1d in ssl_config_perdir_merge (p=0x814e20c, basev=0x0,
addv=0x8192e2c) at ssl_engine_config.c:269
269 cfgMergeArray(aRequirement);
(gdb) bt
#0 0x806dd1d in ssl_config_perdir_merge (p=0x814e20c, basev=0x0,
addv=0x8192e2c) at ssl_engine_config.c:269
#1 0x80a2c15 in ap_merge_per_dir_configs (p=0x814e20c, base=0x8192f14,
new_conf=0x8192c34) at config.c:262
#2 0x80b7284 in ap_location_walk (r=0x814e23c) at request.c:1169
#3 0x80b6511 in ap_process_request_internal (r=0x814e23c) at request.c:154
#4 0x8080ba0 in ap_process_request (r=0x814e23c) at http_request.c:284
#5 0x807c4fa in ap_process_http_connection (c=0x814c2fc) at http_core.c:287
#6 0x80ac6db in ap_run_process_connection (c=0x814c2fc) at connection.c:82
#7 0x80ac8b0 in ap_process_connection (c=0x814c2fc) at connection.c:219
#8 0x80a13b2 in child_main (child_num_arg=1) at prefork.c:829
#9 0x80a150a in make_child (s=0x80e831c, slot=1) at prefork.c:916
#10 0x80a174f in perform_idle_server_maintenance (p=0x80e6cc4) at
prefork.c:1057
#11 0x80a1b2f in ap_mpm_run (_pconf=0x80e6cc4, plog=0x811ee84, s=0x80e831c)
at prefork.c:1235
#12 0x80a7323 in main (argc=1, argv=0xbd9c) at main.c:431
#13 0x401f026a in __libc_start_main (main=0x80a6d98 main, argc=1,
ubp_av=0xbd9c, init=0x8062460 _init, fini=0x80be7c4 _fini,
rtld_fini=0x4000daa4 _dl_fini, stack_end=0xbd8c) at
../sysdeps/generic/libc-start.c:129
Somewhere between ap_merge_per_dir_configs and ssl_config_perdir_merge
base is reset to NULL. Will investigate further.
For completeness, the relevant part of my config:
VirtualHost _default_:443
SSLEngine on
SSLCACertificatePath /var/openssl/ca/private
SSLCACertificateFile /var/openssl/ca/private/cacert.pem
SSLCertificateFile /var/openssl/ca/certs/striker.xs4all.nl-cert.pem
SSLCertificateKeyFile /var/openssl/ca/certs/striker.xs4all.nl-key.pem
DocumentRoot /opt/httpd/htdocs
ServerName striker.xs4all.nl
Location /svn
SSLRequireSSL
#SSLVerifyClient require
#SSLVerifyDepth 1
DAV svn
SVNPath /home/svn
/Location
/VirtualHost
Location /svn
DAV svn
SVNPath /home/svn
/Location
[trying again]
Ah, the first loop through ap_merge_per_dir_configs seems to
be ok.
Second time through ap_merge_per_dir_configs:
Breakpoint 4, ap_merge_per_dir_configs (p=0x814e20c, base=0x8192f14,
new_conf=0x8192c34) at config.c:262
262 conf_vector[i] = (*df) (p, base_vector[i],
new_vector[i]);
(gdb) p *modp
$6 = {version = 20010825, minor_version = 0, module_index = 7, name =
0x80c0caf mod_ssl.c, dynamic_load_handle = 0x0, next = 0x80da140,
magic = 1095774768, rewrite_args = 0, create_dir_config = 0x806dc6c
ssl_config_perdir_create, merge_dir_config = 0x806dce0
ssl_config_perdir_merge,
create_server_config = 0x806d9c4 ssl_config_server_create,
merge_server_config = 0x806dab4 ssl_config_server_merge, cmds = 0x80da180,
register_hooks = 0x806d760 ssl_register_hooks}
(gdb) s
0x806dce3 in ssl_config_perdir_merge (p=0x814e20c, basev=0x0,
addv=0x8192e2c) at ssl_engine_config.c:263
263 {
Now, why is base_vector[i] == NULL now?
It isn't wrowes caching code, because that is not in here (request.c 1.47).
Thoughts anyone?
Sander
RE: mod_ssl broken
From: William A. Rowe, Jr. [mailto:[EMAIL PROTECTED]]
From: Ben Laurie [EMAIL PROTECTED]
Sent: Sunday, September 09, 2001 3:50 PM
Sander Striker wrote:
[...]
The problem is that basev == NULL, which causes
apr_array_append to barf.
I'll be looking into this next week (or someone working on
mod_ssl will).
I've looked into this slightly, and the underlying problem is that
ap_merge_per_dir_configs finds a previous cached merge (for /?) and
tries to use it but the entry for mod_ssl is NULL. The URL, btw, is
/svn.
This is very, very odd. Would someone please post the svn
create/merge dir config
code, just in case it's clobbering a sibling? The last silly
guess, perhaps we have
a subrequest where the prev-pool has been destroyed (sounds
unlikely to me.)
#define INHERIT_VALUE(parent, child, field) \
((child)-field ? (child)-field : (parent)-field)
[...]
static void *dav_svn_create_dir_config(apr_pool_t *p, char *dir)
{
/* NOTE: dir==NULL creates the default per-dir config */
return apr_pcalloc(p, sizeof(dav_svn_dir_conf));
}
static void *dav_svn_merge_dir_config(apr_pool_t *p,
void *base, void *overrides)
{
dav_svn_dir_conf *parent = base;
dav_svn_dir_conf *child = overrides;
dav_svn_dir_conf *newconf;
newconf = apr_pcalloc(p, sizeof(*newconf));
newconf-fs_path = INHERIT_VALUE(parent, child, fs_path);
return newconf;
}
Looks ok to me...
There are no conditional create/merge results in mod_ssl,
whatsoever. Ergo, it should always be given a base and addv, and return
a new conf.
I got stuck at that point, coz it really isn't at all clear where the
entry was expected to be set in the first place. BTW, all but the first
entry in the cached merge were also NULL. I don't have time before
bedtime to figure this out, I'm afraid.
It sounds as if I've failed to build/record/retrieve the
cache-walked result properly. I'll hack at this a bit tommorow. Hint,
server/request.c:1085 is the start of this block, and we should have a
stack of walk_walked_t structures for every merge performed, recorded in
the (cache_walk_t*)cache-walked apr_array.
Bill
RE: Authentication and Authorization
From: sterling [mailto:[EMAIL PROTECTED]] Hi - IMHO, there is no apache dependency that requires auth and authz to be in the same module usually, it is just logical that the application handle both phases - but that is on the module writer. Yes, but the choice of doing group lookups using a file/db/whatever is tied in with the authz phase. I'd like to be able to reuse the check if a user (belonging to a certain group) is authorized to access a certain url. On Fri, 7 Sep 2001, Sander Striker wrote: Hi, I've been going through the modules/aaa directory and found that modules there seem to implement both authentication and authorization. IMO this should be split. Auth and authz are completely different things and it would be nice to have different modules to do authentication in a different way, but still utilize the same authorization method. To accomplish this, an extra field would be needed in request_req (and that's probably not going to happen): request_req-groups, which holds a string with all the groups the authenticated user belongs to. welp, authorization does NOT imply groups... it could require all sorts of information to authorize. Authentication is simply 'who is this person' and authorization is 'are they authorized to receive the requested location'. Yes, but when looking at the aaa modules, all of them seem to authenticate the user in check_user_id (which is good). In auth_checker, the group is looked up and then authz is checked. The last part is highly duplicated (I don't care too much about that). What I would like to see is that a _common_ thing as group lookups be seperated out of the authz part. Ie, let there be a hook get_user_groups(?), which is invoked when ap_get_user_groups is called. Hence, the authorization phase (read auth_checker) is responsible for reading the requirements and trying to validate them (given usually a valid-user in the r-user field). If that means check if they are part of a group, so be it. If it means check if they have red hair - that works too. Doh! :) Group lookup is NOT part of authentication IMHO. Many authz solutions have nothing to do with 'groups'. Although many do not, it is common (you only have to look at the aaa modules). Or, there could be a new hook which is used to lookup the groups a user belongs to, or, if a user belongs to a certain group. This hook will be called whenever the framework equivalent of this function is called. Thoughts? Sander /me hides from the 'core stabilizers' that probably are going to hate me for bringing this up. I don't think this should be skipped because of 'stabilization'. I think it should be skipped because it is not the right architecture. I think I agree with you when it comes to adding fields to request_req, but to introduce a new hook should be ok IMO. But, this won't be touched anyhow very quickly, looking at the ~'hold off until 2.1' reactions. sterling Sander
RE: mod_ssl broken
[dropped dev@subversion] From: William A. Rowe, Jr. [mailto:[EMAIL PROTECTED]] Sent: 10 September 2001 14:37 From: Sander Striker [EMAIL PROTECTED] Sent: Monday, September 10, 2001 3:24 AM I've looked into this slightly, and the underlying problem is that ap_merge_per_dir_configs finds a previous cached merge (for /?) and tries to use it but the entry for mod_ssl is NULL. The URL, btw, is /svn. This is very, very odd. Would someone please post the svn create/merge dir config code, just in case it's clobbering a sibling? #define INHERIT_VALUE(parent, child, field) \ ((child)-field ? (child)-field : (parent)-field) Looks ok to me... Looks OK to me too... Who wants to toss together a patch to pull all the helper code from mod_ssl, change it from eastern to western European, and drop it back in the right place (AP_ decorated) such as ap_config.h? If _we_ need these sort of helpers... #define cfgMerge(el,unset) new-el = (add-el == (unset)) ? base-el : add-el #define cfgMergeArray(el) new-el = apr_array_append(p, add-el, base-el) #define cfgMergeTable(el) new-el = apr_table_overlay(p, add-el, base-el) #define cfgMergeCtx(el) new-el = apr_table_overlay(p, add-el, base-el) #define cfgMergeString(el) cfgMerge(el, NULL) #define cfgMergeBool(el)cfgMerge(el, UNSET) #define cfgMergeInt(el) cfgMerge(el, UNSET) then apparently (as your example shows) the rest of the world needs these as well. I don't mind putting a patch together that does this (and to use it in all core modules). I would appreciate suggestions for the final names though (naming isn't my strong side). AP_CFG_MERGE AP_CFG_MERGE_ARRAY ... ? Bill Sander
RE: merge macros (was: Re: mod_ssl broken)
From: Greg Stein [mailto:[EMAIL PROTECTED]] Sent: 10 September 2001 22:08 On Mon, Sep 10, 2001 at 09:23:59AM -0500, William A. Rowe, Jr. wrote: From: Ryan Bloom [EMAIL PROTECTED] Sent: Monday, September 10, 2001 8:39 AM I don't mind putting a patch together that does this (and to use it in all core modules). I would appreciate suggestions for the final names though (naming isn't my strong side). AP_CFG_MERGE AP_CFG_MERGE_ARRAY Don't do that. That is just Ralf's coding style. It is not proof that we need this in every single core module. HUH? The point is that even subversion uses such helpers for clarity. It would help all authors to offer these, consistently. We've (collectively) proven that config merge errors are simple to author, and difficult to debug. And the SVN merge macros came from mod_dav. The only reason mod_dav_fs doesn't have a similar macro is that it only has a single item to merge, so I just spelled it out. So you could say that I'm hitting 3 for 3 with using that INHERIT_VALUE macro, and would appreciate a core version of it. AP_CFG_MERGE_* located in http_config.h would make some sense to me. In the mod_ssl version, there was the unset param; I would suggest the simplest for assumes NULL. A second one would take an unset param. Cheers, -g Ryan, was that a -1, or just a 'don't invest time in it, I don't think it's worth it'? Consistency wise I would find it usefull to put a patch together. But if I know beforehand that it is not going in, I'd rather defer and use my energy elsewhere. Sander
Authentication and Authorization
Hi, I've been going through the modules/aaa directory and found that modules there seem to implement both authentication and authorization. IMO this should be split. Auth and authz are completely different things and it would be nice to have different modules to do authentication in a different way, but still utilize the same authorization method. To accomplish this, an extra field would be needed in request_req (and that's probably not going to happen): request_req-groups, which holds a string with all the groups the authenticated user belongs to. Or, there could be a new hook which is used to lookup the groups a user belongs to, or, if a user belongs to a certain group. This hook will be called whenever the framework equivalent of this function is called. Thoughts? Sander /me hides from the 'core stabilizers' that probably are going to hate me for bringing this up.
RE: Authentication and Authorization
Sander Striker wrote: IMO this should be split. Auth and authz are completely different things and it would be nice to have different modules to do authentication in a different way, but still utilize the same authorization method. I'm not sure if splitting them will accomplish this though. From the LDAP auth stuff, the authentication phase and the authorisation phase are separate, but share common configuration parameters (LDAP bind info, for example), so splitting them wouldn't make much sense. In all the modules the phases are seperate, because they all hook check_user_id and check_user_access. There is no way however to determine the group a user is in from check_user_id in a non module specific way. I would like _that_ to be possible, since now, the authz part (check_user_access) is doing stuff auth should do: checking for group membership. Also - there isn't a clear line over what constitutes an authentication token - again, the LDAP authenticator converts a provided username into a DN, which the authorisation phase uses to apply to the require directives. If you have to mix up the different modules, you would need to make sure they are all talking the same language (so to speak). Yes, but I don't see that as a problem. Right now, the same is true for the FakeBasicAuth feature of mod_ssl which provides a one line DN as the username. Regards, Graham Sander
RE: Authentication and Authorization
[replying to my own msg] Sander Striker wrote: IMO this should be split. Auth and authz are completely different things and it would be nice to have different modules to do authentication in a different way, but still utilize the same authorization method. I'm not sure if splitting them will accomplish this though. From the LDAP auth stuff, the authentication phase and the authorisation phase are separate, but share common configuration parameters (LDAP bind info, for example), so splitting them wouldn't make much sense. In all the modules the phases are seperate, because they all hook check_user_id and check_user_access. There is no way however to determine the group a user is in from check_user_id in a non module ^^ this should ofcourse be 'access'. specific way. I would like _that_ to be possible, since now, the authz part (check_user_access) is doing stuff auth should do: checking for group membership. Also - there isn't a clear line over what constitutes an authentication token - again, the LDAP authenticator converts a provided username into a DN, which the authorisation phase uses to apply to the require directives. If you have to mix up the different modules, you would need to make sure they are all talking the same language (so to speak). Yes, but I don't see that as a problem. Right now, the same is true for the FakeBasicAuth feature of mod_ssl which provides a one line DN as the username. Regards, Graham Sander
mod_include.c WAS: RE: remaining CPU bottlenecks in 2.0
* find_start_sequence() is the main scanning function within mod_include. There's some research in progress to try to speed this up significantly. Based on the patches you submitted (and my quasi-errant formatting patch), I had to read most of the code in mod_include, so I'm more familiar with mod_include now. I do think there are some obvious ways to optimize find_start_sequence. I wonder if we could apply a KMP-string matching algorithm here. I dunno. I'll take a look at it though. Something bugs me about the restarts. I bet that we spend even more time in find_start_sequence when a HTML file has lots of comments. =-) I suggested to Ian yesterday night that I'd look into a Boyer-Moore matching algorithm. I'll work on that too. Sander
RE: [PATCH] Potential replacement for find_start_sequence...
I'm not totally sure I'm sold on this approach being better. But, I'm not sure that it is any worse either. Don't have time to benchmark this right now. I'm going to throw it to the wolves and see what you think. Me neither. Rabin-Karp introduces a lot of * and %. I'll try Boyer-Moore with precalced tables for '!--#' and '---'. [...] Sander
RE: [PATCH] Add mod_gz to httpd-2.0
Hi, From what I have seen on the list I am on the +1 side of adding mod_gz(ip) to the distribution. Ofcourse, my vote doesn't count since I don't have httpd commit. I find the following arguments convincing (summarized): - The gzip content encoding is part of the HTTP spec. - Most clients support gzip transfer coding. - It is a real solution to the problem of network bandwidth being the limiting factor on many heavily-loaded web servers and on thin-piped clients. - It makes the compression transparent to the admin of the site and allows for dynamically generated content (which can grow quite large) to be compressed aswell. I haven't seen anything that held on the negative side yet. Sander
RE: Review of possible replacement for pools
Why are we spending time trying to optimize pools when we haven't eliminated the malloc/frees in the bucket brigade calls? The miniscule performance improvements you -might- get optimizing pools will be completely obscured by the overhead of the malloc/frees. Bill Because it also eliminates the locking in the threaded mpm. And, I wanted to see if I could squeeze some extra performance out of it and make it a bit more readable. I don't know if I succeeded in the latter. Or the performance for that matter :) Btw, why couldn't we use pools in brigades again? Sander
RE: Review of possible replacement for pools
Why are we spending time trying to optimize pools when we haven't eliminated the malloc/frees in the bucket brigade calls? The miniscule performance improvements you -might- get optimizing pools will be completely obscured by the overhead of the malloc/frees. Bill Because it also eliminates the locking in the threaded mpm. And, I wanted to see if I could squeeze some extra performance out of it and make it a bit more readable. I don't know if I succeeded in the latter. Or the performance for that matter :) Btw, why couldn't we use pools in brigades again? Brigades need to be maintained across multiple requests to handle http pipelined responses, so allocating them out of a request pool clearly will not work. You could allocate the brigades out of a connection pool, but connections can remain active for 100's of requests so you could leak a lot of storage for the duration of the connection. Wait, couldn't you create a new pool for each brigade, allocate the brigade out of that pool, associate that pool with the brigade? Then, when the brigade needs to go, just destroy that pool. What am I missing? Bill
