Repository: thrift Updated Branches: refs/heads/master 0104da5a6 -> 90c60e340
THRIFT-3239 Limit recursion depth Client: Haxe Patch: Jens Geyer This closes #547 Project: http://git-wip-us.apache.org/repos/asf/thrift/repo Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/90c60e34 Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/90c60e34 Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/90c60e34 Branch: refs/heads/master Commit: 90c60e340c322d398adc0de3ed45aed8d6f0c1f9 Parents: 0104da5 Author: Jens Geyer <[email protected]> Authored: Sat Jul 11 01:19:53 2015 +0200 Committer: Jens Geyer <[email protected]> Committed: Sat Jul 11 01:19:53 2015 +0200 ---------------------------------------------------------------------- compiler/cpp/src/generate/t_haxe_generator.cc | 54 +++++- .../apache/thrift/protocol/TBinaryProtocol.hx | 2 +- .../apache/thrift/protocol/TCompactProtocol.hx | 2 +- .../org/apache/thrift/protocol/TJSONProtocol.hx | 2 +- .../src/org/apache/thrift/protocol/TProtocol.hx | 3 + .../org/apache/thrift/protocol/TProtocolUtil.hx | 163 ++++++++----------- .../apache/thrift/protocol/TRecursionTracker.hx | 48 ++++++ .../org/apache/thrift/server/TSimpleServer.hx | 2 +- test/haxe/src/TestServer.hx | 2 +- 9 files changed, 170 insertions(+), 108 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/thrift/blob/90c60e34/compiler/cpp/src/generate/t_haxe_generator.cc ---------------------------------------------------------------------- diff --git a/compiler/cpp/src/generate/t_haxe_generator.cc b/compiler/cpp/src/generate/t_haxe_generator.cc index a0e2f28..dfa36c5 100644 --- a/compiler/cpp/src/generate/t_haxe_generator.cc +++ b/compiler/cpp/src/generate/t_haxe_generator.cc @@ -818,6 +818,10 @@ void t_haxe_generator::generate_haxe_struct_reader(ofstream& out, t_struct* tstr const vector<t_field*>& fields = tstruct->get_members(); vector<t_field*>::const_iterator f_iter; + indent(out) << "iprot.IncrementRecursionDepth();" << endl; + indent(out) << "try" << endl; + scope_up(out); + // Declare stack tmp variables and read struct header out << indent() << "var field : TField;" << endl << indent() << "iprot.readStructBegin();" << endl; @@ -869,6 +873,14 @@ void t_haxe_generator::generate_haxe_struct_reader(ofstream& out, t_struct* tstr out << indent() << "iprot.readStructEnd();" << endl << endl; + indent(out) << "iprot.DecrementRecursionDepth();" << endl; + scope_down(out); + indent(out) << "catch(e:Dynamic)" << endl; + scope_up(out); + indent(out) << "iprot.DecrementRecursionDepth();" << endl; + indent(out) << "throw e;" << endl; + scope_down(out); + // check for required fields of primitive type // (which can be checked here but not in the general validate method) out << endl << indent() << "// check for required fields of primitive type, which can't be " @@ -952,7 +964,10 @@ void t_haxe_generator::generate_haxe_struct_writer(ofstream& out, t_struct* tstr vector<t_field*>::const_iterator f_iter; // performs various checks (e.g. check that all required fields are set) - indent(out) << "validate();" << endl << endl; + indent(out) << "validate();" << endl; + indent(out) << "oprot.IncrementRecursionDepth();" << endl; + indent(out) << "try" << endl; + scope_up(out); indent(out) << "oprot.writeStructBegin(STRUCT_DESC);" << endl; @@ -977,10 +992,18 @@ void t_haxe_generator::generate_haxe_struct_writer(ofstream& out, t_struct* tstr indent(out) << "}" << endl; } } - // Write the struct map - out << indent() << "oprot.writeFieldStop();" << endl << indent() << "oprot.writeStructEnd();" - << endl; - + + indent(out) << "oprot.writeFieldStop();" << endl; + indent(out) << "oprot.writeStructEnd();" << endl; + + indent(out) << "oprot.DecrementRecursionDepth();" << endl; + scope_down(out); + indent(out) << "catch(e:Dynamic)" << endl; + scope_up(out); + indent(out) << "oprot.DecrementRecursionDepth();" << endl; + indent(out) << "throw e;" << endl; + scope_down(out); + indent_down(); out << indent() << "}" << endl << endl; } @@ -1001,6 +1024,10 @@ void t_haxe_generator::generate_haxe_struct_result_writer(ofstream& out, t_struc const vector<t_field*>& fields = tstruct->get_sorted_members(); vector<t_field*>::const_iterator f_iter; + indent(out) << "oprot.IncrementRecursionDepth();" << endl; + indent(out) << "try" << endl; + scope_up(out); + indent(out) << "oprot.writeStructBegin(STRUCT_DESC);" << endl; bool first = true; @@ -1028,10 +1055,19 @@ void t_haxe_generator::generate_haxe_struct_result_writer(ofstream& out, t_struc indent_down(); indent(out) << "}"; } - // Write the struct map - out << endl << indent() << "oprot.writeFieldStop();" << endl << indent() - << "oprot.writeStructEnd();" << endl; - + + indent(out) << endl; + indent(out) << "oprot.writeFieldStop();" << endl; + indent(out) << "oprot.writeStructEnd();" << endl; + + indent(out) << "oprot.DecrementRecursionDepth();" << endl; + scope_down(out); + indent(out) << "catch(e:Dynamic)" << endl; + scope_up(out); + indent(out) << "oprot.DecrementRecursionDepth();" << endl; + indent(out) << "throw e;" << endl; + scope_down(out); + indent_down(); out << indent() << "}" << endl << endl; } http://git-wip-us.apache.org/repos/asf/thrift/blob/90c60e34/lib/haxe/src/org/apache/thrift/protocol/TBinaryProtocol.hx ---------------------------------------------------------------------- diff --git a/lib/haxe/src/org/apache/thrift/protocol/TBinaryProtocol.hx b/lib/haxe/src/org/apache/thrift/protocol/TBinaryProtocol.hx index 377e7ef..7ef291c 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(); http://git-wip-us.apache.org/repos/asf/thrift/blob/90c60e34/lib/haxe/src/org/apache/thrift/protocol/TCompactProtocol.hx ---------------------------------------------------------------------- diff --git a/lib/haxe/src/org/apache/thrift/protocol/TCompactProtocol.hx b/lib/haxe/src/org/apache/thrift/protocol/TCompactProtocol.hx index c4d0ced..03b13e2 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); http://git-wip-us.apache.org/repos/asf/thrift/blob/90c60e34/lib/haxe/src/org/apache/thrift/protocol/TJSONProtocol.hx ---------------------------------------------------------------------- diff --git a/lib/haxe/src/org/apache/thrift/protocol/TJSONProtocol.hx b/lib/haxe/src/org/apache/thrift/protocol/TJSONProtocol.hx index aeed8f4..e20ff33 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; http://git-wip-us.apache.org/repos/asf/thrift/blob/90c60e34/lib/haxe/src/org/apache/thrift/protocol/TProtocol.hx ---------------------------------------------------------------------- diff --git a/lib/haxe/src/org/apache/thrift/protocol/TProtocol.hx b/lib/haxe/src/org/apache/thrift/protocol/TProtocol.hx index 0998e92..22e88e4 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; } http://git-wip-us.apache.org/repos/asf/thrift/blob/90c60e34/lib/haxe/src/org/apache/thrift/protocol/TProtocolUtil.hx ---------------------------------------------------------------------- diff --git a/lib/haxe/src/org/apache/thrift/protocol/TProtocolUtil.hx b/lib/haxe/src/org/apache/thrift/protocol/TProtocolUtil.hx index 794e397..71ed4ba 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; + } } } http://git-wip-us.apache.org/repos/asf/thrift/blob/90c60e34/lib/haxe/src/org/apache/thrift/protocol/TRecursionTracker.hx ---------------------------------------------------------------------- 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 0000000..b882cf2 --- /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; + } + + +} http://git-wip-us.apache.org/repos/asf/thrift/blob/90c60e34/lib/haxe/src/org/apache/thrift/server/TSimpleServer.hx ---------------------------------------------------------------------- diff --git a/lib/haxe/src/org/apache/thrift/server/TSimpleServer.hx b/lib/haxe/src/org/apache/thrift/server/TSimpleServer.hx index f3408e2..3b64b62 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) { http://git-wip-us.apache.org/repos/asf/thrift/blob/90c60e34/test/haxe/src/TestServer.hx ---------------------------------------------------------------------- diff --git a/test/haxe/src/TestServer.hx b/test/haxe/src/TestServer.hx index 4490a8c..bff5a47 100644 --- a/test/haxe/src/TestServer.hx +++ b/test/haxe/src/TestServer.hx @@ -106,7 +106,7 @@ class TestServer } catch (x : TException) { - trace('$x'); + trace('$x ${x.errorID} ${x.errorMsg}'); } catch (x : Dynamic) {
