You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Jeff Trawick <tr...@attglobal.net> on 2003/02/20 19:06:55 UTC
[PATCH] clean up debug patch used with Apache 2 on daedalus
Network apps can all too often be tricked by clients into segfaulting or
worse. When this occurs the first time, it can be important to know
what data was received and with what boundaries (i.e., how much was
passed up at a time to the app).
For quite a while (>1.5 years I think) www.apache.org has run with a
patch to APR which saves a certain amount of data read from the client.
It is then used for debugging in the case of a segfault. Having
access to the actual data and in what segments it was returned to Apache
helped fix some bugs pretty quickly.
I've cleaned up that patch a bit in hopes of showing how the
functionality could be added to APR. In this new patch, the saving of
data is turned on by the app via a socket option rather than always
enabled as in the www.apache.org patch.
Is anybody for/against putting something like this in APR?
It would be nice to enable the app to change the parameters (how many
buffers to save, how much of each buffer to save). I guess those could
be separate options in the future, with the current constants "20" and
"1024" simply the defaults.
It would also be nice to allow the app to retrieve the list of saved
input buffers, though it is still useful even without that feature.
Re: [PATCH] clean up debug patch used with Apache 2 on daedalus
Posted by Ian Holsman <ia...@apache.org>.
+1 from me.
now all we need is a directive in httpd to turn this feature on I guess.
Jeff Trawick wrote:
> Network apps can all too often be tricked by clients into segfaulting or
> worse. When this occurs the first time, it can be important to know
> what data was received and with what boundaries (i.e., how much was
> passed up at a time to the app).
>
> For quite a while (>1.5 years I think) www.apache.org has run with a
> patch to APR which saves a certain amount of data read from the client.
> It is then used for debugging in the case of a segfault. Having access
> to the actual data and in what segments it was returned to Apache helped
> fix some bugs pretty quickly.
>
> I've cleaned up that patch a bit in hopes of showing how the
> functionality could be added to APR. In this new patch, the saving of
> data is turned on by the app via a socket option rather than always
> enabled as in the www.apache.org patch.
>
> Is anybody for/against putting something like this in APR?
>
> It would be nice to enable the app to change the parameters (how many
> buffers to save, how much of each buffer to save). I guess those could
> be separate options in the future, with the current constants "20" and
> "1024" simply the defaults.
>
> It would also be nice to allow the app to retrieve the list of saved
> input buffers, though it is still useful even without that feature.
Re: [PATCH] clean up debug patch used with Apache 2 on daedalus
Posted by David Reid <dr...@jetnet.co.uk>.
This could be useful in many situations... I've got no objections.
david
----- Original Message -----
From: "Jeff Trawick" <tr...@attglobal.net>
To: <de...@apr.apache.org>
Sent: Thursday, February 20, 2003 6:06 PM
Subject: [PATCH] clean up debug patch used with Apache 2 on daedalus
> Network apps can all too often be tricked by clients into segfaulting or
> worse. When this occurs the first time, it can be important to know
> what data was received and with what boundaries (i.e., how much was
> passed up at a time to the app).
>
> For quite a while (>1.5 years I think) www.apache.org has run with a
> patch to APR which saves a certain amount of data read from the client.
> It is then used for debugging in the case of a segfault. Having
> access to the actual data and in what segments it was returned to Apache
> helped fix some bugs pretty quickly.
>
> I've cleaned up that patch a bit in hopes of showing how the
> functionality could be added to APR. In this new patch, the saving of
> data is turned on by the app via a socket option rather than always
> enabled as in the www.apache.org patch.
>
> Is anybody for/against putting something like this in APR?
>
> It would be nice to enable the app to change the parameters (how many
> buffers to save, how much of each buffer to save). I guess those could
> be separate options in the future, with the current constants "20" and
> "1024" simply the defaults.
>
> It would also be nice to allow the app to retrieve the list of saved
> input buffers, though it is still useful even without that feature.
>
----------------------------------------------------------------------------
----
> Index: include/apr_network_io.h
> ===================================================================
> RCS file: /home/cvs/apr/include/apr_network_io.h,v
> retrieving revision 1.136
> diff -u -r1.136 apr_network_io.h
> --- include/apr_network_io.h 1 Jan 2003 00:01:45 -0000 1.136
> +++ include/apr_network_io.h 20 Feb 2003 17:53:17 -0000
> @@ -133,6 +133,9 @@
> #define APR_IPV6_V6ONLY 16384 /**< Don't accept IPv4 connections on
an
> * IPv6 listening socket.
> */
> +#define APR_SO_SAVE_INPUT 32768 /**< Maintain a copy of data read from
the
> + * network.
> + */
>
> /** @} */
>
> Index: include/arch/unix/apr_arch_networkio.h
> ===================================================================
> RCS file: /home/cvs/apr/include/arch/unix/apr_arch_networkio.h,v
> retrieving revision 1.1
> diff -u -r1.1 apr_arch_networkio.h
> --- include/arch/unix/apr_arch_networkio.h 6 Jan 2003 23:44:26 -0000 1.1
> +++ include/arch/unix/apr_arch_networkio.h 20 Feb 2003 17:53:17 -0000
> @@ -111,6 +111,9 @@
> #if APR_HAVE_SYS_SENDFILE_H
> #include <sys/sendfile.h>
> #endif
> +#ifdef HAVE_STDDEF_H
> +#include <stddef.h>
> +#endif
> /* End System Headers */
>
> #ifndef HAVE_POLLIN
> @@ -122,6 +125,14 @@
> #define POLLNVAL 32
> #endif
>
> +typedef struct apr_net_input_buffer_t apr_net_input_buffer_t;
> +struct apr_net_input_buffer_t {
> + struct apr_net_input_buffer_t *prev;
> + apr_int32_t saved_len;
> + apr_int32_t actual_len;
> + char data[1]; /* actual data starts here */
> +};
> +
> struct apr_socket_t {
> apr_pool_t *cntxt;
> int socketdes;
> @@ -138,6 +149,8 @@
> int remote_addr_unknown;
> apr_int32_t netmask;
> apr_int32_t inherit;
> + apr_int32_t num_input_buffers;
> + apr_net_input_buffer_t *input_buffers;
> };
>
> const char *apr_inet_ntop(int af, const void *src, char *dst, apr_size_t
size);
> Index: network_io/unix/sendrecv.c
> ===================================================================
> RCS file: /home/cvs/apr/network_io/unix/sendrecv.c,v
> retrieving revision 1.95
> diff -u -r1.95 sendrecv.c
> --- network_io/unix/sendrecv.c 7 Jan 2003 00:52:56 -0000 1.95
> +++ network_io/unix/sendrecv.c 20 Feb 2003 17:53:17 -0000
> @@ -141,6 +141,21 @@
> sock->netmask |= APR_INCOMPLETE_READ;
> }
> (*len) = rv;
> + if (sock->netmask & APR_SO_SAVE_INPUT) {
> + if (sock->num_input_buffers < 20) {
> + apr_size_t bytes_to_save = (*len > 1024) ? 1024 : *len;
> + apr_net_input_buffer_t *new =
> + apr_palloc(sock->cntxt,
> + offsetof(struct apr_net_input_buffer_t,
> + data) + bytes_to_save);
> +
> + new->saved_len = bytes_to_save;
> + new->actual_len = *len;
> + new->prev = sock->input_buffers;
> + sock->input_buffers = new;
> + ++sock->num_input_buffers;
> + }
> + }
> if (rv == 0) {
> return APR_EOF;
> }
> Index: network_io/unix/sockopt.c
> ===================================================================
> RCS file: /home/cvs/apr/network_io/unix/sockopt.c,v
> retrieving revision 1.66
> diff -u -r1.66 sockopt.c
> --- network_io/unix/sockopt.c 7 Feb 2003 20:34:27 -0000 1.66
> +++ network_io/unix/sockopt.c 20 Feb 2003 17:53:17 -0000
> @@ -325,6 +325,9 @@
> return APR_ENOTIMPL;
> #endif
> break;
> + case APR_SO_SAVE_INPUT:
> + apr_set_option(&sock->netmask, APR_SO_SAVE_INPUT, on);
> + break;
> default:
> return APR_EINVAL;
> }
>
Re: [PATCH] clean up debug patch used with Apache 2 on daedalus
Posted by Jeff Trawick <tr...@attglobal.net>.
Ian Holsman wrote:
> Jeff Trawick wrote:
>
> > *"not to share" is a bit strong... our Apache2-based server is freely
> > available for download and the modifications made to pure Apache2+APR
> > are provided in a patch file that gets installed... but that isn't
> > what I usually mean by sharing code :)
> >
> >
> do you have a link for your apache2-based server and the patch file ?
I trimmed down the patch file we use with Apache 2.0.44 code to just the
APR networking changes (what I call the "daedalus patch" + socket
IOLs)... It is at
http://www.apache.org/~trawick/http://www.apache.org/~trawick/apr_networking_patch
Alas, the socket IOL portion of the patch is optimized for minimizing
changes to ASF files, not for putting stuff where it really belongs.
But I'm not too proud to show it, and it does work reliably and allows
multiple modules to insert their tentacles :)
(The vast majority of the patch you get when you install our
Apache-based server, beyond the patch above, is merely the backports of
selected fixes that came out after the level of Apache the server was
based on. There's not a line of code in the resulting httpd that you
can't get from an ASF tarball or the patch file.)
Our Apache2-based server can be downloaded from here, by the way:
http://www-3.ibm.com/software/webservers/httpservers/IHS20.html
But I always recommend starting at http://httpd.apache.org/download.cgi
unless you don't know how to compile or you want to pay for handholding :)
Re: [PATCH] clean up debug patch used with Apache 2 on daedalus
Posted by Jeff Trawick <tr...@attglobal.net>.
Ian Holsman wrote:
> Jeff Trawick wrote:
>
> > *"not to share" is a bit strong... our Apache2-based server is freely
> > available for download and the modifications made to pure Apache2+APR
> > are provided in a patch file that gets installed... but that isn't
> > what I usually mean by sharing code :)
> >
> >
> do you have a link for your apache2-based server and the patch file ?
I trimmed down the patch file we use with Apache 2.0.44 code to just the
APR networking changes (what I call the "daedalus patch" + socket
IOLs)... It is at
http://www.apache.org/~trawick/http://www.apache.org/~trawick/apr_networking_patch
Alas, the socket IOL portion of the patch is optimized for minimizing
changes to ASF files, not for putting stuff where it really belongs.
But I'm not too proud to show it, and it does work reliably and allows
multiple modules to insert their tentacles :)
(The vast majority of the patch you get when you install our
Apache-based server, beyond the patch above, is merely the backports of
selected fixes that came out after the level of Apache the server was
based on. There's not a line of code in the resulting httpd that you
can't get from an ASF tarball or the patch file.)
Our Apache2-based server can be downloaded from here, by the way:
http://www-3.ibm.com/software/webservers/httpservers/IHS20.html
But I always recommend starting at http://httpd.apache.org/download.cgi
unless you don't know how to compile or you want to pay for handholding :)
Re: [PATCH] clean up debug patch used with Apache 2 on daedalus
Posted by Ian Holsman <ia...@apache.org>.
Jeff Trawick wrote:
> *"not to share" is a bit strong... our Apache2-based server is freely
> available for download and the modifications made to pure Apache2+APR
> are provided in a patch file that gets installed... but that isn't what
> I usually mean by sharing code :)
>
>
do you have a link for your apache2-based server and the patch file ?
Re: [PATCH] clean up debug patch used with Apache 2 on daedalus
Posted by Jeff Trawick <tr...@attglobal.net>.
Joe Orton wrote:
> On Fri, Feb 21, 2003 at 02:27:08PM -0500, Jeff Trawick wrote:
> ...persuasively and at length
>
> Consider my misgivings misguided ;) You might want to swap
> stddef.h+offsetof for apr_general.h+APR_OFFSETOF in the patch.
you're too easy
will-do
Re: [PATCH] clean up debug patch used with Apache 2 on daedalus
Posted by Joe Orton <jo...@manyfish.co.uk>.
On Fri, Feb 21, 2003 at 02:27:08PM -0500, Jeff Trawick wrote:
...persuasively and at length
Consider my misgivings misguided ;) You might want to swap
stddef.h+offsetof for apr_general.h+APR_OFFSETOF in the patch.
joe
Re: [PATCH] clean up debug patch used with Apache 2 on daedalus
Posted by Jeff Trawick <tr...@attglobal.net>.
Joe Orton wrote:
> On Fri, Feb 21, 2003 at 07:58:16AM -0500, Jeff Trawick wrote:
>
> >Joe Orton wrote:
> >>Surely this can be done already outside APR?
> >
> >Joe, what is it you really want to say :) I think we all know the
> >answer to that question already.
>
>
> Just seems like classic creeping featurism to me... will you duplicate
> this in the every socket_recv implementation, for example.
theoretically easier to duplicate in every socket_recv than in every APR
app (not that many would really care)
> Sounds like
> a good idea for a filter though ;)
not suitable for me, since I assume from the start that filters are
going to be part of the problem, so I need to be outside of the scope of
filtering :)
and yes, I have used filter-based trace mechanisms (mod_ext_filter+tee
is my current favorite for output, though I once wrote a filter-based
module for Apache to trace what was passed from one filter to another)
--/--
Certainly I don't consider this a required or even a pleasant addition
to APR, but it has proven very useful for Apache in the past, and
keeping it from being generally available is uncool.
[By the way Joe, I don't mean anything in the following to express
negative feelings towards your negative feelings on my patch, or towards
negative feelings previously expressed at some of the other patches
mentioned below. I really do respect the issues and don't pretend there
is some obvious answer.]
I find the issue of code serviceability vs. code maintainability pretty
interesting. There is a certain amount of serviceability you get for
free on Unix (packet traces, backtraces on active processes, system call
traces, kernel traces, core dumps), and you don't have to clutter your
code to take advantage of it. As long as the Unix variant has the right
tools (not all do) and the user has the right skills (not all do), the
vast majority of problems can eventually be solved without cluttering
the code with trace capability and saving footprints or other data in
case of an exception.
If your Unix variant is missing some diagnostic features or you don't
have the skills or you want to debug the problem very quickly, then you
are broken, and need something more. Note that one of these conditions
is always true when time is money and the transactions going through the
server are actually important.
I posted a patch a day or so to dev@httpd to call a hook from
sig_coredump. Several people indicated strong displeasure at it but
gave no concrete objections. To hell with that... I'm sticking it in
the version of Apache "we at the office" support and when somebody comes
to my office bemoaning the fact that there is a problem getting the
right info at the time of the SIGWHATEVER I'll grin and start whipping
up something the customer can LoadModule into their server to make it
happen.
Many moons ago Bill Stoddard posted a patch to add socket IOLs to APR.
I tried to convince folks that it was great for adding tracing to APR
apps. But it was not strictly necessary and Apache filtering was really
the right way to go and whatever else. To hell with that... It is in
the version of Apache we support and when customers have some types of
problems I don't care whether they can spell tcpdump or can get a
working version or I can figure out the right incantation to avoid
superfluous information in the output or even if the customer is willing
to install such code on their box. Instead, I give them a network trace
module for the version of Apache we support that lets them easily
capture the right stuff with no command-line hocus pocus.
Any Unix kernel routine on OS/390 a.k.a. z/OS sets a secondary error
code that encapsulates the source code module that set the error and
usually something more finely-grained the errno that tells what went
wrong. You or I can't do much with the source code module but there are
people I know that can. The finely-grained error code is documented.
Many moons ago I suggested a mechanism in APR such that apps could find
out any such extra information for the kernel routine that caused the
APR function to fail (not necessarily the last kernel routine APR called
before returning). But this was too ugly or cluttering or some such.
(I bet you'll guess the next part.) To hell with that... If I ever
have to support Apache on that OS you bet we won't be losing that info,
at least for the common points of failure.)
Oh, I almost forgot. Our customers are running with the daedalus patch too.
I'm tickled that we can add serviceability aids to the version of Apache
that we support without breaking binary compatibility with unpatched APR
and unpatched Apache, but it really is a bummer not to share* and it is
a bummer not to have a richer community to make it work cleaner/better
and it is a bummer that it would be hard for j random user to make use
of some of the tools (first apply patch from some non-ASF location and
then rebuild everything). Perhaps what is needed is for some related
project to maintain such patches along with appropriate documentation.
Enough rambling :)
*"not to share" is a bit strong... our Apache2-based server is freely
available for download and the modifications made to pure Apache2+APR
are provided in a patch file that gets installed... but that isn't what
I usually mean by sharing code :)
Re: [PATCH] clean up debug patch used with Apache 2 on daedalus
Posted by Jeff Trawick <tr...@attglobal.net>.
Joe Orton wrote:
> On Fri, Feb 21, 2003 at 07:58:16AM -0500, Jeff Trawick wrote:
>
> >Joe Orton wrote:
> >>Surely this can be done already outside APR?
> >
> >Joe, what is it you really want to say :) I think we all know the
> >answer to that question already.
>
>
> Just seems like classic creeping featurism to me... will you duplicate
> this in the every socket_recv implementation, for example.
theoretically easier to duplicate in every socket_recv than in every APR
app (not that many would really care)
> Sounds like
> a good idea for a filter though ;)
not suitable for me, since I assume from the start that filters are
going to be part of the problem, so I need to be outside of the scope of
filtering :)
and yes, I have used filter-based trace mechanisms (mod_ext_filter+tee
is my current favorite for output, though I once wrote a filter-based
module for Apache to trace what was passed from one filter to another)
--/--
Certainly I don't consider this a required or even a pleasant addition
to APR, but it has proven very useful for Apache in the past, and
keeping it from being generally available is uncool.
[By the way Joe, I don't mean anything in the following to express
negative feelings towards your negative feelings on my patch, or towards
negative feelings previously expressed at some of the other patches
mentioned below. I really do respect the issues and don't pretend there
is some obvious answer.]
I find the issue of code serviceability vs. code maintainability pretty
interesting. There is a certain amount of serviceability you get for
free on Unix (packet traces, backtraces on active processes, system call
traces, kernel traces, core dumps), and you don't have to clutter your
code to take advantage of it. As long as the Unix variant has the right
tools (not all do) and the user has the right skills (not all do), the
vast majority of problems can eventually be solved without cluttering
the code with trace capability and saving footprints or other data in
case of an exception.
If your Unix variant is missing some diagnostic features or you don't
have the skills or you want to debug the problem very quickly, then you
are broken, and need something more. Note that one of these conditions
is always true when time is money and the transactions going through the
server are actually important.
I posted a patch a day or so to dev@httpd to call a hook from
sig_coredump. Several people indicated strong displeasure at it but
gave no concrete objections. To hell with that... I'm sticking it in
the version of Apache "we at the office" support and when somebody comes
to my office bemoaning the fact that there is a problem getting the
right info at the time of the SIGWHATEVER I'll grin and start whipping
up something the customer can LoadModule into their server to make it
happen.
Many moons ago Bill Stoddard posted a patch to add socket IOLs to APR.
I tried to convince folks that it was great for adding tracing to APR
apps. But it was not strictly necessary and Apache filtering was really
the right way to go and whatever else. To hell with that... It is in
the version of Apache we support and when customers have some types of
problems I don't care whether they can spell tcpdump or can get a
working version or I can figure out the right incantation to avoid
superfluous information in the output or even if the customer is willing
to install such code on their box. Instead, I give them a network trace
module for the version of Apache we support that lets them easily
capture the right stuff with no command-line hocus pocus.
Any Unix kernel routine on OS/390 a.k.a. z/OS sets a secondary error
code that encapsulates the source code module that set the error and
usually something more finely-grained the errno that tells what went
wrong. You or I can't do much with the source code module but there are
people I know that can. The finely-grained error code is documented.
Many moons ago I suggested a mechanism in APR such that apps could find
out any such extra information for the kernel routine that caused the
APR function to fail (not necessarily the last kernel routine APR called
before returning). But this was too ugly or cluttering or some such.
(I bet you'll guess the next part.) To hell with that... If I ever
have to support Apache on that OS you bet we won't be losing that info,
at least for the common points of failure.)
Oh, I almost forgot. Our customers are running with the daedalus patch too.
I'm tickled that we can add serviceability aids to the version of Apache
that we support without breaking binary compatibility with unpatched APR
and unpatched Apache, but it really is a bummer not to share* and it is
a bummer not to have a richer community to make it work cleaner/better
and it is a bummer that it would be hard for j random user to make use
of some of the tools (first apply patch from some non-ASF location and
then rebuild everything). Perhaps what is needed is for some related
project to maintain such patches along with appropriate documentation.
Enough rambling :)
*"not to share" is a bit strong... our Apache2-based server is freely
available for download and the modifications made to pure Apache2+APR
are provided in a patch file that gets installed... but that isn't what
I usually mean by sharing code :)
Re: [PATCH] clean up debug patch used with Apache 2 on daedalus
Posted by Joe Orton <jo...@manyfish.co.uk>.
On Fri, Feb 21, 2003 at 07:58:16AM -0500, Jeff Trawick wrote:
> Joe Orton wrote:
>
> >On Thu, Feb 20, 2003 at 01:06:55PM -0500, Jeff Trawick wrote:
> >
> >>Network apps can all too often be tricked by clients into segfaulting or
> >>worse. When this occurs the first time, it can be important to know
> >>what data was received and with what boundaries (i.e., how much was
> >>passed up at a time to the app).
> >
> >
> >Surely this can be done already outside APR?
>
> Joe, what is it you really want to say :) I think we all know the
> answer to that question already.
Just seems like classic creeping featurism to me... will you duplicate
this in the every socket_recv implementation, for example. Sounds like
a good idea for a filter though ;)
joe
Re: [PATCH] clean up debug patch used with Apache 2 on daedalus
Posted by Jeff Trawick <tr...@attglobal.net>.
Joe Orton wrote:
> On Thu, Feb 20, 2003 at 01:06:55PM -0500, Jeff Trawick wrote:
>
> >Network apps can all too often be tricked by clients into segfaulting or
> >worse. When this occurs the first time, it can be important to know
> >what data was received and with what boundaries (i.e., how much was
> >passed up at a time to the app).
>
>
> Surely this can be done already outside APR?
Joe, what is it you really want to say :) I think we all know the
answer to that question already.
Re: [PATCH] clean up debug patch used with Apache 2 on daedalus
Posted by Joe Orton <jo...@manyfish.co.uk>.
On Thu, Feb 20, 2003 at 01:06:55PM -0500, Jeff Trawick wrote:
> Network apps can all too often be tricked by clients into segfaulting or
> worse. When this occurs the first time, it can be important to know
> what data was received and with what boundaries (i.e., how much was
> passed up at a time to the app).
Surely this can be done already outside APR?
joe
Re: [PATCH] clean up debug patch used with Apache 2 on daedalus
Posted by Jeff Trawick <tr...@attglobal.net>.
William A. Rowe, Jr. wrote:
> At 12:15 PM 3/3/2003, Jeff Trawick wrote:
>
> >William A. Rowe, Jr. wrote:
> >
> >
> >>Why not tie this feature as a compile-time option, perhaps work it into
> >>our --maintainer-mode builds. It doesn't help if APR releases are built
> >>without any debugging information anyways.
> >
> >It is certainly easier if APR symbols are known, but it is possible
> otherwise (level of effort depends on the platform and tenacity of the
> debugger).
>
>
> And totally beyond the comprehension of the casual user... that's a good
> reason to add a flag to optionally enable this for most release
> builds, but
> make it the default (and allow it to be disabled) for maintainer-mode
> builds ;-)
So the admin has to have configure just right and the app has to have
the socket options just right? That isn't something I find reasonable.
(Not just because maintainer-mode builds are slower globally and this
socket option is slower locally.)
Re: [PATCH] clean up debug patch used with Apache 2 on daedalus
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 12:15 PM 3/3/2003, Jeff Trawick wrote:
>William A. Rowe, Jr. wrote:
>
>>Why not tie this feature as a compile-time option, perhaps work it into
>>our --maintainer-mode builds. It doesn't help if APR releases are built
>>without any debugging information anyways.
>
>It is certainly easier if APR symbols are known, but it is possible otherwise (level of effort depends on the platform and tenacity of the debugger).
And totally beyond the comprehension of the casual user... that's a good
reason to add a flag to optionally enable this for most release builds, but
make it the default (and allow it to be disabled) for maintainer-mode builds ;-)
Bill
Re: [PATCH] clean up debug patch used with Apache 2 on daedalus
Posted by Jeff Trawick <tr...@attglobal.net>.
William A. Rowe, Jr. wrote:
> Why not tie this feature as a compile-time option, perhaps work it into
> our --maintainer-mode builds. It doesn't help if APR releases are built
> without any debugging information anyways.
It is certainly easier if APR symbols are known, but it is possible
otherwise (level of effort depends on the platform and tenacity of the
debugger).
Re: [PATCH] clean up debug patch used with Apache 2 on daedalus
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Why not tie this feature as a compile-time option, perhaps work it into
our --maintainer-mode builds. It doesn't help if APR releases are built
without any debugging information anyways.
Bill
At 12:06 PM 2/20/2003, Jeff Trawick wrote:
>Network apps can all too often be tricked by clients into segfaulting or worse. When this occurs the first time, it can be important to know what data was received and with what boundaries (i.e., how much was passed up at a time to the app).
>
>For quite a while (>1.5 years I think) www.apache.org has run with a patch to APR which saves a certain amount of data read from the client. It is then used for debugging in the case of a segfault. Having access to the actual data and in what segments it was returned to Apache helped fix some bugs pretty quickly.
>
>I've cleaned up that patch a bit in hopes of showing how the functionality could be added to APR. In this new patch, the saving of data is turned on by the app via a socket option rather than always enabled as in the www.apache.org patch.
>
>Is anybody for/against putting something like this in APR?
>
>It would be nice to enable the app to change the parameters (how many buffers to save, how much of each buffer to save). I guess those could be separate options in the future, with the current constants "20" and "1024" simply the defaults.
>
>It would also be nice to allow the app to retrieve the list of saved input buffers, though it is still useful even without that feature.
>
>
>Index: include/apr_network_io.h
>===================================================================
>RCS file: /home/cvs/apr/include/apr_network_io.h,v
>retrieving revision 1.136
>diff -u -r1.136 apr_network_io.h
>--- include/apr_network_io.h 1 Jan 2003 00:01:45 -0000 1.136
>+++ include/apr_network_io.h 20 Feb 2003 17:53:17 -0000
>@@ -133,6 +133,9 @@
> #define APR_IPV6_V6ONLY 16384 /**< Don't accept IPv4 connections on an
> * IPv6 listening socket.
> */
>+#define APR_SO_SAVE_INPUT 32768 /**< Maintain a copy of data read from the
>+ * network.
>+ */
>
> /** @} */
>
>Index: include/arch/unix/apr_arch_networkio.h
>===================================================================
>RCS file: /home/cvs/apr/include/arch/unix/apr_arch_networkio.h,v
>retrieving revision 1.1
>diff -u -r1.1 apr_arch_networkio.h
>--- include/arch/unix/apr_arch_networkio.h 6 Jan 2003 23:44:26 -0000 1.1
>+++ include/arch/unix/apr_arch_networkio.h 20 Feb 2003 17:53:17 -0000
>@@ -111,6 +111,9 @@
> #if APR_HAVE_SYS_SENDFILE_H
> #include <sys/sendfile.h>
> #endif
>+#ifdef HAVE_STDDEF_H
>+#include <stddef.h>
>+#endif
> /* End System Headers */
>
> #ifndef HAVE_POLLIN
>@@ -122,6 +125,14 @@
> #define POLLNVAL 32
> #endif
>
>+typedef struct apr_net_input_buffer_t apr_net_input_buffer_t;
>+struct apr_net_input_buffer_t {
>+ struct apr_net_input_buffer_t *prev;
>+ apr_int32_t saved_len;
>+ apr_int32_t actual_len;
>+ char data[1]; /* actual data starts here */
>+};
>+
> struct apr_socket_t {
> apr_pool_t *cntxt;
> int socketdes;
>@@ -138,6 +149,8 @@
> int remote_addr_unknown;
> apr_int32_t netmask;
> apr_int32_t inherit;
>+ apr_int32_t num_input_buffers;
>+ apr_net_input_buffer_t *input_buffers;
> };
>
> const char *apr_inet_ntop(int af, const void *src, char *dst, apr_size_t size);
>Index: network_io/unix/sendrecv.c
>===================================================================
>RCS file: /home/cvs/apr/network_io/unix/sendrecv.c,v
>retrieving revision 1.95
>diff -u -r1.95 sendrecv.c
>--- network_io/unix/sendrecv.c 7 Jan 2003 00:52:56 -0000 1.95
>+++ network_io/unix/sendrecv.c 20 Feb 2003 17:53:17 -0000
>@@ -141,6 +141,21 @@
> sock->netmask |= APR_INCOMPLETE_READ;
> }
> (*len) = rv;
>+ if (sock->netmask & APR_SO_SAVE_INPUT) {
>+ if (sock->num_input_buffers < 20) {
>+ apr_size_t bytes_to_save = (*len > 1024) ? 1024 : *len;
>+ apr_net_input_buffer_t *new =
>+ apr_palloc(sock->cntxt,
>+ offsetof(struct apr_net_input_buffer_t,
>+ data) + bytes_to_save);
>+
>+ new->saved_len = bytes_to_save;
>+ new->actual_len = *len;
>+ new->prev = sock->input_buffers;
>+ sock->input_buffers = new;
>+ ++sock->num_input_buffers;
>+ }
>+ }
> if (rv == 0) {
> return APR_EOF;
> }
>Index: network_io/unix/sockopt.c
>===================================================================
>RCS file: /home/cvs/apr/network_io/unix/sockopt.c,v
>retrieving revision 1.66
>diff -u -r1.66 sockopt.c
>--- network_io/unix/sockopt.c 7 Feb 2003 20:34:27 -0000 1.66
>+++ network_io/unix/sockopt.c 20 Feb 2003 17:53:17 -0000
>@@ -325,6 +325,9 @@
> return APR_ENOTIMPL;
> #endif
> break;
>+ case APR_SO_SAVE_INPUT:
>+ apr_set_option(&sock->netmask, APR_SO_SAVE_INPUT, on);
>+ break;
> default:
> return APR_EINVAL;
> }