From 6b97e28f594dfe3d87c337aeaf1ab5b5f3dfa927 Mon Sep 17 00:00:00 2001
From: fairwinds <fairwinds.dp@gmail.com>
Date: Sat, 27 Oct 2012 09:32:21 +0530
Subject: [PATCH] [TIDESK-574] Fix HTTPServer crashing issue. Before we used
 the HTTPServerRequestHandler for the HTTPRequest object we exposed into
 Kroll. The issue is this object's lifetime is restricted by Poco to the
 handleRequest callback. Poco deletes this right after this method returns.
 Once the GC of the interpreter runs it will try cleaning up this HTTPRequest
 native object, but it's already deleted thus a crash results.

Moved HTTPRequest into it's own class to avoid the limited lifetime of the handler object.
Note: we may see issues with the request/response object if they are accessed outside the
callback's scope. Poco will most likely delete this after the handleRequest callback as well.
---
 .../protocols/http/http_server_request.cpp    | 19 +------------
 .../protocols/http/http_server_request.h      |  8 ++----
 .../http/http_server_request_factory.cpp      | 28 +++++++++++++++++--
 3 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/modules/ti.Network/protocols/http/http_server_request.cpp b/modules/ti.Network/protocols/http/http_server_request.cpp
index cc2fc94e..626ed382 100644
--- a/modules/ti.Network/protocols/http/http_server_request.cpp
+++ b/modules/ti.Network/protocols/http/http_server_request.cpp
@@ -10,11 +10,8 @@
 
 namespace ti
 {
-	HttpServerRequest::HttpServerRequest(Host *host, KMethodRef callback, 
-		Poco::Net::HTTPServerRequest& request) :
+	HttpServerRequest::HttpServerRequest(Poco::Net::HTTPServerRequest& request) :
 			StaticBoundObject("Network.HttpServerRequest"),
-			host(host),
-			callback(callback),
 			request(request)
 	{
 		/**
@@ -80,20 +77,6 @@ namespace ti
 	{
 	}
 
-	void HttpServerRequest::handleRequest(Poco::Net::HTTPServerRequest &request, Poco::Net::HTTPServerResponse &response)
-	{
-		ValueList args;
-
-		// We MUST duplicate 'this' before casting it into an
-		// AutoPtr or else we will free this memory at the wrong time.
-		this->duplicate();
-		AutoPtr<HttpServerRequest> autoThis = this;
-
-		args.push_back(Value::NewObject(autoThis));
-		args.push_back(Value::NewObject(new HttpServerResponse(response)));
-		RunOnMainThread(callback, args);
-	}
-
 	void HttpServerRequest::GetMethod(const ValueList& args, KValueRef result)
 	{
 		std::string method = request.getMethod();
diff --git a/modules/ti.Network/protocols/http/http_server_request.h b/modules/ti.Network/protocols/http/http_server_request.h
index 4b4740f0..e6eb401f 100644
--- a/modules/ti.Network/protocols/http/http_server_request.h
+++ b/modules/ti.Network/protocols/http/http_server_request.h
@@ -14,17 +14,13 @@
 
 namespace ti
 {
-	class HttpServerRequest : public Poco::Net::HTTPRequestHandler, public StaticBoundObject
+	class HttpServerRequest : public StaticBoundObject
 	{
 	public:
-		HttpServerRequest(Host *host, KMethodRef callback, Poco::Net::HTTPServerRequest& request);
+		HttpServerRequest(Poco::Net::HTTPServerRequest& request);
 		virtual ~HttpServerRequest();
 
-		void handleRequest(Poco::Net::HTTPServerRequest &request, Poco::Net::HTTPServerResponse& response);
-
 	private:
-		Host *host;
-		KMethodRef callback;
 		Poco::Net::HTTPServerRequest& request;
 
 		void GetMethod(const ValueList& args, KValueRef result);
diff --git a/modules/ti.Network/protocols/http/http_server_request_factory.cpp b/modules/ti.Network/protocols/http/http_server_request_factory.cpp
index 20d5d2a9..f2682f07 100644
--- a/modules/ti.Network/protocols/http/http_server_request_factory.cpp
+++ b/modules/ti.Network/protocols/http/http_server_request_factory.cpp
@@ -5,10 +5,34 @@
  */
 
 #include "http_server_request.h"
+#include "http_server_response.h"
 #include "http_server_request_factory.h"
 
 namespace ti
 {
+	class HTTPRequestHandler : public Poco::Net::HTTPRequestHandler {
+		public:
+			HTTPRequestHandler(KMethodRef callback)
+				: m_callback(callback)
+			{
+			}
+
+			virtual void handleRequest(Poco::Net::HTTPServerRequest&, Poco::Net::HTTPServerResponse&);
+
+		private:
+			KMethodRef m_callback;
+	};
+
+	void HTTPRequestHandler::handleRequest(Poco::Net::HTTPServerRequest& request, Poco::Net::HTTPServerResponse& response) {
+		// XXX(Josh): The request and response object's lifetime is limited to this functions call.
+		// If the developer should keep a reference to these around past the callback lifetime and then
+		// attempts to access it may result in a crash!
+		ValueList args;
+		args.push_back(Value::NewObject(new HttpServerRequest(request)));
+		args.push_back(Value::NewObject(new HttpServerResponse(response)));
+		RunOnMainThread(m_callback, args);
+	}
+
 	HttpServerRequestFactory::HttpServerRequestFactory(Host *host, KMethodRef callback) :
 		host(host),
 		callback(callback)
@@ -20,8 +44,8 @@ namespace ti
 	}
 
 	Poco::Net::HTTPRequestHandler* HttpServerRequestFactory::createRequestHandler(
-		const Poco::Net::HTTPServerRequest& request)
+			const Poco::Net::HTTPServerRequest& request)
 	{
-		return new HttpServerRequest(host, callback, const_cast<Poco::Net::HTTPServerRequest&>(request));
+		return new HTTPRequestHandler(callback);
 	}
 }