Increase request related max limits or stop enforcing them

[gnotaras]

Please either increase the maximum limits for --limit-request-line & --limit-request-field-size (currently hardcoded at 8190) or stop enforcing those limits once the user has used the aforementioned options in the configuration file or in the command line.

IMHO, enforcing a maximum size is not useful. I'd suggest that you provided some information in the documentation about how the application performance and security would be affected by setting those settings too high or too low, instead of hardcoding a max limit.

Thank you

[gnotaras]

I increased the maximum request line limit to 16384 with the following patch:

--- gunicorn/http/message.py    Sun Jul 01 19:59:55 2012
+++ gunicorn/http/message.py    Sun Jul 01 19:59:41 2012
@@ -16,7 +16,7 @@
 InvalidRequestLine, InvalidRequestMethod, InvalidHTTPVersion, \
 LimitRequestLine, LimitRequestHeaders

-MAX_REQUEST_LINE = 8190
+MAX_REQUEST_LINE = 16384
 MAX_HEADERS = 32768
 MAX_HEADERFIELD_SIZE = 8190

[benoitc]

What kind of line can be > 8192 ?

[gnotaras]

Hello and thanks for your reply.

I encountered the issue in a django app that uses the standard admin edit forms. The edit form has some textareas for pasting lengthy pieces of text, eg base64 encoded certificates and public keys, server configuration snippets, etc.

The form is defined as follows:

<form enctype="multipart/form-data" action="" method="post" id="domain_form">
...
</form>

It seems that the size of the request line also counts the size of the post payload of the request. One would expect that this MAX_REQUEST_LINE limit would only affect GET requests. I'm wild guessing here. I haven't dug into the code to make sure that this is the case.

Thanks for looking into this.

[davisp]

Its possible that there's a bug here, but that's a bug that's just being covered up by extending those limits. Can you perhaps capture a request that demonstrates the issue?

Also, we may want to make those limits configurable regardless.

[gnotaras]

I used the following patch in order to capture what data is checked for max request line limit:

--- gunicorn/http/message.py    Tue Jul 03 12:21:23 2012
+++ gunicorn/http/message.py    Tue Jul 03 12:22:27 2012
@@ -159,17 +159,22 @@
         buf = StringIO()
         self.get_data(unreader, buf, stop=True)

+        ff = open('/tmp/gunicorn-capture.log', 'a')
         # Request line
         data = buf.getvalue()
+        ff.write(data)
         while True:
             idx = data.find("\r\n")
             if idx >= 0:
                 break
             self.get_data(unreader, buf)
-            data = buf.getvalue()

+            data = buf.getvalue()
+            ff.write(data)
             if len(data) - 2 > self.limit_request_line > 0:
                 raise LimitRequestLine(len(data), self.limit_request_line)
+        ff.close()
+

         self.parse_request_line(data[:idx])
         buf = StringIO()

I got the following gist: https://gist.github.com/3038688

All values, certificates etc are from a test environment.

I expected to see only the following line:

POST /testpanel/hosting/domain/3/ HTTP/1.1

Then again I might be wrong.

If you want me to follow a specific procedure for capturing the request, so as to be able to reproduce the error, please let me know. It would be really helpful if the req object had a str() method.

[davisp]

Yep, looks like a bug. Try this simple patch:

diff --git a/gunicorn/http/message.py b/gunicorn/http/message.py
index 4f7dfe5..e2c7500 100644
--- a/gunicorn/http/message.py
+++ b/gunicorn/http/message.py
@@ -165,12 +165,13 @@ class Request(Message):
             idx = data.find("\r\n")
             if idx >= 0:
                 break
-            self.get_data(unreader, buf)
-            data = buf.getvalue()
 
             if len(data) - 2 > self.limit_request_line > 0:
                 raise LimitRequestLine(len(data), self.limit_request_line)
 
+            self.get_data(unreader, buf)
+            data = buf.getvalue()
+
         self.parse_request_line(data[:idx])
         buf = StringIO()
         buf.write(data[idx+2:]) # Skip \r\n

The basic logic is that we're testing length without testing for the presence of '\r\n' which is wrong because that read can bring in lots of data that would exceed the threshold but also contain a valid request line. Moving the test should change the logic to "there are greater than self.limit_request_line bytes that don't contain a '\r\n'" which is what we want.

[gnotaras]

That seems to do it. No more 400 errors. Thank you.

[benoitc]

+1 for me too

[djoume]

This patch breaks the build:

http://travis-ci.org/#!/benoitc/gunicorn/builds

I haven't been able to reproduce the issue, gnotaras could you share a minimum program that cause it?

[djoume]

The patch has been reverted here 4b478e1
@gnotaras if you could share a minimum program to reproduce your issue I could look into it

[benoitc]

Gunicorn is using similar defaults to apache See for example LimirRequestLine . Nginx have the same if I remember. Generally you don want a bigger request line since it can be used for a DoS. Itś easy to ignore them if needed by setting them to 0. I don think we should change them.

[benoitc]

Is this still a problem?

[benoitc]

no answer since. considering the problem is fixed. closing the issue.

[fossilet]

I use Nginx + Gunicorn. It seems Nginx does not pose a hard limit for the request line length and I can adjust it to a quite large number. For Gunicorn, I have to modify the source to make my application work.

Even though Apache hard-code the limit to 8190, in RFC 2616 it says:

Servers MUST be able to handle the URI of any resource they
serve, and SHOULD be able to handle URIs of unbounded length if they
provide GET-based forms that could generate such URIs.

I think HTTP server should not put a hard limit on the max request-line length. It is up to the server administrator to set it to a sensible value for him.

[benoitc]

Actually nginx has a limit which is 4k or 8k depending on the machine.

This limit here is for secure reason since it's really easy to use it to DOS a server. You can increase the default if you need.

[fossilet]

I mean the maximum length which users can set. 4k and 8k are the default values, which people can override. I think Nginx does not limit the maximum length.

Anyway it's easy to change and OK for me.

[janaki-sasidhar]

What kind of line can be > 8192 ?

datatables