Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed a severe memory leak by refactoring request body buffering.

Thanks to Qingshan Luo for reporting this issue!

PyPI bugfix release follows.
  • Loading branch information...
commit 9a7aee060cac4836d92f27e66f312c61c53f9246 1 parent b0a24fd
@jonashaag authored
View
19 Makefile
@@ -59,11 +59,24 @@ prepare-build:
clean:
@rm -f $(BUILD_DIR)/*
-ab:
- ab -c 100 -n 10000 'http://127.0.0.1:8080/a/b/c?k=v&k2=v2#fragment'
+AB = ab -c 100 -n 10000
+TEST_URL = "http://127.0.0.1:8080/a/b/c?k=v&k2=v2"
+
+ab: ab1 ab2 ab3 ab4
+
+ab1:
+ $(AB) $(TEST_URL)
+ab2:
+ @echo 'asdfghjkl=asdfghjkl&qwerty=qwertyuiop' > /tmp/bjoern-post.tmp
+ $(AB) -p /tmp/bjoern-post.tmp $(TEST_URL)
+ab3:
+ $(AB) -k $(TEST_URL)
+ab4:
+ @echo 'asdfghjkl=asdfghjkl&qwerty=qwertyuiop' > /tmp/bjoern-post.tmp
+ $(AB) -k -p /tmp/bjoern-post.tmp $(TEST_URL)
wget:
- wget -O - -q -S 'http://127.0.0.1:8080/a/b/c?k=v&k2=v2#fragment'
+ wget -O - -q -S $(TEST_URL)
test:
cd tests && python ~/dev/wsgitest/runner.py
View
45 bjoern/request.c
@@ -7,6 +7,14 @@ static PyObject* wsgi_http_header(Request*, const char*, const size_t);
static http_parser_settings parser_settings;
static PyObject* wsgi_base_dict = NULL;
+/* Non-public type from cStringIO I abuse in on_body */
+typedef struct {
+ PyObject_HEAD
+ char *buf;
+ Py_ssize_t pos, string_size;
+ PyObject *pbuf;
+} Iobject;
+
Request* Request_new(int client_fd, const char* client_addr)
{
Request* request = malloc(sizeof(Request));
@@ -51,7 +59,6 @@ void Request_clean(Request* request)
Py_DECREF(request->iterable);
}
Py_XDECREF(request->iterator);
- Py_XDECREF(request->body);
if(request->headers)
assert(request->headers->ob_refcnt >= 1);
if(request->status)
@@ -172,19 +179,26 @@ on_headers_complete(http_parser* parser)
}
static int
-on_body(http_parser* parser, const char* body, const size_t len)
+on_body(http_parser* parser, const char* data, const size_t len)
{
- if(!REQUEST->body) {
+ Iobject* body;
+
+ body = (Iobject*)PyDict_GetItem(REQUEST->headers, _wsgi_input);
+ if(body == NULL) {
if(!parser->content_length) {
REQUEST->state.error_code = HTTP_LENGTH_REQUIRED;
return 1;
}
- REQUEST->body = PycStringIO->NewOutput(parser->content_length);
- }
- if(PycStringIO->cwrite(REQUEST->body, body, len) < 0) {
- REQUEST->state.error_code = HTTP_SERVER_ERROR;
- return 1;
+ PyObject* buf = PyString_FromStringAndSize(NULL, parser->content_length);
+ body = (Iobject*)PycStringIO->NewInput(buf);
+ Py_XDECREF(buf);
+ if(body == NULL)
+ return 1;
+ _set_header(_wsgi_input, (PyObject*)body);
+ Py_DECREF(body);
}
+ memcpy(body->buf + body->pos, data, len);
+ body->pos += len;
return 0;
}
@@ -210,12 +224,15 @@ on_message_complete(http_parser* parser)
/* REMOTE_ADDR */
_set_header(_REMOTE_ADDR, REQUEST->client_addr);
- /* wsgi.input */
- _set_header_free_value(
- _wsgi_input,
- PycStringIO->NewInput(REQUEST->body ? PycStringIO->cgetvalue(REQUEST->body)
- : _empty_string)
- );
+ PyObject* body = PyDict_GetItem(REQUEST->headers, _wsgi_input);
+ if(body) {
+ /* We abused the `pos` member for tracking the amount of data copied from
+ * the buffer in on_body, so reset it to zero here. */
+ ((Iobject*)body)->pos = 0;
+ } else {
+ /* Request has no body */
+ _set_header_free_value(_wsgi_input, PycStringIO->NewInput(_empty_string));
+ }
PyDict_Update(REQUEST->headers, wsgi_base_dict);
View
1  bjoern/request.h
@@ -38,7 +38,6 @@ typedef struct {
request_state state;
PyObject* headers;
- PyObject* body;
PyObject* current_chunk;
Py_ssize_t current_chunk_p;
PyObject* iterable;
View
1  tests/headers.py
@@ -3,6 +3,7 @@
def app(env, start_response):
pprint.pprint(env)
+ print(len(env['wsgi.input'].read()))
start_response('200 yo', [])
return []
View
4 tests/pure-req.py
@@ -16,7 +16,9 @@
# Content-{Length, Type}
'GET / HTTP/1.0\r\nContent-Length: 11\r\n'
'Content-Type: text/blah\r\nContent-Fype: bla\r\n'
- 'Content-Tength: bla\r\n\r\nhello world'
+ 'Content-Tength: bla\r\n\r\nhello world',
+ # POST memory leak
+ 'POST / HTTP/1.0\r\nContent-Length: 1000\r\n\r\n%s' % ('a'*1000)
]
conn.send(msgs[int(sys.argv[1])])
while 1:
Please sign in to comment.
Something went wrong with that request. Please try again.