You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by ha...@thebackrow.net on 2001/08/17 20:43:26 UTC
[PATCH] fix segfaults related to ap_custom_response()
Hi there. I wrote in earlier this week about bug #6336 and
ap_custom_response (basically, any use of ap_custom_response() seems to
guaruntee a segfault in ap_die() on the next error response). The
attached patch (againsts 1.3.20) seems to fix the problem. I had
someone at work shove about 3.5 million requests into it, and the
server seems stable with the patch applied.
Here is what I did:
* Added a char** response_code_strings field to the end of request_rec
* Modified ap_custom_response() to set r->response_code_strings ...
previously, it set conf->response_code_strings, using memory from
r->pool. After the current request was finished,
conf->response_code_strings still pointed to stuff in the now-defunct
r->pool, and the next time ap_die() tried to use it to find an
ErrorDocument, it'd segfault.
* Modified ap_response_code_string() to check if the request_rec has a
response_code_string that applies. If it doesn't, it then checks the
per-dir config as before. This, to me anyway, is the expected
behavior: if I call ap_custom_response(), I mean to be setting a
custom response for only the current request, and I expect it to
override the directory configuration.
If anybody with more apache-hacking experience wants to suggest another
solution, I'm game to try implementing it. But as far as I can tell,
without a fix of some sort, ap_custom_response() is basically
unusable.
--
thanks,
Will
Re: [PATCH] fix segfaults related to ap_custom_response()
Posted by Will Lowe <ha...@thebackrow.net>.
> Please resend your patch without the compression. We like our
Oops. Here you go.
Will
diff -ur apache_1.3.20/src/main/http_core.c apache_1.3.20-new/src/main/http_core.c
--- apache_1.3.20/src/main/http_core.c Fri Mar 9 02:10:25 2001
+++ apache_1.3.20-new/src/main/http_core.c Fri Aug 17 23:27:58 2001
@@ -581,9 +581,16 @@
{
core_dir_config *conf;
- conf = (core_dir_config *)ap_get_module_config(r->per_dir_config,
+ /* prefer per-request settings */
+ conf = (core_dir_config *)ap_get_module_config(r->request_config,
&core_module);
+ /* but if there aren't any, try the dir config */
+ if ( conf == NULL ) {
+ conf = (core_dir_config *) ap_get_module_config(r->per_dir_config,
+ &core_module);
+ }
+
if (conf->response_code_strings == NULL) {
return NULL;
}
@@ -1165,8 +1172,14 @@
API_EXPORT(void) ap_custom_response(request_rec *r, int status, char *string)
{
core_dir_config *conf =
- ap_get_module_config(r->per_dir_config, &core_module);
+ ap_get_module_config(r->request_config, &core_module);
int idx;
+
+ if(conf == NULL) {
+ /* if this doesn't exist, we'll have to make one */
+ conf = (core_dir_config*) ap_pcalloc(r->pool, sizeof(core_dir_config));
+ ap_set_module_config(r->request_config, &core_module, conf);
+ }
if(conf->response_code_strings == NULL) {
conf->response_code_strings =
Re: [PATCH] fix segfaults related to ap_custom_response()
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Tue, Aug 21, 2001 at 01:02:28AM -0400, harpo@thebackrow.net wrote:
> > > We are unlikely to accept a patch that modifies request_rec structures,
> > struct ap_conf_vector_t *request_config;
>
> How about the attached patch, which fixes the same thing but uses
> r->request_config instead of adding a field to r?
Please resend your patch without the compression. We like our
patches inlined (no attachments unless you use LookOut! which munges
long lines) and definitely not compressed.
http://dev.apache.org/patches.html
-- justin
[PATCH] fix segfaults related to ap_custom_response()
Posted by ha...@thebackrow.net.
> > We are unlikely to accept a patch that modifies request_rec structures,
> struct ap_conf_vector_t *request_config;
How about the attached patch, which fixes the same thing but uses
r->request_config instead of adding a field to r?
--
thanks,
Will
Re: [PATCH] fix segfaults related to ap_custom_response()
Posted by Jeff Trawick <tr...@attglobal.net>.
Will Lowe <ha...@thebackrow.net> writes:
> On Fri, Aug 17, 2001 at 01:59:34PM -0500, William A. Rowe, Jr. wrote:
> > We are unlikely to accept a patch that modifies request_rec structures,
> > since that (significantly) breaks binary compatiblity.
>
> I was sorta afraid of that. Even if it's at the *end* of the struct?
> I figured that wouldn't break much.
>
> > I'd be happy to entertain a patch if you can avoid breaking binary
>
> Hmm. The problem is that ap_custom_response() seems to be meant to set
> custom responses on a per-request basis. Is there anyplace else that I
> can store per-request data?
struct request_rec {
...
/** Notes on *this* request */
struct ap_conf_vector_t *request_config;
};
(I didn't look at the patch and have no opinion whether or not
request_config is appropriate, but it is a possibility. See how it is
used in existing code.)
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] fix segfaults related to ap_custom_response()
Posted by Will Lowe <ha...@thebackrow.net>.
On Fri, Aug 17, 2001 at 01:59:34PM -0500, William A. Rowe, Jr. wrote:
> We are unlikely to accept a patch that modifies request_rec structures,
> since that (significantly) breaks binary compatiblity.
I was sorta afraid of that. Even if it's at the *end* of the struct?
I figured that wouldn't break much.
> I'd be happy to entertain a patch if you can avoid breaking binary
Hmm. The problem is that ap_custom_response() seems to be meant to set
custom responses on a per-request basis. Is there anyplace else that I
can store per-request data?
--
thanks,
Will
Re: [PATCH] fix segfaults related to ap_custom_response()
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
We are unlikely to accept a patch that modifies request_rec structures,
since that (significantly) breaks binary compatiblity.
I'd be happy to entertain a patch if you can avoid breaking binary compatibility.
Bill
----- Original Message -----
From: <ha...@thebackrow.net>
To: <ne...@apache.org>
Sent: Friday, August 17, 2001 1:43 PM
Subject: [PATCH] fix segfaults related to ap_custom_response()
> Hi there. I wrote in earlier this week about bug #6336 and
> ap_custom_response (basically, any use of ap_custom_response() seems to
> guaruntee a segfault in ap_die() on the next error response). The
> attached patch (againsts 1.3.20) seems to fix the problem. I had
> someone at work shove about 3.5 million requests into it, and the
> server seems stable with the patch applied.
>
> Here is what I did:
>
> * Added a char** response_code_strings field to the end of request_rec
>
> * Modified ap_custom_response() to set r->response_code_strings ...
> previously, it set conf->response_code_strings, using memory from
> r->pool. After the current request was finished,
> conf->response_code_strings still pointed to stuff in the now-defunct
> r->pool, and the next time ap_die() tried to use it to find an
> ErrorDocument, it'd segfault.
>
> * Modified ap_response_code_string() to check if the request_rec has a
> response_code_string that applies. If it doesn't, it then checks the
> per-dir config as before. This, to me anyway, is the expected
> behavior: if I call ap_custom_response(), I mean to be setting a
> custom response for only the current request, and I expect it to
> override the directory configuration.
>
> If anybody with more apache-hacking experience wants to suggest another
> solution, I'm game to try implementing it. But as far as I can tell,
> without a fix of some sort, ap_custom_response() is basically
> unusable.
>
> --
> thanks,
>
> Will
>