You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by ns...@apache.org on 2016/01/11 03:49:00 UTC

[6/6] thrift git commit: THRIFT-3531 Create cross lang feature test for string and container read length limit

THRIFT-3531 Create cross lang feature test for string and container read length limit

This closes #780


Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/85650612
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/85650612
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/85650612

Branch: refs/heads/master
Commit: 85650612e15c79c79e470553d3779d18f755150c
Parents: a3b88a0
Author: Nobuaki Sukegawa <ns...@apache.org>
Authored: Fri Jan 8 03:26:44 2016 +0900
Committer: Nobuaki Sukegawa <ns...@gmail.com>
Committed: Mon Jan 11 11:41:56 2016 +0900

----------------------------------------------------------------------
 test/cpp/src/TestServer.cpp             | 64 ++++++++++++-------------
 test/crossrunner/collect.py             |  1 +
 test/crossrunner/test.py                | 13 +++--
 test/features/container_limit.py        | 72 ++++++++++++++++++++++++++++
 test/features/known_failures_Linux.json | 46 ++++++++++++++++++
 test/features/string_limit.py           | 61 +++++++++++++++++++++++
 test/features/tests.json                | 36 ++++++++++++++
 test/features/util.py                   | 31 ++++++++++--
 test/test.py                            | 13 +++--
 9 files changed, 293 insertions(+), 44 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/thrift/blob/85650612/test/cpp/src/TestServer.cpp
----------------------------------------------------------------------
diff --git a/test/cpp/src/TestServer.cpp b/test/cpp/src/TestServer.cpp
index 12c4b97..b3c292a 100644
--- a/test/cpp/src/TestServer.cpp
+++ b/test/cpp/src/TestServer.cpp
@@ -533,6 +533,8 @@ protected:
   boost::shared_ptr<TestHandler> _delegate;
 };
 
+namespace po = boost::program_options;
+
 int main(int argc, char** argv) {
 
   string file_path = boost::filesystem::system_complete(argv[0]).string();
@@ -549,34 +551,27 @@ int main(int argc, char** argv) {
   string domain_socket = "";
   bool abstract_namespace = false;
   size_t workers = 4;
-
-  boost::program_options::options_description desc("Allowed options");
-  desc.add_options()("help,h", "produce help message")(
-      "port",
-      boost::program_options::value<int>(&port)->default_value(port),
-      "Port number to listen")("domain-socket",
-                               boost::program_options::value<string>(&domain_socket)
-                                   ->default_value(domain_socket),
-                               "Unix Domain Socket (e.g. /tmp/ThriftTest.thrift)")(
-      "abstract-namespace",
-      "Create the domain socket in the Abstract Namespace (no connection with filesystem pathnames)")(
-      "server-type",
-      boost::program_options::value<string>(&server_type)->default_value(server_type),
-      "type of server, \"simple\", \"thread-pool\", \"threaded\", or \"nonblocking\"")(
-      "transport",
-      boost::program_options::value<string>(&transport_type)->default_value(transport_type),
-      "transport: buffered, framed, http")(
-      "protocol",
-      boost::program_options::value<string>(&protocol_type)->default_value(protocol_type),
-      "protocol: binary, compact, header, json")("ssl", "Encrypted Transport using SSL")(
-      "processor-events",
-      "processor-events")("workers,n",
-                          boost::program_options::value<size_t>(&workers)->default_value(workers),
-                          "Number of thread pools workers. Only valid for thread-pool server type");
-
-  boost::program_options::variables_map vm;
-  boost::program_options::store(boost::program_options::parse_command_line(argc, argv, desc), vm);
-  boost::program_options::notify(vm);
+  int string_limit = 0;
+  int container_limit = 0;
+
+  po::options_description desc("Allowed options");
+  desc.add_options()
+    ("help,h", "produce help message")
+    ("port", po::value<int>(&port)->default_value(port), "Port number to listen")
+    ("domain-socket", po::value<string>(&domain_socket) ->default_value(domain_socket), "Unix Domain Socket (e.g. /tmp/ThriftTest.thrift)")
+    ("abstract-namespace", "Create the domain socket in the Abstract Namespace (no connection with filesystem pathnames)")
+    ("server-type", po::value<string>(&server_type)->default_value(server_type), "type of server, \"simple\", \"thread-pool\", \"threaded\", or \"nonblocking\"")
+    ("transport", po::value<string>(&transport_type)->default_value(transport_type), "transport: buffered, framed, http")
+    ("protocol", po::value<string>(&protocol_type)->default_value(protocol_type), "protocol: binary, compact, header, json")
+    ("ssl", "Encrypted Transport using SSL")
+    ("processor-events", "processor-events")
+    ("workers,n", po::value<size_t>(&workers)->default_value(workers), "Number of thread pools workers. Only valid for thread-pool server type")
+    ("string-limit", po::value<int>(&string_limit))
+    ("container-limit", po::value<int>(&container_limit));
+
+  po::variables_map vm;
+  po::store(po::parse_command_line(argc, argv, desc), vm);
+  po::notify(vm);
 
   if (vm.count("help")) {
     cout << desc << "\n";
@@ -633,15 +628,18 @@ int main(int argc, char** argv) {
     boost::shared_ptr<TProtocolFactory> jsonProtocolFactory(new TJSONProtocolFactory());
     protocolFactory = jsonProtocolFactory;
   } else if (protocol_type == "compact") {
-    boost::shared_ptr<TProtocolFactory> compactProtocolFactory(new TCompactProtocolFactory());
-    protocolFactory = compactProtocolFactory;
+    TCompactProtocolFactoryT<TBufferBase> *compactProtocolFactory = new TCompactProtocolFactoryT<TBufferBase>();
+    compactProtocolFactory->setContainerSizeLimit(container_limit);
+    compactProtocolFactory->setStringSizeLimit(string_limit);
+    protocolFactory.reset(compactProtocolFactory);
   } else if (protocol_type == "header") {
     boost::shared_ptr<TProtocolFactory> headerProtocolFactory(new THeaderProtocolFactory());
     protocolFactory = headerProtocolFactory;
   } else {
-    boost::shared_ptr<TProtocolFactory> binaryProtocolFactory(
-        new TBinaryProtocolFactoryT<TBufferBase>());
-    protocolFactory = binaryProtocolFactory;
+    TBinaryProtocolFactoryT<TBufferBase>* binaryProtocolFactory = new TBinaryProtocolFactoryT<TBufferBase>();
+    binaryProtocolFactory->setContainerSizeLimit(container_limit);
+    binaryProtocolFactory->setStringSizeLimit(string_limit);
+    protocolFactory.reset(binaryProtocolFactory);
   }
 
   // Processor

http://git-wip-us.apache.org/repos/asf/thrift/blob/85650612/test/crossrunner/collect.py
----------------------------------------------------------------------
diff --git a/test/crossrunner/collect.py b/test/crossrunner/collect.py
index 455189c..f92b9e2 100644
--- a/test/crossrunner/collect.py
+++ b/test/crossrunner/collect.py
@@ -44,6 +44,7 @@ VALID_JSON_KEYS = [
   'workdir',  # work directory where command is executed
   'command',  # test command
   'extra_args',  # args appended to command after other args are appended
+  'remote_args',  # args added to the other side of the program
   'join_args',  # whether args should be passed as single concatenated string
   'env',  # additional environmental variable
 ]

http://git-wip-us.apache.org/repos/asf/thrift/blob/85650612/test/crossrunner/test.py
----------------------------------------------------------------------
diff --git a/test/crossrunner/test.py b/test/crossrunner/test.py
index 49ba7d3..bb81c4f 100644
--- a/test/crossrunner/test.py
+++ b/test/crossrunner/test.py
@@ -31,7 +31,7 @@ def domain_socket_path(port):
 
 class TestProgram(object):
   def __init__(self, kind, name, protocol, transport, socket, workdir, command, env=None,
-               extra_args=[], join_args=False, **kwargs):
+               extra_args=[], extra_args2=[], join_args=False, **kwargs):
     self.kind = kind
     self.name = name
     self.protocol = protocol
@@ -46,6 +46,7 @@ class TestProgram(object):
     else:
       self.env = os.environ
     self._extra_args = extra_args
+    self._extra_args2 = extra_args2
     self._join_args = join_args
 
   def _fix_cmd_path(self, cmd):
@@ -69,7 +70,7 @@ class TestProgram(object):
 
   def build_command(self, port):
     cmd = copy.copy(self._base_command)
-    args = []
+    args = self._extra_args2
     args.append('--protocol=' + self.protocol)
     args.append('--transport=' + self.transport)
     socket_args = self._socket_args(self.socket, port)
@@ -94,8 +95,12 @@ class TestEntry(object):
     self.protocol = kwargs['protocol']
     self.transport = kwargs['transport']
     self.socket = kwargs['socket']
-    self.server = TestProgram('server', **self._fix_workdir(merge_dict(self._config, server)))
-    self.client = TestProgram('client', **self._fix_workdir(merge_dict(self._config, client)))
+    srv_dict = self._fix_workdir(merge_dict(self._config, server))
+    cli_dict = self._fix_workdir(merge_dict(self._config, client))
+    cli_dict['extra_args2'] = srv_dict.pop('remote_args', [])
+    srv_dict['extra_args2'] = cli_dict.pop('remote_args', [])
+    self.server = TestProgram('server', **srv_dict)
+    self.client = TestProgram('client', **cli_dict)
     self.delay = delay
     self.timeout = timeout
     self._name = None

http://git-wip-us.apache.org/repos/asf/thrift/blob/85650612/test/features/container_limit.py
----------------------------------------------------------------------
diff --git a/test/features/container_limit.py b/test/features/container_limit.py
new file mode 100644
index 0000000..4a7da60
--- /dev/null
+++ b/test/features/container_limit.py
@@ -0,0 +1,72 @@
+#!/usr/bin/env python
+
+import argparse
+import sys
+
+from util import add_common_args, init_protocol
+from local_thrift import thrift
+from thrift.Thrift import TMessageType, TType
+
+
+# TODO: generate from ThriftTest.thrift
+def test_list(proto, value):
+  method_name = 'testList'
+  ttype = TType.LIST
+  etype = TType.I32
+  proto.writeMessageBegin(method_name, TMessageType.CALL, 3)
+  proto.writeStructBegin(method_name + '_args')
+  proto.writeFieldBegin('thing', ttype, 1)
+  proto.writeListBegin(etype, len(value))
+  for e in value:
+    proto.writeI32(e)
+  proto.writeListEnd()
+  proto.writeFieldEnd()
+  proto.writeFieldStop()
+  proto.writeStructEnd()
+  proto.writeMessageEnd()
+  proto.trans.flush()
+
+  _, mtype, _ = proto.readMessageBegin()
+  assert mtype == TMessageType.REPLY
+  proto.readStructBegin()
+  _, ftype, fid = proto.readFieldBegin()
+  assert fid == 0
+  assert ftype == ttype
+  etype2, len2 = proto.readListBegin()
+  assert etype == etype2
+  assert len2 == len(value)
+  for i in range(len2):
+    v = proto.readI32()
+    assert v == value[i]
+  proto.readListEnd()
+  proto.readFieldEnd()
+  _, ftype, _ = proto.readFieldBegin()
+  assert ftype == TType.STOP
+  proto.readStructEnd()
+  proto.readMessageEnd()
+
+
+def main(argv):
+  p = argparse.ArgumentParser()
+  add_common_args(p)
+  p.add_argument('--limit', type=int)
+  args = p.parse_args()
+  proto = init_protocol(args)
+  # TODO: test set and map
+  test_list(proto, list(range(args.limit - 1)))
+  test_list(proto, list(range(args.limit - 1)))
+  print('[OK]: limit - 1')
+  test_list(proto, list(range(args.limit)))
+  test_list(proto, list(range(args.limit)))
+  print('[OK]: just limit')
+  try:
+    test_list(proto, list(range(args.limit + 1)))
+  except:
+    print('[OK]: limit + 1')
+  else:
+    print('[ERROR]: limit + 1')
+    assert False
+
+
+if __name__ == '__main__':
+  sys.exit(main(sys.argv[1:]))

http://git-wip-us.apache.org/repos/asf/thrift/blob/85650612/test/features/known_failures_Linux.json
----------------------------------------------------------------------
diff --git a/test/features/known_failures_Linux.json b/test/features/known_failures_Linux.json
new file mode 100644
index 0000000..edff41a
--- /dev/null
+++ b/test/features/known_failures_Linux.json
@@ -0,0 +1,46 @@
+[
+  "c_glib-limit_container_length_binary_buffered-ip",
+  "c_glib-limit_string_length_binary_buffered-ip",
+  "csharp-limit_container_length_binary_buffered-ip",
+  "csharp-limit_container_length_compact_buffered-ip",
+  "csharp-limit_string_length_binary_buffered-ip",
+  "csharp-limit_string_length_compact_buffered-ip",
+  "erl-limit_container_length_binary_buffered-ip",
+  "erl-limit_container_length_compact_buffered-ip",
+  "erl-limit_string_length_binary_buffered-ip",
+  "erl-limit_string_length_compact_buffered-ip",
+  "go-limit_container_length_binary_buffered-ip",
+  "go-limit_container_length_compact_buffered-ip",
+  "go-limit_string_length_binary_buffered-ip",
+  "go-limit_string_length_compact_buffered-ip",
+  "hs-limit_container_length_binary_buffered-ip",
+  "hs-limit_container_length_compact_buffered-ip",
+  "hs-limit_string_length_binary_buffered-ip",
+  "hs-limit_string_length_compact_buffered-ip",
+  "java-limit_container_length_binary_buffered-ip",
+  "java-limit_container_length_compact_buffered-ip",
+  "java-limit_string_length_binary_buffered-ip",
+  "java-limit_string_length_compact_buffered-ip",
+  "nodejs-limit_container_length_binary_buffered-ip",
+  "nodejs-limit_container_length_compact_buffered-ip",
+  "nodejs-limit_string_length_binary_buffered-ip",
+  "nodejs-limit_string_length_compact_buffered-ip",
+  "perl-limit_container_length_binary_buffered-ip",
+  "perl-limit_string_length_binary_buffered-ip",
+  "py-limit_container_length_accel-binary_buffered-ip",
+  "py-limit_container_length_binary_buffered-ip",
+  "py-limit_container_length_compact_buffered-ip",
+  "py-limit_string_length_accel-binary_buffered-ip",
+  "py-limit_string_length_binary_buffered-ip",
+  "py-limit_string_length_compact_buffered-ip",
+  "py3-limit_container_length_binary_buffered-ip",
+  "py3-limit_container_length_compact_buffered-ip",
+  "py3-limit_string_length_binary_buffered-ip",
+  "py3-limit_string_length_compact_buffered-ip",
+  "rb-limit_container_length_accel-binary_buffered-ip",
+  "rb-limit_container_length_binary_buffered-ip",
+  "rb-limit_container_length_compact_buffered-ip",
+  "rb-limit_string_length_accel-binary_buffered-ip",
+  "rb-limit_string_length_binary_buffered-ip",
+  "rb-limit_string_length_compact_buffered-ip"
+]
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/thrift/blob/85650612/test/features/string_limit.py
----------------------------------------------------------------------
diff --git a/test/features/string_limit.py b/test/features/string_limit.py
new file mode 100644
index 0000000..b4d48ac
--- /dev/null
+++ b/test/features/string_limit.py
@@ -0,0 +1,61 @@
+#!/usr/bin/env python
+
+import argparse
+import sys
+
+from util import add_common_args, init_protocol
+from local_thrift import thrift
+from thrift.Thrift import TMessageType, TType
+
+
+# TODO: generate from ThriftTest.thrift
+def test_string(proto, value):
+  method_name = 'testString'
+  ttype = TType.STRING
+  proto.writeMessageBegin(method_name, TMessageType.CALL, 3)
+  proto.writeStructBegin(method_name + '_args')
+  proto.writeFieldBegin('thing', ttype, 1)
+  proto.writeString(value)
+  proto.writeFieldEnd()
+  proto.writeFieldStop()
+  proto.writeStructEnd()
+  proto.writeMessageEnd()
+  proto.trans.flush()
+
+  _, mtype, _ = proto.readMessageBegin()
+  assert mtype == TMessageType.REPLY
+  proto.readStructBegin()
+  _, ftype, fid = proto.readFieldBegin()
+  assert fid == 0
+  assert ftype == ttype
+  result = proto.readString()
+  proto.readFieldEnd()
+  _, ftype, _ = proto.readFieldBegin()
+  assert ftype == TType.STOP
+  proto.readStructEnd()
+  proto.readMessageEnd()
+  assert value == result
+
+
+def main(argv):
+  p = argparse.ArgumentParser()
+  add_common_args(p)
+  p.add_argument('--limit', type=int)
+  args = p.parse_args()
+  proto = init_protocol(args)
+  test_string(proto, 'a' * (args.limit - 1))
+  test_string(proto, 'a' * (args.limit - 1))
+  print('[OK]: limit - 1')
+  test_string(proto, 'a' * args.limit)
+  test_string(proto, 'a' * args.limit)
+  print('[OK]: just limit')
+  try:
+    test_string(proto, 'a' * (args.limit + 1))
+  except:
+    print('[OK]: limit + 1')
+  else:
+    print('[ERROR]: limit + 1')
+    assert False
+
+if __name__ == '__main__':
+  main(sys.argv[1:])

http://git-wip-us.apache.org/repos/asf/thrift/blob/85650612/test/features/tests.json
----------------------------------------------------------------------
diff --git a/test/features/tests.json b/test/features/tests.json
index 0836309..f726dad 100644
--- a/test/features/tests.json
+++ b/test/features/tests.json
@@ -23,5 +23,41 @@
     "transports": ["buffered"],
     "sockets": ["ip"],
     "workdir": "features"
+  },
+  {
+    "name": "limit_string_length",
+    "command": [
+      "python",
+      "string_limit.py",
+      "--limit=50"
+    ],
+    "remote_args": [
+      "--string-limit=50"
+    ],
+    "protocols": [
+      "binary",
+      "compact"
+    ],
+    "transports": ["buffered"],
+    "sockets": ["ip"],
+    "workdir": "features"
+  },
+  {
+    "name": "limit_container_length",
+    "command": [
+      "python",
+      "container_limit.py",
+      "--limit=50"
+    ],
+    "remote_args": [
+      "--container-limit=50"
+    ],
+    "protocols": [
+      "binary",
+      "compact"
+    ],
+    "transports": ["buffered"],
+    "sockets": ["ip"],
+    "workdir": "features"
   }
 ]

http://git-wip-us.apache.org/repos/asf/thrift/blob/85650612/test/features/util.py
----------------------------------------------------------------------
diff --git a/test/features/util.py b/test/features/util.py
index cff7ff8..e364136 100644
--- a/test/features/util.py
+++ b/test/features/util.py
@@ -1,11 +1,20 @@
 import argparse
+import socket
+
+from local_thrift import thrift
+from thrift.transport.TSocket import TSocket
+from thrift.transport.TTransport import TBufferedTransport, TFramedTransport
+from thrift.transport.THttpClient import THttpClient
+from thrift.protocol.TBinaryProtocol import TBinaryProtocol
+from thrift.protocol.TCompactProtocol import TCompactProtocol
+from thrift.protocol.TJSONProtocol import TJSONProtocol
 
 
 def add_common_args(p):
   p.add_argument('--host', default='localhost')
-  p.add_argument('--port', type=int)
-  p.add_argument('--protocol')
-  p.add_argument('--transport')
+  p.add_argument('--port', type=int, default=9090)
+  p.add_argument('--protocol', default='binary')
+  p.add_argument('--transport', default='buffered')
   p.add_argument('--ssl', action='store_true')
 
 
@@ -13,3 +22,19 @@ def parse_common_args(argv):
   p = argparse.ArgumentParser()
   add_common_args(p)
   return p.parse_args(argv)
+
+
+def init_protocol(args):
+  sock = TSocket(args.host, args.port, socket_family=socket.AF_INET)
+  sock.setTimeout(500)
+  trans = {
+    'buffered': TBufferedTransport,
+    'framed': TFramedTransport,
+    'http': THttpClient,
+  }[args.transport](sock)
+  trans.open()
+  return {
+    'binary': TBinaryProtocol,
+    'compact': TCompactProtocol,
+    'json': TJSONProtocol,
+  }[args.protocol](trans)

http://git-wip-us.apache.org/repos/asf/thrift/blob/85650612/test/test.py
----------------------------------------------------------------------
diff --git a/test/test.py b/test/test.py
index 0c799b9..20d76f4 100755
--- a/test/test.py
+++ b/test/test.py
@@ -39,6 +39,7 @@ import crossrunner
 from crossrunner.compat import path_join
 
 TEST_DIR = os.path.realpath(os.path.dirname(__file__))
+FEATURE_DIR = path_join(TEST_DIR, 'features')
 CONFIG_FILE = 'tests.json'
 
 
@@ -101,8 +102,7 @@ def run_cross_tests(server_match, client_match, jobs, skip_known_failures):
 
 
 def run_feature_tests(server_match, feature_match, jobs, skip_known_failures):
-  basedir = path_join(TEST_DIR, 'features')
-  # run_tests(crossrunner.collect_feature_tests, basedir, server_match, feature_match, jobs, skip_known_failures)
+  basedir = FEATURE_DIR
   logger = multiprocessing.get_logger()
   logger.debug('Collecting tests')
   with open(path_join(TEST_DIR, CONFIG_FILE), 'r') as fp:
@@ -153,7 +153,7 @@ def main(argv):
                       default=default_concurrenty(),
                       help='number of concurrent test executions')
   parser.add_argument('-F', '--features', nargs='*', default=None,
-                      help='run feature tests instead of cross language tests')
+                      help='run server feature tests instead of cross language tests')
 
   g = parser.add_argument_group(title='Advanced')
   g.add_argument('-v', '--verbose', action='store_const',
@@ -170,13 +170,18 @@ def main(argv):
   logger = multiprocessing.log_to_stderr()
   logger.setLevel(options.log_level)
 
+  if options.features is not None and options.client:
+    print('Cannot specify both --features and --client ', file=sys.stderr)
+    return 1
+
   # Allow multiple args separated with ',' for backward compatibility
   server_match = list(chain(*[x.split(',') for x in options.server]))
   client_match = list(chain(*[x.split(',') for x in options.client]))
 
   if options.update_failures or options.print_failures:
+    dire = FEATURE_DIR if options.features is not None else TEST_DIR
     res = crossrunner.generate_known_failures(
-        TEST_DIR, options.update_failures == 'overwrite',
+        dire, options.update_failures == 'overwrite',
         options.update_failures, options.print_failures)
   elif options.features is not None:
     features = options.features or ['.*']