From 7c4ad5431d5cd9fcfdc5af6180c107e0abd11ac7 Mon Sep 17 00:00:00 2001 From: Manvendra Singh Date: Thu, 16 Mar 2017 23:08:44 +0530 Subject: [PATCH 1/8] setup.py: add mock module as dependency mock is used to mock output from subprocess in tests. this is a standard module in py3 but no present in py27, that's why this dependency --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index b977db2ee..8acedd337 100755 --- a/setup.py +++ b/setup.py @@ -54,6 +54,7 @@ 'schema-salad >= 2.4.20170308171942, < 3', 'typing >= 3.5.2, < 3.6', 'six >= 1.10.0', + 'mock >= 2.0.0', ], setup_requires=[] + pytest_runner, From 79f5d898f128d855e889b285938559eb769763a9 Mon Sep 17 00:00:00 2001 From: Manvendra Singh Date: Thu, 16 Mar 2017 23:10:50 +0530 Subject: [PATCH 2/8] sandboxjs.py: checks for valid version of node/nodejs - raises runtime exception if install node version is below a threshold version. - Fixes https://github.com/common-workflow-language/cwltool/issues/254 --- cwltool/sandboxjs.py | 23 +++++++++++++++++++++++ setup.py | 3 +-- tests/test_js_sandbox.py | 26 ++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 tests/test_js_sandbox.py diff --git a/cwltool/sandboxjs.py b/cwltool/sandboxjs.py index cadfec60f..e9d38c292 100644 --- a/cwltool/sandboxjs.py +++ b/cwltool/sandboxjs.py @@ -23,6 +23,19 @@ class JavascriptException(Exception): have_node_slim = False +def check_js_threshold_version(working_alias): + # type: (str) -> bool + """ parse node version: 'v4.2.6\n' -> ['4', '2', '6'], + https://github.com/nodejs/node/blob/master/CHANGELOG.md#nodejs-changelog """ + + v1, v2, v3 = [int(v) for v in subprocess.check_output( + [working_alias, "-v"]).decode('ascii').strip().strip('v').split('.')] + + if v1 == 0: + if v2 == 10 and v3 <= 25 or v2 < 10: + return False + return True + def new_js_proc(): # type: () -> subprocess.Popen @@ -31,9 +44,11 @@ def new_js_proc(): nodejs = None trynodes = ("nodejs", "node") + working_alias = None for n in trynodes: try: if subprocess.check_output([n, "--eval", "process.stdout.write('t')"]) != "t": + working_alias = n continue nodejs = subprocess.Popen([n, "--eval", nodecode], stdin=subprocess.PIPE, @@ -48,6 +63,14 @@ def new_js_proc(): else: raise + """ check nodejs version, if it is below certain threshold version, + raise Runtime Exception. Such a test won't be required for docker nodejs""" + if nodejs is not None: + if check_js_threshold_version(working_alias) is False: + raise JavascriptException(u"cwltool requires updated version of Node.js engine. " + "Try updating: https://docs.npmjs.com/getting-started/installing-node") + + if nodejs is None: try: nodeimg = "node:slim" diff --git a/setup.py b/setup.py index 8acedd337..7a67f0e2a 100755 --- a/setup.py +++ b/setup.py @@ -46,6 +46,7 @@ 'extensions.yml']}, include_package_data=True, install_requires=[ + 'mock >= 2.0.0', 'setuptools', 'requests >= 1.0', 'ruamel.yaml >= 0.12.4', @@ -54,8 +55,6 @@ 'schema-salad >= 2.4.20170308171942, < 3', 'typing >= 3.5.2, < 3.6', 'six >= 1.10.0', - 'mock >= 2.0.0', - ], setup_requires=[] + pytest_runner, test_suite='tests', diff --git a/tests/test_js_sandbox.py b/tests/test_js_sandbox.py new file mode 100644 index 000000000..4ccb205cd --- /dev/null +++ b/tests/test_js_sandbox.py @@ -0,0 +1,26 @@ +import unittest +from mock import Mock + +# we should modify the subprocess imported from cwltool.sandboxjs +from cwltool.sandboxjs import check_js_threshold_version, subprocess + +class Javascript_Sanity_Checks(unittest.TestCase): + + def test_node_version(self): + subprocess.check_output = Mock(return_value=b'v0.8.26\n') + self.assertEquals(check_js_threshold_version('node'), False) + + subprocess.check_output = Mock(return_value=b'v0.10.25\n') + self.assertEquals(check_js_threshold_version('node'), False) + + subprocess.check_output = Mock(return_value=b'v0.10.26\n') + self.assertEquals(check_js_threshold_version('node'), True) + + subprocess.check_output = Mock(return_value=b'v4.4.2\n') + self.assertEquals(check_js_threshold_version('node'), True) + + subprocess.check_output = Mock(return_value=b'v7.7.3\n') + self.assertEquals(check_js_threshold_version('node'), True) + + def test_is_javascript_installed(self): + pass From a2af80ad45520454e816d2cb3b3266a367349bd4 Mon Sep 17 00:00:00 2001 From: Manvendra Singh Date: Thu, 30 Mar 2017 05:11:22 +0530 Subject: [PATCH 3/8] sandboxjs.py: catch subprocess exception --- cwltool/sandboxjs.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/cwltool/sandboxjs.py b/cwltool/sandboxjs.py index e9d38c292..7285f0a6f 100644 --- a/cwltool/sandboxjs.py +++ b/cwltool/sandboxjs.py @@ -26,10 +26,15 @@ class JavascriptException(Exception): def check_js_threshold_version(working_alias): # type: (str) -> bool """ parse node version: 'v4.2.6\n' -> ['4', '2', '6'], - https://github.com/nodejs/node/blob/master/CHANGELOG.md#nodejs-changelog """ - - v1, v2, v3 = [int(v) for v in subprocess.check_output( - [working_alias, "-v"]).decode('ascii').strip().strip('v').split('.')] + https://github.com/nodejs/node/blob/master/CHANGELOG.md#nodejs-changelog""" + + try: + v1, v2, v3 = [int(v) for v in subprocess.check_output( + [working_alias, "-v"]).decode('ascii').strip().strip('v').split('.')] + except Exception as e: + _logger.debug(str(e)) + _logger.debug("Calling subprocess failed") + return True if v1 == 0: if v2 == 10 and v3 <= 25 or v2 < 10: From 1ed6bc36969684b9f19f17105763cd4dd36e9b83 Mon Sep 17 00:00:00 2001 From: Manvendra Singh Date: Thu, 25 May 2017 13:45:32 +0530 Subject: [PATCH 4/8] sandbox temp diff --- cwltool/main.py | 4 ++-- cwltool/sandboxjs.py | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cwltool/main.py b/cwltool/main.py index 0e34f86d6..df53f6969 100755 --- a/cwltool/main.py +++ b/cwltool/main.py @@ -649,8 +649,8 @@ def main(argsl=None, # type: List[str] if args.version: print(versionfunc()) return 0 - else: - _logger.info(versionfunc()) + # else: + # _logger.info(versionfunc()) if not args.workflow: if os.path.isfile("CWLFile"): diff --git a/cwltool/sandboxjs.py b/cwltool/sandboxjs.py index 7285f0a6f..e530fce4e 100644 --- a/cwltool/sandboxjs.py +++ b/cwltool/sandboxjs.py @@ -28,13 +28,13 @@ def check_js_threshold_version(working_alias): """ parse node version: 'v4.2.6\n' -> ['4', '2', '6'], https://github.com/nodejs/node/blob/master/CHANGELOG.md#nodejs-changelog""" - try: - v1, v2, v3 = [int(v) for v in subprocess.check_output( - [working_alias, "-v"]).decode('ascii').strip().strip('v').split('.')] - except Exception as e: - _logger.debug(str(e)) - _logger.debug("Calling subprocess failed") - return True + # try: + v1, v2, v3 = [int(v) for v in subprocess.check_output( + [working_alias, "-v"]).decode('ascii').strip().strip('v').split('.')] + # except Exception as e: + # _logger.debug(str(e)) + # _logger.debug("Calling subprocess failed") + # return True if v1 == 0: if v2 == 10 and v3 <= 25 or v2 < 10: From b8c647e906660cc4bc1e1b44ecf2fce30cf1c0e2 Mon Sep 17 00:00:00 2001 From: Manvendra Singh Date: Thu, 25 May 2017 19:37:25 +0530 Subject: [PATCH 5/8] sandboxjs.py : - better method to cmp versions - remove tests monkey patch - better condition checking while raising exceptions --- cwltool/sandboxjs.py | 78 +++++++++++++++++++++------------------- tests/test_js_sandbox.py | 10 ++++-- 2 files changed, 50 insertions(+), 38 deletions(-) diff --git a/cwltool/sandboxjs.py b/cwltool/sandboxjs.py index e530fce4e..43443d04b 100644 --- a/cwltool/sandboxjs.py +++ b/cwltool/sandboxjs.py @@ -8,7 +8,8 @@ from io import BytesIO from pkg_resources import resource_stream -from typing import Any, Dict, List, Mapping, Text, Union +from typing import Any, Dict, List, Mapping, Text, Union, Tuple + class JavascriptException(Exception): pass @@ -21,25 +22,28 @@ class JavascriptException(Exception): localdata = threading.local() have_node_slim = False - +# minimum acceptable version of nodejs engine +minimum_node_version_str = '0.10.26' def check_js_threshold_version(working_alias): - # type: (str) -> bool - """ parse node version: 'v4.2.6\n' -> ['4', '2', '6'], - https://github.com/nodejs/node/blob/master/CHANGELOG.md#nodejs-changelog""" - - # try: - v1, v2, v3 = [int(v) for v in subprocess.check_output( - [working_alias, "-v"]).decode('ascii').strip().strip('v').split('.')] - # except Exception as e: - # _logger.debug(str(e)) - # _logger.debug("Calling subprocess failed") - # return True - - if v1 == 0: - if v2 == 10 and v3 <= 25 or v2 < 10: - return False - return True + # type: (str) -> Tuple[bool, str] + + """Checks if the nodeJS engine version on the system + with the allowed minimum version. + https://github.com/nodejs/node/blob/master/CHANGELOG.md#nodejs-changelog + """ + # parse nodejs version into int Tuple: 'v4.2.6\n' -> [4, 2, 6] + current_version_str = subprocess.check_output( + [working_alias, "-v"]).decode('ascii') + + current_version = [int(v) for v in current_version_str.strip().strip('v').split('.')] + minimum_node_version = [int(v) for v in minimum_node_version_str.split('.')] + + if current_version >= minimum_node_version: + return True + else: + return False + def new_js_proc(): # type: () -> subprocess.Popen @@ -47,19 +51,21 @@ def new_js_proc(): res = resource_stream(__name__, 'cwlNodeEngine.js') nodecode = res.read() + required_node_version, docker = (False,)*2 nodejs = None trynodes = ("nodejs", "node") - working_alias = None for n in trynodes: try: if subprocess.check_output([n, "--eval", "process.stdout.write('t')"]) != "t": - working_alias = n continue - nodejs = subprocess.Popen([n, "--eval", nodecode], - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE) - break + else: + nodejs = subprocess.Popen([n, "--eval", nodecode], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + + required_node_version = check_js_threshold_version(n) + break except subprocess.CalledProcessError: pass except OSError as e: @@ -68,15 +74,7 @@ def new_js_proc(): else: raise - """ check nodejs version, if it is below certain threshold version, - raise Runtime Exception. Such a test won't be required for docker nodejs""" - if nodejs is not None: - if check_js_threshold_version(working_alias) is False: - raise JavascriptException(u"cwltool requires updated version of Node.js engine. " - "Try updating: https://docs.npmjs.com/getting-started/installing-node") - - - if nodejs is None: + if nodejs is None or nodejs is not None and required_node_version is False: try: nodeimg = "node:slim" global have_node_slim @@ -91,6 +89,7 @@ def new_js_proc(): "--sig-proxy=true", "--interactive", "--rm", nodeimg, "node", "--eval", nodecode], stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + docker = True except OSError as e: if e.errno == errno.ENOENT: pass @@ -99,12 +98,19 @@ def new_js_proc(): except subprocess.CalledProcessError: pass + # docker failed and nodejs not on system if nodejs is None: raise JavascriptException( u"cwltool requires Node.js engine to evaluate Javascript " "expressions, but couldn't find it. Tried %s, docker run " "node:slim" % u", ".join(trynodes)) + # docker failed, but nodejs is installed on system but the version is below the required version + if docker is False and required_node_version is False: + raise JavascriptException( + u'cwltool requires minimum v{} version of Node.js engine.'.format(minimum_node_version_str), + u'Try updating: https://docs.npmjs.com/getting-started/installing-node') + return nodejs @@ -115,7 +121,7 @@ def execjs(js, jslib, timeout=None, debug=False): # type: (Union[Mapping, Text] nodejs = localdata.proc - fn = u"\"use strict\";\n%s\n(function()%s)()" %\ + fn = u"\"use strict\";\n%s\n(function()%s)()" % \ (jslib, js if isinstance(js, basestring) and len(js) > 1 and js[0] == '{' else ("{return (%s);}" % js)) killed = [] @@ -182,7 +188,7 @@ def stdfmt(data): # type: (unicode) -> unicode nodejs.poll() if debug: - info = u"returncode was: %s\nscript was:\n%s\nstdout was: %s\nstderr was: %s\n" %\ + info = u"returncode was: %s\nscript was:\n%s\nstdout was: %s\nstderr was: %s\n" % \ (nodejs.returncode, fn_linenum(), stdfmt(stdoutdata), stdfmt(stderrdata)) else: info = stdfmt(stderrdata) diff --git a/tests/test_js_sandbox.py b/tests/test_js_sandbox.py index 4ccb205cd..5abe79ed8 100644 --- a/tests/test_js_sandbox.py +++ b/tests/test_js_sandbox.py @@ -1,11 +1,17 @@ import unittest -from mock import Mock +from mock import Mock, patch # we should modify the subprocess imported from cwltool.sandboxjs -from cwltool.sandboxjs import check_js_threshold_version, subprocess +from cwltool.sandboxjs import check_js_threshold_version, subprocess, minimum_node_version_str class Javascript_Sanity_Checks(unittest.TestCase): + def setUp(self): + self.check_output = subprocess.check_output + + def tearDown(self): + subprocess.check_output = self.check_output + def test_node_version(self): subprocess.check_output = Mock(return_value=b'v0.8.26\n') self.assertEquals(check_js_threshold_version('node'), False) From 70605b3e2979356013a3a19a26f2a6adbc35f24b Mon Sep 17 00:00:00 2001 From: Manvendra Singh Date: Thu, 25 May 2017 19:43:43 +0530 Subject: [PATCH 6/8] fix func return type annotaion --- cwltool/sandboxjs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cwltool/sandboxjs.py b/cwltool/sandboxjs.py index 43443d04b..79bebfb9c 100644 --- a/cwltool/sandboxjs.py +++ b/cwltool/sandboxjs.py @@ -26,7 +26,7 @@ class JavascriptException(Exception): minimum_node_version_str = '0.10.26' def check_js_threshold_version(working_alias): - # type: (str) -> Tuple[bool, str] + # type: (str) -> bool """Checks if the nodeJS engine version on the system with the allowed minimum version. From 8a54e7c57e089004d3bd63e4b9037bbea8f26246 Mon Sep 17 00:00:00 2001 From: Manvendra Singh Date: Thu, 25 May 2017 19:55:08 +0530 Subject: [PATCH 7/8] revert changes in cwltool/main.py --- cwltool/main.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cwltool/main.py b/cwltool/main.py index df53f6969..0e34f86d6 100755 --- a/cwltool/main.py +++ b/cwltool/main.py @@ -649,8 +649,8 @@ def main(argsl=None, # type: List[str] if args.version: print(versionfunc()) return 0 - # else: - # _logger.info(versionfunc()) + else: + _logger.info(versionfunc()) if not args.workflow: if os.path.isfile("CWLFile"): From 342194bcdb06c7c63703d5ad5890c7a0db7d8255 Mon Sep 17 00:00:00 2001 From: Manvendra Singh Date: Thu, 25 May 2017 19:57:00 +0530 Subject: [PATCH 8/8] revert erroneous changes in cwltool/sandbox.py --- cwltool/sandboxjs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cwltool/sandboxjs.py b/cwltool/sandboxjs.py index 79bebfb9c..48bd0f58e 100644 --- a/cwltool/sandboxjs.py +++ b/cwltool/sandboxjs.py @@ -121,7 +121,7 @@ def execjs(js, jslib, timeout=None, debug=False): # type: (Union[Mapping, Text] nodejs = localdata.proc - fn = u"\"use strict\";\n%s\n(function()%s)()" % \ + fn = u"\"use strict\";\n%s\n(function()%s)()" %\ (jslib, js if isinstance(js, basestring) and len(js) > 1 and js[0] == '{' else ("{return (%s);}" % js)) killed = [] @@ -188,7 +188,7 @@ def stdfmt(data): # type: (unicode) -> unicode nodejs.poll() if debug: - info = u"returncode was: %s\nscript was:\n%s\nstdout was: %s\nstderr was: %s\n" % \ + info = u"returncode was: %s\nscript was:\n%s\nstdout was: %s\nstderr was: %s\n" %\ (nodejs.returncode, fn_linenum(), stdfmt(stdoutdata), stdfmt(stderrdata)) else: info = stdfmt(stderrdata)