THRIFT-3239 Limit recursion depth

Client: Haxe
Patch: Jens Geyer

This closes #547

7 files changed, 124 insertions, 98 deletions

diff --git a/lib/haxe/src/org/apache/thrift/protocol/TBinaryProtocol.hx b/lib/haxe/src/org/apache/thrift/protocol/TBinaryProtocol.hx
index 377e7ef4c..7ef291c0e 100644
--- a/lib/haxe/src/org/apache/thrift/protocol/TBinaryProtocol.hx
+++ b/lib/haxe/src/org/apache/thrift/protocol/TBinaryProtocol.hx
@@ -31,7 +31,7 @@ import org.apache.thrift.transport.TTransport;
 /**
 * Binary protocol implementation for thrift.
 */
-class TBinaryProtocol implements TProtocol {
+class TBinaryProtocol extends TRecursionTracker implements TProtocol {
 
     private static var ANONYMOUS_STRUCT:TStruct = new TStruct();
 
diff --git a/lib/haxe/src/org/apache/thrift/protocol/TCompactProtocol.hx b/lib/haxe/src/org/apache/thrift/protocol/TCompactProtocol.hx
index c4d0ced8d..03b13e2f6 100644
--- a/lib/haxe/src/org/apache/thrift/protocol/TCompactProtocol.hx
+++ b/lib/haxe/src/org/apache/thrift/protocol/TCompactProtocol.hx
@@ -37,7 +37,7 @@ import org.apache.thrift.helper.BitConverter;
 /**
 * Compact protocol implementation for thrift.
 */
-class TCompactProtocol implements TProtocol {
+class TCompactProtocol extends TRecursionTracker implements TProtocol {
 
     private static var ANONYMOUS_STRUCT : TStruct = new TStruct("");
     private static var TSTOP : TField = new TField("", TType.STOP, 0);
diff --git a/lib/haxe/src/org/apache/thrift/protocol/TJSONProtocol.hx b/lib/haxe/src/org/apache/thrift/protocol/TJSONProtocol.hx
index aeed8f451..e20ff33c5 100644
--- a/lib/haxe/src/org/apache/thrift/protocol/TJSONProtocol.hx
+++ b/lib/haxe/src/org/apache/thrift/protocol/TJSONProtocol.hx
@@ -45,7 +45,7 @@ import org.apache.thrift.transport.TTransport;
 *
 *  Adapted from the Java version.
 */
-class TJSONProtocol implements TProtocol {
+class TJSONProtocol extends TRecursionTracker implements TProtocol {
 
     public var trans(default,null) : TTransport;
 
diff --git a/lib/haxe/src/org/apache/thrift/protocol/TProtocol.hx b/lib/haxe/src/org/apache/thrift/protocol/TProtocol.hx
index 0998e92db..22e88e44b 100644
--- a/lib/haxe/src/org/apache/thrift/protocol/TProtocol.hx
+++ b/lib/haxe/src/org/apache/thrift/protocol/TProtocol.hx
@@ -79,4 +79,7 @@ interface TProtocol {
     function readString() : String;
     function readBinary() : Bytes;
 
+	// recursion tracking
+	function IncrementRecursionDepth() : Void;
+	function DecrementRecursionDepth() : Void;
 }
diff --git a/lib/haxe/src/org/apache/thrift/protocol/TProtocolUtil.hx b/lib/haxe/src/org/apache/thrift/protocol/TProtocolUtil.hx
index 794e397fd..71ed4ba36 100644
--- a/lib/haxe/src/org/apache/thrift/protocol/TProtocolUtil.hx
+++ b/lib/haxe/src/org/apache/thrift/protocol/TProtocolUtil.hx
@@ -29,107 +29,82 @@ import org.apache.thrift.*;
 class TProtocolUtil {
 
     /**
-     * The maximum recursive depth the skip() function will traverse before
-     * throwing a TException.
-     */
-    private static var maxSkipDepth : Int = Limits.I32_MAX;
-
-    /**
-     * Specifies the maximum recursive depth that the skip function will
-     * traverse before throwing a TException.  This is a global setting, so
-     * any call to skip in this JVM will enforce this value.
-     *
-     * @param depth  the maximum recursive depth.  A value of 2 would allow
-     *    the skip function to skip a structure or collection with basic children,
-     *    but it would not permit skipping a struct that had a field containing
-     *    a child struct.  A value of 1 would only allow skipping of simple
-     *    types and empty structs/collections.
-     */
-    public function setMaxSkipDepth(depth : Int) : Void {
-      maxSkipDepth = depth;
-    }
-
-    /**
      * Skips over the next data element from the provided input TProtocol object.
      *
      * @param prot  the protocol object to read from
      * @param type  the next value will be intepreted as this TType value.
      */
     public static function skip(prot:TProtocol, type : Int) : Void {
-      skipMaxDepth(prot, type, maxSkipDepth);
-    }
+		prot.IncrementRecursionDepth();
+		try
+		{
+			switch (type) {
+				case TType.BOOL: 
+					prot.readBool();
 
-     /**
-     * Skips over the next data element from the provided input TProtocol object.
-     *
-     * @param prot  the protocol object to read from
-     * @param type  the next value will be intepreted as this TType value.
-     * @param maxDepth  this function will only skip complex objects to this
-     *   recursive depth, to prevent Java stack overflow.
-     */
-    public static function skipMaxDepth(prot:TProtocol, type : Int, maxDepth : Int) : Void {
-      if (maxDepth <= 0) {
-        throw new TException("Maximum skip depth exceeded");
-      }
-      switch (type) {
-        case TType.BOOL: {
-          prot.readBool();
-        }
-        case TType.BYTE: {
-          prot.readByte();
-        }
-        case TType.I16: {
-          prot.readI16();
-        }
-        case TType.I32: {
-          prot.readI32();
-        }
-        case TType.I64: {
-          prot.readI64();
-        }
-        case TType.DOUBLE: {
-          prot.readDouble();
-        }
-        case TType.STRING: {
-          prot.readBinary();
-        }
-        case TType.STRUCT: {
-          prot.readStructBegin();
-          while (true) {
-            var field:TField = prot.readFieldBegin();
-            if (field.type == TType.STOP) {
-              break;
-            }
-            skipMaxDepth(prot, field.type, maxDepth - 1);
-            prot.readFieldEnd();
-          }
-          prot.readStructEnd();
-        }
-        case TType.MAP: {
-          var map:TMap = prot.readMapBegin();
-          for (i in 0 ... map.size) {
-            skipMaxDepth(prot, map.keyType, maxDepth - 1);
-            skipMaxDepth(prot, map.valueType, maxDepth - 1);
-          }
-          prot.readMapEnd();
-        }
-        case TType.SET: {
-          var set:TSet = prot.readSetBegin();
-          for (j in 0 ... set.size) {
-            skipMaxDepth(prot, set.elemType, maxDepth - 1);
-          }
-          prot.readSetEnd();
-        }
-        case TType.LIST: {
-          var list:TList = prot.readListBegin();
-          for (k in 0 ... list.size) {
-            skipMaxDepth(prot, list.elemType, maxDepth - 1);
-          }
-          prot.readListEnd();
-        }
-        default:
-          trace("Unknown field type ",type," in skipMaxDepth()");
-      }
+				case TType.BYTE: 
+					prot.readByte();
+
+				case TType.I16: 
+					prot.readI16();
+
+				case TType.I32: 
+					prot.readI32();
+
+				case TType.I64: 
+					prot.readI64();
+
+				case TType.DOUBLE: 
+					prot.readDouble();
+
+				case TType.STRING: 
+					prot.readBinary();
+
+				case TType.STRUCT: 
+					prot.readStructBegin();
+					while (true) {
+						var field:TField = prot.readFieldBegin();
+						if (field.type == TType.STOP) {
+						  break;
+						}
+						skip(prot, field.type);
+						prot.readFieldEnd();
+					}
+					prot.readStructEnd();
+
+				case TType.MAP: 
+					var map:TMap = prot.readMapBegin();
+					for (i in 0 ... map.size) {
+						skip(prot, map.keyType);
+						skip(prot, map.valueType);
+					}
+					prot.readMapEnd();
+
+				case TType.SET: 
+					var set:TSet = prot.readSetBegin();
+					for (j in 0 ... set.size) {
+						skip(prot, set.elemType);
+					}
+					prot.readSetEnd();
+
+				case TType.LIST: 
+					var list:TList = prot.readListBegin();
+					for (k in 0 ... list.size) {
+						skip(prot, list.elemType);
+					}
+					prot.readListEnd();
+
+				default:
+					trace("Unknown field type ",type," in skipMaxDepth()");
+			}
+			
+			prot.DecrementRecursionDepth();
+		}
+		catch(e:Dynamic)
+		{
+			prot.DecrementRecursionDepth();
+			throw e;
+		}
     }
 
 }
diff --git a/lib/haxe/src/org/apache/thrift/protocol/TRecursionTracker.hx b/lib/haxe/src/org/apache/thrift/protocol/TRecursionTracker.hx
new file mode 100644
index 000000000..b882cf21f
--- /dev/null
+++ b/lib/haxe/src/org/apache/thrift/protocol/TRecursionTracker.hx
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.thrift.protocol;
+
+import org.apache.thrift.*;
+
+
+class TRecursionTracker {
+
+	// default 
+    private static inline var DEFAULT_RECURSION_DEPTH : Int = 64; 
+
+	// limit and actual value
+	public var recursionLimit : Int = DEFAULT_RECURSION_DEPTH;
+    private var recursionDepth : Int = 0;
+
+	public function IncrementRecursionDepth() : Void 
+	{
+		if (recursionDepth < recursionLimit)
+			++recursionDepth;
+		else
+			throw new TProtocolException(TProtocolException.DEPTH_LIMIT, "Depth limit exceeded");
+	}
+
+	public function DecrementRecursionDepth() : Void 
+	{
+		--recursionDepth;
+	}
+
+
+}
diff --git a/lib/haxe/src/org/apache/thrift/server/TSimpleServer.hx b/lib/haxe/src/org/apache/thrift/server/TSimpleServer.hx
index f3408e272..3b64b62be 100644
--- a/lib/haxe/src/org/apache/thrift/server/TSimpleServer.hx
+++ b/lib/haxe/src/org/apache/thrift/server/TSimpleServer.hx
@@ -105,7 +105,7 @@ class TSimpleServer extends TServer  {
             }
             catch( pex : TProtocolException)
             {
-                logDelegate(pex); // Unexpected
+                logDelegate('$pex ${pex.errorID} ${pex.errorMsg}'); // Unexpected
             }
             catch( e : Dynamic)
             {