You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by jk...@apache.org on 2019/02/14 21:46:47 UTC
[thrift] branch master updated: THRIFT-4024,
THRIFT-4783: throw when skipping invalid type (#1742)
This is an automated email from the ASF dual-hosted git repository.
jking pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/thrift.git
The following commit(s) were added to refs/heads/master by this push:
new dbc1f8d THRIFT-4024, THRIFT-4783: throw when skipping invalid type (#1742)
dbc1f8d is described below
commit dbc1f8def5018ce5d85d38b9875c6c6b6b424478
Author: James E. King III <jk...@apache.org>
AuthorDate: Thu Feb 14 16:46:38 2019 -0500
THRIFT-4024, THRIFT-4783: throw when skipping invalid type (#1742)
* THRIFT-4024: make c_glib throw on unsupported type when skipping
* THRIFT-4783: throw on invalid skip (py)
* THRIFT-4024: make cpp throw on unsupported type when skipping
* THRIFT-4024: uniform skip behavior on unsupported type
---
.../src/thrift/c_glib/protocol/thrift_protocol.c | 72 ++++++++++++++--------
lib/cpp/src/thrift/protocol/TProtocol.h | 12 ++--
lib/d/src/thrift/protocol/base.d | 8 +--
lib/dart/lib/src/protocol/t_protocol_util.dart | 2 +-
lib/go/thrift/protocol.go | 2 -
.../org/apache/thrift/protocol/TProtocolUtil.java | 3 +-
lib/js/src/thrift.js | 3 -
lib/lua/TProtocol.lua | 8 ++-
lib/nodejs/lib/thrift/binary_protocol.js | 2 -
lib/nodejs/lib/thrift/compact_protocol.js | 2 -
lib/nodejs/lib/thrift/json_protocol.js | 2 -
lib/ocaml/src/Thrift.ml | 3 +-
lib/py/src/protocol/TProtocol.py | 8 ++-
lib/rb/lib/thrift/protocol/base_protocol.rb | 4 +-
lib/rb/spec/base_protocol_spec.rb | 1 -
lib/swift/Sources/TProtocol.swift | 3 +-
16 files changed, 73 insertions(+), 62 deletions(-)
diff --git a/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c b/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c
index 8296a8c..6e6ae4d 100644
--- a/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c
+++ b/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c
@@ -419,6 +419,13 @@ thrift_protocol_read_binary (ThriftProtocol *protocol, gpointer *buf,
len, error);
}
+#define THRIFT_SKIP_RESULT_OR_RETURN(_RES, _CALL) \
+ { \
+ gint32 _x = (_CALL); \
+ if (_x < 0) { return _x; } \
+ (_RES) += _x; \
+ }
+
gint32
thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error)
{
@@ -469,24 +476,24 @@ thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error)
gchar *name;
gint16 fid;
ThriftType ftype;
- result += thrift_protocol_read_struct_begin (protocol, &name, error);
-
+ THRIFT_SKIP_RESULT_OR_RETURN(result,
+ thrift_protocol_read_struct_begin (protocol, &name, error))
while (1)
{
- result += thrift_protocol_read_field_begin (protocol, &name, &ftype,
- &fid, error);
- if (result < 0)
- {
- return result;
- }
+ THRIFT_SKIP_RESULT_OR_RETURN(result,
+ thrift_protocol_read_field_begin (protocol, &name, &ftype,
+ &fid, error))
if (ftype == T_STOP)
{
break;
}
- result += thrift_protocol_skip (protocol, ftype, error);
- result += thrift_protocol_read_field_end (protocol, error);
+ THRIFT_SKIP_RESULT_OR_RETURN(result,
+ thrift_protocol_skip (protocol, ftype, error))
+ THRIFT_SKIP_RESULT_OR_RETURN(result,
+ thrift_protocol_read_field_end (protocol, error))
}
- result += thrift_protocol_read_struct_end (protocol, error);
+ THRIFT_SKIP_RESULT_OR_RETURN(result,
+ thrift_protocol_read_struct_end (protocol, error))
return result;
}
case T_SET:
@@ -494,13 +501,16 @@ thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error)
gint32 result = 0;
ThriftType elem_type;
guint32 i, size;
- result += thrift_protocol_read_set_begin (protocol, &elem_type, &size,
- error);
+ THRIFT_SKIP_RESULT_OR_RETURN(result,
+ thrift_protocol_read_set_begin (protocol, &elem_type, &size,
+ error))
for (i = 0; i < size; i++)
{
- result += thrift_protocol_skip (protocol, elem_type, error);
+ THRIFT_SKIP_RESULT_OR_RETURN(result,
+ thrift_protocol_skip (protocol, elem_type, error))
}
- result += thrift_protocol_read_set_end (protocol, error);
+ THRIFT_SKIP_RESULT_OR_RETURN(result,
+ thrift_protocol_read_set_end (protocol, error))
return result;
}
case T_MAP:
@@ -509,14 +519,18 @@ thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error)
ThriftType elem_type;
ThriftType key_type;
guint32 i, size;
- result += thrift_protocol_read_map_begin (protocol, &key_type, &elem_type, &size,
- error);
+ THRIFT_SKIP_RESULT_OR_RETURN(result,
+ thrift_protocol_read_map_begin (protocol, &key_type, &elem_type, &size,
+ error))
for (i = 0; i < size; i++)
{
- result += thrift_protocol_skip (protocol, key_type, error);
- result += thrift_protocol_skip (protocol, elem_type, error);
+ THRIFT_SKIP_RESULT_OR_RETURN(result,
+ thrift_protocol_skip (protocol, key_type, error))
+ THRIFT_SKIP_RESULT_OR_RETURN(result,
+ thrift_protocol_skip (protocol, elem_type, error))
}
- result += thrift_protocol_read_map_end (protocol, error);
+ THRIFT_SKIP_RESULT_OR_RETURN(result,
+ thrift_protocol_read_map_end (protocol, error))
return result;
}
case T_LIST:
@@ -524,18 +538,26 @@ thrift_protocol_skip (ThriftProtocol *protocol, ThriftType type, GError **error)
gint32 result = 0;
ThriftType elem_type;
guint32 i, size;
- result += thrift_protocol_read_list_begin (protocol, &elem_type, &size,
- error);
+ THRIFT_SKIP_RESULT_OR_RETURN(result,
+ thrift_protocol_read_list_begin (protocol, &elem_type, &size,
+ error))
for (i = 0; i < size; i++)
{
- result += thrift_protocol_skip (protocol, elem_type, error);
+ THRIFT_SKIP_RESULT_OR_RETURN(result,
+ thrift_protocol_skip (protocol, elem_type, error))
}
- result += thrift_protocol_read_list_end (protocol, error);
+ THRIFT_SKIP_RESULT_OR_RETURN(result,
+ thrift_protocol_read_list_end (protocol, error))
return result;
}
default:
- return 0;
+ break;
}
+
+ g_set_error (error, THRIFT_PROTOCOL_ERROR,
+ THRIFT_PROTOCOL_ERROR_INVALID_DATA,
+ "unrecognized type");
+ return -1;
}
/* define the GError domain for Thrift protocols */
diff --git a/lib/cpp/src/thrift/protocol/TProtocol.h b/lib/cpp/src/thrift/protocol/TProtocol.h
index a38660f..7566a25 100644
--- a/lib/cpp/src/thrift/protocol/TProtocol.h
+++ b/lib/cpp/src/thrift/protocol/TProtocol.h
@@ -746,16 +746,12 @@ uint32_t skip(Protocol_& prot, TType type) {
result += prot.readListEnd();
return result;
}
- case T_STOP:
- case T_VOID:
- case T_U64:
- case T_UTF8:
- case T_UTF16:
- break;
default:
- throw TProtocolException(TProtocolException::INVALID_DATA);
+ break;
}
- return 0;
+
+ throw TProtocolException(TProtocolException::INVALID_DATA,
+ "invalid TType");
}
}}} // apache::thrift::protocol
diff --git a/lib/d/src/thrift/protocol/base.d b/lib/d/src/thrift/protocol/base.d
index 70648b3..5b6d845 100644
--- a/lib/d/src/thrift/protocol/base.d
+++ b/lib/d/src/thrift/protocol/base.d
@@ -260,7 +260,7 @@ protected:
* in generated code.
*/
void skip(Protocol)(Protocol prot, TType type) if (is(Protocol : TProtocol)) {
- final switch (type) {
+ switch (type) {
case TType.BOOL:
prot.readBool();
break;
@@ -324,9 +324,9 @@ void skip(Protocol)(Protocol prot, TType type) if (is(Protocol : TProtocol)) {
}
prot.readSetEnd();
break;
- case TType.STOP: goto case;
- case TType.VOID:
- assert(false, "Invalid field type passed.");
+
+ default:
+ throw new TProtocolException(TProtocolException.Type.INVALID_DATA);
}
}
diff --git a/lib/dart/lib/src/protocol/t_protocol_util.dart b/lib/dart/lib/src/protocol/t_protocol_util.dart
index ad20068..841ea82 100644
--- a/lib/dart/lib/src/protocol/t_protocol_util.dart
+++ b/lib/dart/lib/src/protocol/t_protocol_util.dart
@@ -101,7 +101,7 @@ class TProtocolUtil {
break;
default:
- break;
+ throw new TProtocolError(TProtocolErrorType.INVALID_DATA, "Invalid data");
}
}
}
diff --git a/lib/go/thrift/protocol.go b/lib/go/thrift/protocol.go
index 615b7a4..2e6bc4b 100644
--- a/lib/go/thrift/protocol.go
+++ b/lib/go/thrift/protocol.go
@@ -96,8 +96,6 @@ func Skip(self TProtocol, fieldType TType, maxDepth int) (err error) {
}
switch fieldType {
- case STOP:
- return
case BOOL:
_, err = self.ReadBool()
return
diff --git a/lib/javame/src/org/apache/thrift/protocol/TProtocolUtil.java b/lib/javame/src/org/apache/thrift/protocol/TProtocolUtil.java
index 9bf10f6..c327448 100644
--- a/lib/javame/src/org/apache/thrift/protocol/TProtocolUtil.java
+++ b/lib/javame/src/org/apache/thrift/protocol/TProtocolUtil.java
@@ -152,7 +152,8 @@ public class TProtocolUtil {
break;
}
default:
- break;
+ throw new TProtocolException(TProtocolException.INVALID_DATA,
+ "Unrecognized type " + type);
}
}
}
diff --git a/lib/js/src/thrift.js b/lib/js/src/thrift.js
index 0b4a292..21a3d65 100644
--- a/lib/js/src/thrift.js
+++ b/lib/js/src/thrift.js
@@ -1434,9 +1434,6 @@ Thrift.Protocol.prototype = {
skip: function(type) {
var ret, i;
switch (type) {
- case Thrift.Type.STOP:
- return null;
-
case Thrift.Type.BOOL:
return this.readBool();
diff --git a/lib/lua/TProtocol.lua b/lib/lua/TProtocol.lua
index 616e167..1306fb3 100644
--- a/lib/lua/TProtocol.lua
+++ b/lib/lua/TProtocol.lua
@@ -107,9 +107,7 @@ function TProtocolBase:readDouble() end
function TProtocolBase:readString() end
function TProtocolBase:skip(ttype)
- if type == TType.STOP then
- return
- elseif ttype == TType.BOOL then
+ if ttype == TType.BOOL then
self:readBool()
elseif ttype == TType.BYTE then
self:readByte()
@@ -153,6 +151,10 @@ function TProtocolBase:skip(ttype)
self:skip(ettype)
end
self:readListEnd()
+ else
+ terror(TProtocolException:new{
+ message = 'Invalid data'
+ })
end
end
diff --git a/lib/nodejs/lib/thrift/binary_protocol.js b/lib/nodejs/lib/thrift/binary_protocol.js
index b57c8c5..6ab9c05 100644
--- a/lib/nodejs/lib/thrift/binary_protocol.js
+++ b/lib/nodejs/lib/thrift/binary_protocol.js
@@ -302,8 +302,6 @@ TBinaryProtocol.prototype.getTransport = function() {
TBinaryProtocol.prototype.skip = function(type) {
switch (type) {
- case Type.STOP:
- return;
case Type.BOOL:
this.readBool();
break;
diff --git a/lib/nodejs/lib/thrift/compact_protocol.js b/lib/nodejs/lib/thrift/compact_protocol.js
index 5c531e5..302a88d 100644
--- a/lib/nodejs/lib/thrift/compact_protocol.js
+++ b/lib/nodejs/lib/thrift/compact_protocol.js
@@ -854,8 +854,6 @@ TCompactProtocol.prototype.zigzagToI64 = function(n) {
TCompactProtocol.prototype.skip = function(type) {
switch (type) {
- case Type.STOP:
- return;
case Type.BOOL:
this.readBool();
break;
diff --git a/lib/nodejs/lib/thrift/json_protocol.js b/lib/nodejs/lib/thrift/json_protocol.js
index 727a3b2..7e2b7c9 100644
--- a/lib/nodejs/lib/thrift/json_protocol.js
+++ b/lib/nodejs/lib/thrift/json_protocol.js
@@ -738,8 +738,6 @@ TJSONProtocol.prototype.getTransport = function() {
*/
TJSONProtocol.prototype.skip = function(type) {
switch (type) {
- case Type.STOP:
- return;
case Type.BOOL:
this.readBool();
break;
diff --git a/lib/ocaml/src/Thrift.ml b/lib/ocaml/src/Thrift.ml
index f0d7a42..063459b 100644
--- a/lib/ocaml/src/Thrift.ml
+++ b/lib/ocaml/src/Thrift.ml
@@ -206,8 +206,6 @@ struct
(* skippage *)
method skip typ =
match typ with
- | T_STOP -> ()
- | T_VOID -> ()
| T_BOOL -> ignore self#readBool
| T_BYTE
| T_I08 -> ignore self#readByte
@@ -248,6 +246,7 @@ struct
self#readListEnd)
| T_UTF8 -> ()
| T_UTF16 -> ()
+ | _ -> raise (Protocol.E (Protocol.INVALID_DATA, "Invalid data"))
end
class virtual factory =
diff --git a/lib/py/src/protocol/TProtocol.py b/lib/py/src/protocol/TProtocol.py
index 8314cf6..3456e8f 100644
--- a/lib/py/src/protocol/TProtocol.py
+++ b/lib/py/src/protocol/TProtocol.py
@@ -191,9 +191,7 @@ class TProtocolBase(object):
return self.readString().decode('utf8')
def skip(self, ttype):
- if ttype == TType.STOP:
- return
- elif ttype == TType.BOOL:
+ if ttype == TType.BOOL:
self.readBool()
elif ttype == TType.BYTE:
self.readByte()
@@ -232,6 +230,10 @@ class TProtocolBase(object):
for i in range(size):
self.skip(etype)
self.readListEnd()
+ else:
+ raise TProtocolException(
+ TProtocolException.INVALID_DATA,
+ "invalid TType")
# tuple of: ( 'reader method' name, is_container bool, 'writer_method' name )
_TTYPE_HANDLERS = (
diff --git a/lib/rb/lib/thrift/protocol/base_protocol.rb b/lib/rb/lib/thrift/protocol/base_protocol.rb
index 5c693e9..4d83a21 100644
--- a/lib/rb/lib/thrift/protocol/base_protocol.rb
+++ b/lib/rb/lib/thrift/protocol/base_protocol.rb
@@ -323,8 +323,6 @@ module Thrift
def skip(type)
case type
- when Types::STOP
- nil
when Types::BOOL
read_bool
when Types::BYTE
@@ -367,6 +365,8 @@ module Thrift
skip(etype)
end
read_list_end
+ else
+ raise ProtocolException.new(ProtocolException::INVALID_DATA, 'Invalid data')
end
end
diff --git a/lib/rb/spec/base_protocol_spec.rb b/lib/rb/spec/base_protocol_spec.rb
index eca936b..cfa7573 100644
--- a/lib/rb/spec/base_protocol_spec.rb
+++ b/lib/rb/spec/base_protocol_spec.rb
@@ -163,7 +163,6 @@ describe 'BaseProtocol' do
@prot.skip(Thrift::Types::I64)
@prot.skip(Thrift::Types::DOUBLE)
@prot.skip(Thrift::Types::STRING)
- @prot.skip(Thrift::Types::STOP) # should do absolutely nothing
end
it "should skip structs" do
diff --git a/lib/swift/Sources/TProtocol.swift b/lib/swift/Sources/TProtocol.swift
index a4e4a20..b111e71 100644
--- a/lib/swift/Sources/TProtocol.swift
+++ b/lib/swift/Sources/TProtocol.swift
@@ -175,8 +175,9 @@ public extension TProtocol {
try skip(type: elemType)
}
try readListEnd()
+
default:
- return
+ throw TProtocolError(error: .invalidData, message: "Invalid data")
}
}
}