You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Marc Slemko <ma...@worldgate.com> on 1998/01/01 07:04:03 UTC
worth fixing "read headers forever" issue?
In PR1028, Dean doesn't really seem to think that one-off fixes for things
like this are the way to go.
I could agree, but don't see anything changing right now.
This particular issue of reading request headers forever is easy to fix;
just add a counter to get_mime_headers that only allows it to read xxx
bytes. I am thinking it is worth it even though it requires x bytes of
bandwidth to require the server to grow x bytes.
Memory attacks are easier to do some generic limiting on via the pools
mechanism Dean suggests.
CPU or disk eating things are hard to do that way, and I am doubtful that
rlimits can be used usefully.
What would be cool to see (and I would like to play with if I worked at a
large web hosting company...) is changes that make it very difficult for
hits on any one part (be it userdir, virtual domain, etc.) of the docspace
to impact other parts while still using the same pool of servers.
Re: worth fixing "read headers forever" issue?
Posted by Dean Gaudet <dg...@arctic.org>.
Yeah he did but my complaint was that it was linear in the number of
children... which is way too slow for some machines.
Dean
On Thu, 1 Jan 1998, Brian Behlendorf wrote:
> At 10:36 PM 12/31/97 -0800, Dean Gaudet wrote:
> >Using gperf or other perfect hash generators you could generate a perfect
> >hash for url-spaces. But you're probably also interested in doing this
> >for incoming IP addresses to prevent too many simultaneous connections,
> >but it should be possible to make the hash table large enough to deal
> >with this too.
>
> Ed Korthof did something along the "prevent too many simultaneous
> connections from the same IP #" path, back when we were getting
> connection-bombed by folks because of a bug in the Windows MSIE 3.0 with
> Macromedia Director installed. Ed, did you ever submit a patch for that?
>
> Brian
>
>
> --=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
> specialization is for insects brian@organic.com
>
Re: worth fixing "read headers forever" issue?
Posted by Brian Behlendorf <br...@organic.com>.
At 10:36 PM 12/31/97 -0800, Dean Gaudet wrote:
>Using gperf or other perfect hash generators you could generate a perfect
>hash for url-spaces. But you're probably also interested in doing this
>for incoming IP addresses to prevent too many simultaneous connections,
>but it should be possible to make the hash table large enough to deal
>with this too.
Ed Korthof did something along the "prevent too many simultaneous
connections from the same IP #" path, back when we were getting
connection-bombed by folks because of a bug in the Windows MSIE 3.0 with
Macromedia Director installed. Ed, did you ever submit a patch for that?
Brian
--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
specialization is for insects brian@organic.com
Re: worth fixing "read headers forever" issue?
Posted by Dean Gaudet <dg...@arctic.org>.
I don't think I'd want this in the code unless it is able to return an
error to the client and bail the rest of the request. Hey Roy, rfc2068 is
lacking a "request headers too large" response. There's 413 and 414 but
neither applies to this specific case. Although I suppose this is really
a server error, so a 500 is fine.
get_mime_headers isn't part of the API so you should be able to change its
return value easily. Make it static while yer at it ;)
You don't have to eat excess or anything special, you just need to have
die(500, r) be called and let it bail all the way into lingering_close I
think.
Dean
On Thu, 1 Jan 1998, Marc Slemko wrote:
> On Thu, 1 Jan 1998, Ben Laurie wrote:
>
> > Marc Slemko wrote:
> > >
> > > below is a sample of a patch that I would suggest to do this;
> > > parts marked XXX are incomplete but easy to see what they would do.
> > >
> > > This limits read headers to ~512k.
> > >
> > > I think this is worthwhile. Does anyone agree?
> > >
> > > Index: http_protocol.c
> > > ===================================================================
> > > RCS file: /export/home/cvs/apachen/src/main/http_protocol.c,v
> > > retrieving revision 1.172
> > > diff -u -r1.172 http_protocol.c
> > > --- http_protocol.c 1997/12/26 18:26:59 1.172
> > > +++ http_protocol.c 1998/01/01 22:23:40
> > > @@ -742,7 +742,7 @@
> > > void get_mime_headers(request_rec *r)
> > > {
> > > conn_rec *c = r->connection;
> > > - int len;
> > > + int len, total = 0;
> > > char *value;
> > > char field[MAX_STRING_LEN];
> > >
> > > @@ -753,6 +753,11 @@
> > > */
> > > while ((len = getline(field, MAX_STRING_LEN, c->client, 1)) > 0) {
> > >
> > > + if (total > 1024*512) { /* XXXX make a define from httpd.h */
> > > + /* XXXX log error */
> > > + /* should puke, too bad we can't */
> > > + return;
> >
> > Presumably it wouldn't be too hard to return true/false, and puke at a
> > higher level? Also, you should eat the excess (or drop the connection).
>
> That would just mean changing the function to actually return something;
> it isn't in the typical situation where you can just return a status code.
> Just closing the connection from within the subroutine would be crude I
> think.
>
> If I did that, I would completely abort it. Yes, it should eat the excess
> if it doesn't.
>
>
Re: worth fixing "read headers forever" issue?
Posted by Marc Slemko <ma...@worldgate.com>.
On Thu, 1 Jan 1998, Ben Laurie wrote:
> Marc Slemko wrote:
> >
> > below is a sample of a patch that I would suggest to do this;
> > parts marked XXX are incomplete but easy to see what they would do.
> >
> > This limits read headers to ~512k.
> >
> > I think this is worthwhile. Does anyone agree?
> >
> > Index: http_protocol.c
> > ===================================================================
> > RCS file: /export/home/cvs/apachen/src/main/http_protocol.c,v
> > retrieving revision 1.172
> > diff -u -r1.172 http_protocol.c
> > --- http_protocol.c 1997/12/26 18:26:59 1.172
> > +++ http_protocol.c 1998/01/01 22:23:40
> > @@ -742,7 +742,7 @@
> > void get_mime_headers(request_rec *r)
> > {
> > conn_rec *c = r->connection;
> > - int len;
> > + int len, total = 0;
> > char *value;
> > char field[MAX_STRING_LEN];
> >
> > @@ -753,6 +753,11 @@
> > */
> > while ((len = getline(field, MAX_STRING_LEN, c->client, 1)) > 0) {
> >
> > + if (total > 1024*512) { /* XXXX make a define from httpd.h */
> > + /* XXXX log error */
> > + /* should puke, too bad we can't */
> > + return;
>
> Presumably it wouldn't be too hard to return true/false, and puke at a
> higher level? Also, you should eat the excess (or drop the connection).
That would just mean changing the function to actually return something;
it isn't in the typical situation where you can just return a status code.
Just closing the connection from within the subroutine would be crude I
think.
If I did that, I would completely abort it. Yes, it should eat the excess
if it doesn't.
Re: worth fixing "read headers forever" issue?
Posted by Ben Laurie <be...@algroup.co.uk>.
Marc Slemko wrote:
>
> below is a sample of a patch that I would suggest to do this;
> parts marked XXX are incomplete but easy to see what they would do.
>
> This limits read headers to ~512k.
>
> I think this is worthwhile. Does anyone agree?
>
> Index: http_protocol.c
> ===================================================================
> RCS file: /export/home/cvs/apachen/src/main/http_protocol.c,v
> retrieving revision 1.172
> diff -u -r1.172 http_protocol.c
> --- http_protocol.c 1997/12/26 18:26:59 1.172
> +++ http_protocol.c 1998/01/01 22:23:40
> @@ -742,7 +742,7 @@
> void get_mime_headers(request_rec *r)
> {
> conn_rec *c = r->connection;
> - int len;
> + int len, total = 0;
> char *value;
> char field[MAX_STRING_LEN];
>
> @@ -753,6 +753,11 @@
> */
> while ((len = getline(field, MAX_STRING_LEN, c->client, 1)) > 0) {
>
> + if (total > 1024*512) { /* XXXX make a define from httpd.h */
> + /* XXXX log error */
> + /* should puke, too bad we can't */
> + return;
Presumably it wouldn't be too hard to return true/false, and puke at a
higher level? Also, you should eat the excess (or drop the connection).
Cheers,
Ben.
--
Ben Laurie |Phone: +44 (181) 735 0686|Apache Group member
Freelance Consultant |Fax: +44 (181) 735 0689|http://www.apache.org
and Technical Director|Email: ben@algroup.co.uk |Apache-SSL author
A.L. Digital Ltd, |http://www.algroup.co.uk/Apache-SSL
London, England. |"Apache: TDG" http://www.ora.com/catalog/apache
Re: worth fixing "read headers forever" issue?
Posted by Marc Slemko <ma...@worldgate.com>.
below is a sample of a patch that I would suggest to do this;
parts marked XXX are incomplete but easy to see what they would do.
This limits read headers to ~512k.
I think this is worthwhile. Does anyone agree?
Index: http_protocol.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_protocol.c,v
retrieving revision 1.172
diff -u -r1.172 http_protocol.c
--- http_protocol.c 1997/12/26 18:26:59 1.172
+++ http_protocol.c 1998/01/01 22:23:40
@@ -742,7 +742,7 @@
void get_mime_headers(request_rec *r)
{
conn_rec *c = r->connection;
- int len;
+ int len, total = 0;
char *value;
char field[MAX_STRING_LEN];
@@ -753,6 +753,11 @@
*/
while ((len = getline(field, MAX_STRING_LEN, c->client, 1)) > 0) {
+ if (total > 1024*512) { /* XXXX make a define from httpd.h */
+ /* XXXX log error */
+ /* should puke, too bad we can't */
+ return;
+ }
if (!(value = strchr(field, ':'))) /* Find the colon separator */
continue; /* or should puke 400 here */
@@ -762,6 +767,7 @@
++value; /* Skip to start of value */
table_merge(r->headers_in, field, value);
+ total += len;
}
}
Re: worth fixing "read headers forever" issue?
Posted by Dean Gaudet <dg...@arctic.org>.
On Wed, 31 Dec 1997, Marc Slemko wrote:
> What would be cool to see (and I would like to play with if I worked at a
> large web hosting company...) is changes that make it very difficult for
> hits on any one part (be it userdir, virtual domain, etc.) of the docspace
> to impact other parts while still using the same pool of servers.
I think we're missing one API phase to do this perfectly as a module...
but here's a solution that doesn't require much of a performance loss.
Let h(r) be a hash function on a request r, such that h(r1) == h(r2)
for two requests r1 and r2 to the "same part of the server". Let H be a
table of unsigned chars in shared memory, indexed by the hash function.
For best effect we want to keep the table within the L2 cache, the size
of the table will depend on h(). Note that unsigned chars work well
for servers with less than 255 children, which is probably a common
enough case.
When a request is received, the child increments H[h(r)] atomically.
If the value is above a threshold the child goes to sleep for K seconds,
if above a higher threshold it returns a 503 error. When done with the
request it decrements H[h(r)] atomically.
There's a problem with kill -9 and core dumps and other unexpected exits.
Using gperf or other perfect hash generators you could generate a perfect
hash for url-spaces. But you're probably also interested in doing this
for incoming IP addresses to prevent too many simultaneous connections,
but it should be possible to make the hash table large enough to deal
with this too.
Actually, I bet you can do this in 1.3 with post_read_request. Shouldn't
be a difficult module to write, except for the lack of shared memory
abstraction.
Dean