From b023d7e23f77e4de33e740dcc37af911e36fb189 Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Fri, 15 May 2015 12:52:29 +0100 Subject: [PATCH] Wrap file objects with codecs to handle unicode Ensure file objects including stdin/stdout objects are wrapped using codecs to handle unicode translation to the selected encoding for input/output. Add tests to simulate different encodings for input/output and consequently fix the reading of input from stdin. Include test to trigger failure to encode a unicode valid character using 'ascii' encoding. Change-Id: I9a5c4c96d131873db0000377729b8b0a45aa470d --- jenkins_jobs/builder.py | 12 +++-- jenkins_jobs/cmd.py | 32 ++++++------ jenkins_jobs/parser.py | 4 +- jenkins_jobs/utils.py | 35 +++++++++++++ tests/cmd/fixtures/unicode-replace001.xml | 25 +++++++++ tests/cmd/fixtures/unicode001.yaml | 12 +++++ tests/cmd/subcommands/test_test.py | 64 +++++++++++++++++++++-- 7 files changed, 160 insertions(+), 24 deletions(-) create mode 100644 jenkins_jobs/utils.py create mode 100644 tests/cmd/fixtures/unicode-replace001.xml create mode 100644 tests/cmd/fixtures/unicode001.yaml diff --git a/jenkins_jobs/builder.py b/jenkins_jobs/builder.py index 060f2384c..dd898f559 100644 --- a/jenkins_jobs/builder.py +++ b/jenkins_jobs/builder.py @@ -29,6 +29,7 @@ import logging from jenkins_jobs.constants import MAGIC_MANAGE_STRING from jenkins_jobs.parser import YamlParser +from jenkins_jobs import utils logger = logging.getLogger(__name__) @@ -213,8 +214,9 @@ class Builder(object): def load_files(self, fn): self.parser = YamlParser(self.global_config, self.plugins_list) - # handle deprecated behavior - if not hasattr(fn, '__iter__'): + # handle deprecated behavior, and check that it's not a file like + # object as these may implement the '__iter__' attribute. + if not hasattr(fn, '__iter__') or hasattr(fn, 'read'): logger.warning( 'Passing single elements for the `fn` argument in ' 'Builder.load_files is deprecated. Please update your code ' @@ -224,7 +226,7 @@ class Builder(object): files_to_process = [] for path in fn: - if os.path.isdir(path): + if not hasattr(path, 'read') and os.path.isdir(path): files_to_process.extend([os.path.join(path, f) for f in os.listdir(path) if (f.endswith('.yml') @@ -236,6 +238,9 @@ class Builder(object): # definitions of macros and templates when loading all from top-level unique_files = [] for f in files_to_process: + if hasattr(f, 'read'): + unique_files.append(f) + continue rpf = os.path.realpath(f) if rpf not in unique_files: unique_files.append(rpf) @@ -318,6 +323,7 @@ class Builder(object): # `output` is a file-like object logger.info("Job name: %s", job.name) logger.debug("Writing XML to '{0}'".format(output)) + output = utils.wrap_stream(output) try: output.write(job.output()) except IOError as exc: diff --git a/jenkins_jobs/cmd.py b/jenkins_jobs/cmd.py index 2b053a8f1..efafdf05f 100755 --- a/jenkins_jobs/cmd.py +++ b/jenkins_jobs/cmd.py @@ -290,30 +290,30 @@ def execute(options, config): plugins_list=plugins_info) if getattr(options, 'path', None): - if options.path == sys.stdin: + if hasattr(options.path, 'read'): logger.debug("Input file is stdin") if options.path.isatty(): key = 'CTRL+Z' if platform.system() == 'Windows' else 'CTRL+D' logger.warn( "Reading configuration from STDIN. Press %s to end input.", key) + else: + # take list of paths + options.path = options.path.split(os.pathsep) - # take list of paths - options.path = options.path.split(os.pathsep) + do_recurse = (getattr(options, 'recursive', False) or + config.getboolean('job_builder', 'recursive')) - do_recurse = (getattr(options, 'recursive', False) or - config.getboolean('job_builder', 'recursive')) - - excludes = [e for elist in options.exclude - for e in elist.split(os.pathsep)] or \ - config.get('job_builder', 'exclude').split(os.pathsep) - paths = [] - for path in options.path: - if do_recurse and os.path.isdir(path): - paths.extend(recurse_path(path, excludes)) - else: - paths.append(path) - options.path = paths + excludes = [e for elist in options.exclude + for e in elist.split(os.pathsep)] or \ + config.get('job_builder', 'exclude').split(os.pathsep) + paths = [] + for path in options.path: + if do_recurse and os.path.isdir(path): + paths.extend(recurse_path(path, excludes)) + else: + paths.append(path) + options.path = paths if options.command == 'delete': for job in options.name: diff --git a/jenkins_jobs/parser.py b/jenkins_jobs/parser.py index 17d4b66ed..60986100f 100644 --- a/jenkins_jobs/parser.py +++ b/jenkins_jobs/parser.py @@ -27,6 +27,7 @@ from jenkins_jobs.constants import MAGIC_MANAGE_STRING from jenkins_jobs.errors import JenkinsJobsException from jenkins_jobs.registry import ModuleRegistry from jenkins_jobs.formatter import deep_format +from jenkins_jobs import utils from jenkins_jobs.xml_config import XmlJob logger = logging.getLogger(__name__) @@ -69,7 +70,8 @@ class YamlParser(object): return keep_desc def parse_fp(self, fp): - data = local_yaml.load(fp, search_path=self.path) + # wrap provided file streams to ensure correct encoding used + data = local_yaml.load(utils.wrap_stream(fp), search_path=self.path) if data: if not isinstance(data, list): raise JenkinsJobsException( diff --git a/jenkins_jobs/utils.py b/jenkins_jobs/utils.py new file mode 100644 index 000000000..14b8443d5 --- /dev/null +++ b/jenkins_jobs/utils.py @@ -0,0 +1,35 @@ +#!/usr/bin/env python +# Copyright (C) 2015 Hewlett-Packard Development Company, L.P. +# +# Licensed 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. + +# functions that don't fit in well elsewhere + +import codecs +import locale + + +def wrap_stream(stream, encoding='utf-8'): + + try: + stream_enc = stream.encoding + except AttributeError: + stream_enc = locale.getpreferredencoding() + + if hasattr(stream, 'buffer'): + stream = stream.buffer + + if str(stream_enc).lower() == str(encoding).lower(): + return stream + + return codecs.EncodedFile(stream, encoding, stream_enc) diff --git a/tests/cmd/fixtures/unicode-replace001.xml b/tests/cmd/fixtures/unicode-replace001.xml new file mode 100644 index 000000000..4828deeb8 --- /dev/null +++ b/tests/cmd/fixtures/unicode-replace001.xml @@ -0,0 +1,25 @@ + + + + <!-- Managed by Jenkins Job Builder --> + false + false + false + false + true + + + + + + + + + #!/bin/bash +echo "Unicode! ?" + + + + + + diff --git a/tests/cmd/fixtures/unicode001.yaml b/tests/cmd/fixtures/unicode001.yaml new file mode 100644 index 000000000..a2bee1639 --- /dev/null +++ b/tests/cmd/fixtures/unicode001.yaml @@ -0,0 +1,12 @@ +- wrapper: + name: unicode-wrapper + wrappers: + - pre-scm-buildstep: + - shell: | + #!/bin/bash + echo "Unicode! ☃" + +- job: + name: test-unicode-wrapper + wrappers: + - unicode-wrapper diff --git a/tests/cmd/subcommands/test_test.py b/tests/cmd/subcommands/test_test.py index dd12ffd57..883d1df12 100644 --- a/tests/cmd/subcommands/test_test.py +++ b/tests/cmd/subcommands/test_test.py @@ -1,5 +1,5 @@ -import os import io +import os import yaml import jenkins @@ -51,6 +51,7 @@ class TestTests(CmdTestsBase): Run test mode and pass a non-existing configuration directory """ args = self.parser.parse_args(['test', 'foo']) + args.output_dir = mock.MagicMock() self.assertRaises(IOError, cmd.execute, args, self.config) def test_non_existing_config_file(self): @@ -58,6 +59,7 @@ class TestTests(CmdTestsBase): Run test mode and pass a non-existing configuration file """ args = self.parser.parse_args(['test', 'non-existing.yaml']) + args.output_dir = mock.MagicMock() self.assertRaises(IOError, cmd.execute, args, self.config) def test_non_existing_job(self): @@ -80,7 +82,7 @@ class TestTests(CmdTestsBase): os.path.join(self.fixtures_path, 'cmd-001.yaml'), 'foo-job']) - args.output_dir = mock.MagicMock() + args.output_dir = mock.Mock(wraps=io.BytesIO()) cmd.execute(args, self.config) # probably better to fail here @mock.patch('jenkins_jobs.cmd.Builder.update_job') @@ -126,6 +128,7 @@ class TestTests(CmdTestsBase): update_job_mock.assert_called_with(paths, [], output=args.output_dir) args = self.parser.parse_args(['test', multipath]) + args.output_dir = mock.MagicMock() self.config.set('job_builder', 'recursive', 'True') cmd.execute(args, self.config) @@ -172,6 +175,59 @@ class TestTests(CmdTestsBase): 'r', encoding='utf-8').read() self.assertEqual(console_out.getvalue().decode('utf-8'), xml_content) + def test_stream_input_output_utf8_encoding(self): + """ + Run test mode simulating using pipes for input and output using + utf-8 encoding + """ + console_out = io.BytesIO() + + input_file = os.path.join(self.fixtures_path, 'cmd-001.yaml') + with io.open(input_file, 'r') as f: + with mock.patch('sys.stdout', console_out): + with mock.patch('sys.stdin', f): + cmd.main(['test']) + + xml_content = io.open(os.path.join(self.fixtures_path, 'cmd-001.xml'), + 'r', encoding='utf-8').read() + value = console_out.getvalue().decode('utf-8') + self.assertEqual(value, xml_content) + + def test_stream_input_output_ascii_encoding(self): + """ + Run test mode simulating using pipes for input and output using + ascii encoding with unicode input + """ + console_out = io.BytesIO() + console_out.encoding = 'ascii' + + input_file = os.path.join(self.fixtures_path, 'cmd-001.yaml') + with io.open(input_file, 'r') as f: + with mock.patch('sys.stdout', console_out): + with mock.patch('sys.stdin', f): + cmd.main(['test']) + + xml_content = io.open(os.path.join(self.fixtures_path, 'cmd-001.xml'), + 'r', encoding='utf-8').read() + value = console_out.getvalue().decode('ascii') + self.assertEqual(value, xml_content) + + def test_stream_output_ascii_encoding_invalid_char(self): + """ + Run test mode simulating using pipes for input and output using + ascii encoding for output with include containing a character + that cannot be converted. + """ + console_out = io.BytesIO() + console_out.encoding = 'ascii' + + input_file = os.path.join(self.fixtures_path, 'unicode001.yaml') + with io.open(input_file, 'r', encoding='utf-8') as f: + with mock.patch('sys.stdout', console_out): + with mock.patch('sys.stdin', f): + e = self.assertRaises(UnicodeError, cmd.main, ['test']) + self.assertIn("'ascii' codec can't encode character", str(e)) + def test_config_with_test(self): """ Run test mode and pass a config file @@ -271,7 +327,7 @@ class TestJenkinsGetPluginInfoError(CmdTestsBase): Verify that retrieval of information from Jenkins instance about its plugins will be skipped when run if no config file provided. """ - with mock.patch('sys.stdout'): + with mock.patch('sys.stdout', new_callable=io.BytesIO): cmd.main(['test', os.path.join(self.fixtures_path, 'cmd-001.yaml')]) self.assertFalse(get_plugins_info_mock.called) @@ -283,7 +339,7 @@ class TestJenkinsGetPluginInfoError(CmdTestsBase): plugins will be skipped when run if a config file provided and disables querying through a config option. """ - with mock.patch('sys.stdout'): + with mock.patch('sys.stdout', new_callable=io.BytesIO): cmd.main(['--conf', os.path.join(self.fixtures_path, 'disable-query-plugins.conf'),