Python 3 support, step 1: Get rid of cStringIO.

jonashaag · Feb 17, 2011 · 6b48d96 · 6b48d96
1 parent 3b38ec1
commit 6b48d96
Show file tree

Hide file tree

Showing 9 changed files with 160 additions and 70 deletions.
diff --git a/bjoern/bjoernmodule.c b/bjoern/bjoernmodule.c
@@ -1,6 +1,7 @@
 #include <Python.h>
 #include "server.h"
 #include "wsgi.h"
+#include "stream.h"
 #include "bjoernmodule.h"
 
 
@@ -73,12 +74,15 @@ run(PyObject* self, PyObject* args)
 static PyMethodDef Bjoern_FunctionTable[] = {
   {"run", run, METH_VARARGS, run_doc},
   {"listen", listen, METH_VARARGS, listen_doc},
-  {NULL,  NULL, 0, NULL}
+  {NULL, NULL, 0, NULL}
 };
 
 PyMODINIT_FUNC initbjoern()
 {
-  _initialize_wsgi_module();
+  PyType_Ready(&StartResponse_Type);
+  PyType_Ready(&StreamType);
+  assert(StreamType.tp_flags & Py_TPFLAGS_READY);
+  assert(StartResponse_Type.tp_flags & Py_TPFLAGS_READY);
   _initialize_static_strings();
 
   PyObject* bjoern_module = Py_InitModule("bjoern", Bjoern_FunctionTable);

diff --git a/bjoern/common.h b/bjoern/common.h
@@ -7,6 +7,8 @@
 #include <stdbool.h>
 #include <string.h>
 
+typedef struct { char* data; size_t len; } string;
+
 enum http_status { HTTP_BAD_REQUEST = 1, HTTP_LENGTH_REQUIRED, HTTP_SERVER_ERROR };
 
 size_t unquote_url_inplace(char* url, size_t len);

diff --git a/bjoern/request.c b/bjoern/request.c
@@ -1,20 +1,12 @@
 #include <Python.h>
-#include <cStringIO.h>
 #include "request.h"
+#include "stream.h"
 
 static inline void PyDict_ReplaceKey(PyObject* dict, PyObject* k1, PyObject* k2);
-static PyObject* wsgi_http_header(Request*, const char*, const size_t);
+static PyObject* wsgi_http_header(string header);
 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));
@@ -34,6 +26,7 @@ void Request_reset(Request* request)
 {
   memset(&request->state, 0, sizeof(Request) - (size_t)&((Request*)NULL)->state);
   request->state.response_length_unknown = true;
+  request->parser.body = (string){NULL, 0};
 }
 
 void Request_free(Request* request)
@@ -88,11 +81,11 @@ void Request_parse(Request* request, const char* data, const size_t data_len)
    *   [old header data ] ...stuff... [ new header data ]
    *   ^-------------- A -------------^--------B--------^
    *
-   * A = XXX- PARSER->XXX_start
+   * A = XXX- PARSER->XXX.data
    * B = len
    * A + B = old header start to new header end
    */ \
-  do { PARSER->name##_len = (name - PARSER->name##_start) + len; } while(0)
+  do { PARSER->name.len = (name - PARSER->name.data) + len; } while(0)
 
 #define _set_header(k, v) PyDict_SetItem(REQUEST->headers, k, v);
 #define _set_header_free_value(k, v) \
@@ -113,10 +106,8 @@ void Request_parse(Request* request, const char* data, const size_t data_len)
 static int on_message_begin(http_parser* parser)
 {
   REQUEST->headers = PyDict_New();
-  PARSER->field_start = NULL;
+  PARSER->field = (string){NULL, 0};
-  PARSER->field_len = 0;
+  PARSER->value = (string){NULL, 0};
-  PARSER->value_start = NULL;
-  PARSER->value_len = 0;
   return 0;
 }
 
@@ -136,43 +127,40 @@ static int on_query_string(http_parser* parser, const char* query, size_t len)
 
 static int on_header_field(http_parser* parser, const char* field, size_t len)
 {
-  if(PARSER->value_start) {
+  if(PARSER->value.data) {
     /* Store previous header and start a new one */
     _set_header_free_both(
-      wsgi_http_header(REQUEST, PARSER->field_start, PARSER->field_len),
+      wsgi_http_header(PARSER->field),
-      PyString_FromStringAndSize(PARSER->value_start, PARSER->value_len)
+      PyString_FromStringAndSize(PARSER->value.data, PARSER->value.len)
     );
-  } else if(PARSER->field_start) {
+  } else if(PARSER->field.data) {
     UPDATE_LENGTH(field);
     return 0;
   }
-  PARSER->field_start = field;
+  PARSER->field = (string){(char*)field, len};
-  PARSER->field_len = len;
+  PARSER->value = (string){NULL, 0};
-  PARSER->value_start = NULL;
-  PARSER->value_len = 0;
   return 0;
 }
 
 static int
 on_header_value(http_parser* parser, const char* value, size_t len)
 {
-  if(PARSER->value_start) {
+  if(PARSER->value.data) {
     UPDATE_LENGTH(value);
   } else {
     /* Start a new value */
-    PARSER->value_start = value;
+    PARSER->value = (string){(char*)value, len};
-    PARSER->value_len = len;
   }
   return 0;
 }
 
 static int
 on_headers_complete(http_parser* parser)
 {
-  if(PARSER->field_start) {
+  if(PARSER->field.data) {
     _set_header_free_both(
-      wsgi_http_header(REQUEST, PARSER->field_start, PARSER->field_len),
+      wsgi_http_header(PARSER->field),
-      PyString_FromStringAndSize(PARSER->value_start, PARSER->value_len)
+      PyString_FromStringAndSize(PARSER->value.data, PARSER->value.len)
     );
   }
   return 0;
@@ -181,24 +169,20 @@ on_headers_complete(http_parser* parser)
 static int
 on_body(http_parser* parser, const char* data, const size_t len)
 {
-  Iobject* body;
+  /* XXX does the body copying really make the code more readable? */
-
+  string body = PARSER->body;
-  body = (Iobject*)PyDict_GetItem(REQUEST->headers, _wsgi_input);
+  if(body.data == NULL) {
-  if(body == NULL) {
     if(!parser->content_length) {
       REQUEST->state.error_code = HTTP_LENGTH_REQUIRED;
       return 1;
     }
-    PyObject* buf = PyString_FromStringAndSize(NULL, parser->content_length);
+    body.data = malloc(parser->content_length);
-    body = (Iobject*)PycStringIO->NewInput(buf);
+    if(body.data == NULL)
-    Py_XDECREF(buf);
-    if(body == NULL)
       return 1;
-    _set_header(_wsgi_input, (PyObject*)body);
-    Py_DECREF(body);
   }
-  memcpy(body->buf + body->pos, data, len);
+  memcpy(body.data + body.len, data, len);
-  body->pos += len;
+  body.len += len;
+  PARSER->body = body;
   return 0;
 }
 
@@ -224,15 +208,10 @@ on_message_complete(http_parser* parser)
   /* REMOTE_ADDR */
   _set_header(_REMOTE_ADDR, REQUEST->client_addr);
 
-  PyObject* body = PyDict_GetItem(REQUEST->headers, _wsgi_input);
+  Stream* body = PyObject_NEW(Stream, &StreamType);
-  if(body) {
+  body->data = PARSER->body;
-    /* We abused the `pos` member for tracking the amount of data copied from
+  body->pos = 0;
-     * the buffer in on_body, so reset it to zero here. */
+  _set_header_free_value(_wsgi_input, (PyObject*)body);
-    ((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);
 
@@ -242,9 +221,9 @@ on_message_complete(http_parser* parser)
 
 
 static PyObject*
-wsgi_http_header(Request* request, const char* data, size_t len)
+wsgi_http_header(string header)
 {
-  PyObject* obj = PyString_FromStringAndSize(NULL, len+strlen("HTTP_"));
+  PyObject* obj = PyString_FromStringAndSize(NULL, header.len+strlen("HTTP_"));
   char* dest = PyString_AS_STRING(obj);
 
   *dest++ = 'H';
@@ -253,8 +232,8 @@ wsgi_http_header(Request* request, const char* data, size_t len)
   *dest++ = 'P';
   *dest++ = '_';
 
-  while(len--) {
+  while(header.len--) {
-    char c = *data++;
+    char c = *header.data++;
     if(c == '-')
       *dest++ = '_';
     else if(c >= 'a' && c <= 'z')
@@ -288,7 +267,6 @@ parser_settings = {
 void _initialize_request_module(const char* server_host, const int server_port)
 {
   if(wsgi_base_dict == NULL) {
-    PycString_IMPORT;
     wsgi_base_dict = PyDict_New();
 
     /* dct['SCRIPT_NAME'] = '' */

diff --git a/bjoern/request.h b/bjoern/request.h
@@ -19,10 +19,9 @@ typedef struct {
 
 typedef struct {
   http_parser parser;
-  const char* field_start;
+  string field;
-  size_t field_len;
+  string value;
-  const char* value_start;
+  string body;
-  size_t value_len;
 } bj_parser;
 
 typedef struct {

diff --git a/bjoern/stream.c b/bjoern/stream.c
@@ -0,0 +1,81 @@
+#include "stream.h"
+
+static PyObject*
+_Stream_read(Stream* stream, size_t howmuch)
+{
+  size_t bytesleft;
+  PyObject* chunk;
+  if(stream->data.data == NULL)
+    return NULL;
+  bytesleft = stream->data.len - stream->pos;
+  if(howmuch > bytesleft)
+    howmuch = bytesleft;
+  if(howmuch == 0)
+    return NULL;
+  chunk = PyString_FromStringAndSize(stream->data.data + stream->pos, howmuch);
+  stream->pos += howmuch;
+  return chunk;
+}
+
+static PyObject*
+Stream_read(PyObject* self, PyObject* args)
+{
+  size_t howmuch;
+  if(!PyArg_ParseTuple(args, "I", &howmuch))
+    return NULL;
+  PyObject* chunk = _Stream_read((Stream*)self, howmuch);
+  if(chunk == NULL) {
+    Py_INCREF(_empty_string);
+    chunk = _empty_string;
+  }
+  return chunk;
+}
+
+static PyObject*
+Stream_notimplemented(PyObject* self, PyObject* args)
+{
+  PyErr_SetString(PyExc_NotImplementedError,
+    "Nobody would ever use readline(s) on a stream of random bytes, right?");
+  return NULL;
+}
+
+static PyObject*
+Stream_iternew(PyObject* self) {
+  Py_INCREF(self);
+  return self;
+}
+
+static PyObject*
+Stream_iternext(PyObject* self)
+{
+  return _Stream_read((Stream*)self, STREAM_CHUNK_SIZE);
+}
+
+static void
+Stream_dealloc(PyObject* self)
+{
+  free(((Stream*)self)->data.data);
+  self->ob_type->tp_free(self);
+}
+
+static PyMethodDef Stream_methods[] = {
+  {"read", Stream_read, METH_VARARGS, NULL},
+  {"readline", Stream_notimplemented, METH_NOARGS, NULL},
+  {"readlines", Stream_notimplemented, METH_VARARGS, NULL},
+  {NULL, NULL, 0, NULL}
+};
+
+PyTypeObject StreamType = {
+  PyObject_HEAD_INIT(NULL)
+  0,
+  "StringStream",
+  sizeof(Stream),
+  0,
+  Stream_dealloc,
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+  Py_TPFLAGS_HAVE_CLASS | Py_TPFLAGS_HAVE_ITER,
+  0, 0, 0, 0, 0,
+  Stream_iternew,
+  Stream_iternext,
+  Stream_methods
+};
diff --git a/bjoern/stream.h b/bjoern/stream.h
@@ -0,0 +1,11 @@
+#include "common.h"
+
+#define STREAM_CHUNK_SIZE 4*1024 /* XXX this is kind of random */
+
+PyTypeObject StreamType;
+
+typedef struct {
+  PyObject_HEAD
+  string data;
+  size_t pos;
+} Stream;
diff --git a/bjoern/wsgi.c b/bjoern/wsgi.c
@@ -20,7 +20,6 @@ typedef struct {
 bool
 wsgi_call_application(Request* request)
 {
-  assert(StartResponse_Type.tp_flags & Py_TPFLAGS_READY);
   StartResponse* start_response = PyObject_NEW(StartResponse, &StartResponse_Type);
   start_response->request = request;
 
@@ -379,9 +378,3 @@ wrap_http_chunk_cruft_around(PyObject* chunk)
   assert(new_chunk_p == PyString_AS_STRING(new_chunk) + n + chunklen + 1);
   return new_chunk;
 }
-
-void _initialize_wsgi_module() {
-  int ready = PyType_Ready(&StartResponse_Type);
-  assert(ready == 0);
-  assert(StartResponse_Type.tp_flags & Py_TPFLAGS_READY);
-}
diff --git a/bjoern/wsgi.h b/bjoern/wsgi.h
@@ -1,7 +1,6 @@
 #include <Python.h>
 #include "request.h"
 
-void _initialize_wsgi_module();
 bool wsgi_call_application(Request*);
 PyObject* wsgi_iterable_get_next_chunk(Request*);
 PyObject* wrap_http_chunk_cruft_around(PyObject* chunk);

diff --git a/tests/readinput.py b/tests/readinput.py
@@ -0,0 +1,23 @@
+from __future__ import print_function
+import pprint
+import bjoern
+import sys
+
+if 'silenceillkillyou' in sys.argv:
+    def print(*args): pass
+
+def app(env, start_response):
+    print('--')
+    stream = env['wsgi.input']
+    print(stream)
+    print(dir(stream))
+    print(repr(stream.read(2)))
+    print(repr(stream.read(10)))
+    print(repr(stream.read(10)))
+    print(repr(stream.read(10)))
+    print(repr(stream.read(10)))
+    print(repr(stream.read(10)))
+    start_response('200 yo', [])
+    return []
+
+bjoern.run(app, '0.0.0.0', 8080)