0001-Potential-fix-for-2438.patch

From c9cee2d14428daacaf4e0188803c26a39b809958 Mon Sep 17 00:00:00 2001
From: Ryan Dahl <ry@tinyclouds.org>
Date: Thu, 29 Dec 2011 16:06:14 -0800
Subject: [PATCH] Potential fix for #2438

version 2
---
 src/node_http_parser.cc         |   61 ++++++++++++++++++++++++++++-----------
 test/simple/test-http-parser.js |    6 ++-
 2 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
index 38251a1..90762e7 100644
--- a/src/node_http_parser.cc
+++ b/src/node_http_parser.cc
@@ -103,27 +103,12 @@ static char* current_buffer_data;
 static size_t current_buffer_len;
 
 
-// gcc 3.x knows the always_inline attribute but fails at build time with a
-// "sorry, unimplemented: inlining failed" error when compiling at -O0
-#if defined(__GNUC__)
-# if __GNUC__ >= 4
-#  define always_inline __attribute__((always_inline))
-# else
-#  define always_inline inline
-# endif
-#elif defined(_MSC_VER)
-# define always_inline __forceinline
-#else
-# define always_inline
-#endif
-
-
 #define HTTP_CB(name)                                               \
 	  static int name(http_parser* p_) {                              \
 	    Parser* self = container_of(p_, Parser, parser_);             \
 	    return self->name##_();                                       \
 	  }                                                               \
-	  int always_inline name##_()
+	  int name##_()
 
 
 #define HTTP_DATA_CB(name)                                          \
@@ -131,7 +116,7 @@ static size_t current_buffer_len;
     Parser* self = container_of(p_, Parser, parser_);               \
     return self->name##_(at, length);                               \
   }                                                                 \
-  int always_inline name##_(const char* at, size_t length)
+  int name##_(const char* at, size_t length)
 
 
 static inline Persistent<String>
@@ -179,6 +164,19 @@ struct StringPtr {
   }
 
 
+  // If str_ does not point to a heap string yet, this function makes it do
+  // so. This is called at the end of each http_parser_execute() so as not
+  // to leak references. See issue #2438 and test-http-parser-bad-ref.js.
+  void Save() {
+    if (!on_heap_) {
+      char* s = new char[size_];
+      memcpy(s, str_, size_);
+      str_ = s;
+      on_heap_ = true;
+    }
+  }
+
+
   void Reset() {
     if (on_heap_) {
       delete[] str_;
@@ -258,6 +256,14 @@ public:
         num_values_ = -1;
       }
       fields_[num_fields_].Reset();
+    } else {
+      // Old field name. This was in a previous packet because we know that
+      // http_parser_execute will not emit more than one on_header_field
+      // callback per packet. Therefore we must have called Save on it.
+      // Therefore on_heap_ must be true.
+      if (num_fields_ >= 0) {
+        assert(fields_[num_fields_].on_heap_);
+      }
     }
 
     assert(num_fields_ < (int)ARRAY_SIZE(fields_));
@@ -273,6 +279,12 @@ public:
     if (num_values_ != num_fields_) {
       // start of new header value
       values_[++num_values_].Reset();
+    } else {
+      // Old value name. This was in a previous packet because we know that
+      // http_parser_execute will not emit more than one on_header_value
+      // callback per packet. Therefore we must have called Save on it.
+      // Therefore on_heap_ must be true.
+      assert(values_[num_values_].on_heap_);
     }
 
     assert(num_values_ < (int)ARRAY_SIZE(values_));
@@ -401,6 +413,19 @@ public:
   }
 
 
+  void Save() {
+    url_.Save();
+
+    for (int i = 0; i <= num_fields_; i++) {
+      fields_[i].Save();
+    }
+
+    for (int i = 0; i <= num_values_; i++) {
+      values_[i].Save();
+    }
+  }
+
+
   // var bytesParsed = parser->execute(buffer, off, len);
   static Handle<Value> Execute(const Arguments& args) {
     HandleScope scope;
@@ -447,6 +472,8 @@ public:
     size_t nparsed =
       http_parser_execute(&parser->parser_, &settings, buffer_data + off, len);
 
+    parser->Save();
+
     // Unassign the 'buffer_' variable
     assert(current_buffer);
     current_buffer = NULL;
diff --git a/test/simple/test-http-parser.js b/test/simple/test-http-parser.js
index 086d53b..7dc1504 100644
--- a/test/simple/test-http-parser.js
+++ b/test/simple/test-http-parser.js
@@ -381,7 +381,7 @@ function expectBody(expected) {
 //
 (function() {
   var request = Buffer(
-    'POST /it HTTP/1.1' + CRLF +
+    'POST /helpme HTTP/1.1' + CRLF +
     'Content-Type: text/plain' + CRLF +
     'Transfer-Encoding: chunked' + CRLF +
     CRLF +
@@ -403,7 +403,7 @@ function expectBody(expected) {
 
     parser.onHeadersComplete = mustCall(function(info) {
       assert.equal(info.method, 'POST');
-      assert.equal(info.url || parser.url, '/it');
+      assert.equal(info.url || parser.url, '/helpme');
       assert.equal(info.versionMajor, 1);
       assert.equal(info.versionMinor, 1);
     });
@@ -424,7 +424,9 @@ function expectBody(expected) {
 
   for (var i = 1; i < request.length - 1; ++i) {
     var a = request.slice(0, i);
+    console.error("request.slice(0, " + i + ") = ", JSON.stringify(a.toString()));
     var b = request.slice(i);
+    console.error("request.slice(" + i + ") = ", JSON.stringify(b.toString()));
     test(a, b);
   }
 })();
-- 
1.7.2