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
This commit is contained in:
Darragh Bailey 2015-05-15 12:52:29 +01:00 committed by Khai Do
parent 556deebe25
commit b023d7e23f
7 changed files with 160 additions and 24 deletions

View File

@ -29,6 +29,7 @@ import logging
from jenkins_jobs.constants import MAGIC_MANAGE_STRING from jenkins_jobs.constants import MAGIC_MANAGE_STRING
from jenkins_jobs.parser import YamlParser from jenkins_jobs.parser import YamlParser
from jenkins_jobs import utils
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@ -213,8 +214,9 @@ class Builder(object):
def load_files(self, fn): def load_files(self, fn):
self.parser = YamlParser(self.global_config, self.plugins_list) self.parser = YamlParser(self.global_config, self.plugins_list)
# handle deprecated behavior # handle deprecated behavior, and check that it's not a file like
if not hasattr(fn, '__iter__'): # object as these may implement the '__iter__' attribute.
if not hasattr(fn, '__iter__') or hasattr(fn, 'read'):
logger.warning( logger.warning(
'Passing single elements for the `fn` argument in ' 'Passing single elements for the `fn` argument in '
'Builder.load_files is deprecated. Please update your code ' 'Builder.load_files is deprecated. Please update your code '
@ -224,7 +226,7 @@ class Builder(object):
files_to_process = [] files_to_process = []
for path in fn: 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) files_to_process.extend([os.path.join(path, f)
for f in os.listdir(path) for f in os.listdir(path)
if (f.endswith('.yml') if (f.endswith('.yml')
@ -236,6 +238,9 @@ class Builder(object):
# definitions of macros and templates when loading all from top-level # definitions of macros and templates when loading all from top-level
unique_files = [] unique_files = []
for f in files_to_process: for f in files_to_process:
if hasattr(f, 'read'):
unique_files.append(f)
continue
rpf = os.path.realpath(f) rpf = os.path.realpath(f)
if rpf not in unique_files: if rpf not in unique_files:
unique_files.append(rpf) unique_files.append(rpf)
@ -318,6 +323,7 @@ class Builder(object):
# `output` is a file-like object # `output` is a file-like object
logger.info("Job name: %s", job.name) logger.info("Job name: %s", job.name)
logger.debug("Writing XML to '{0}'".format(output)) logger.debug("Writing XML to '{0}'".format(output))
output = utils.wrap_stream(output)
try: try:
output.write(job.output()) output.write(job.output())
except IOError as exc: except IOError as exc:

View File

@ -290,30 +290,30 @@ def execute(options, config):
plugins_list=plugins_info) plugins_list=plugins_info)
if getattr(options, 'path', None): if getattr(options, 'path', None):
if options.path == sys.stdin: if hasattr(options.path, 'read'):
logger.debug("Input file is stdin") logger.debug("Input file is stdin")
if options.path.isatty(): if options.path.isatty():
key = 'CTRL+Z' if platform.system() == 'Windows' else 'CTRL+D' key = 'CTRL+Z' if platform.system() == 'Windows' else 'CTRL+D'
logger.warn( logger.warn(
"Reading configuration from STDIN. Press %s to end input.", "Reading configuration from STDIN. Press %s to end input.",
key) key)
else:
# take list of paths
options.path = options.path.split(os.pathsep)
# take list of paths do_recurse = (getattr(options, 'recursive', False) or
options.path = options.path.split(os.pathsep) config.getboolean('job_builder', 'recursive'))
do_recurse = (getattr(options, 'recursive', False) or excludes = [e for elist in options.exclude
config.getboolean('job_builder', 'recursive')) for e in elist.split(os.pathsep)] or \
config.get('job_builder', 'exclude').split(os.pathsep)
excludes = [e for elist in options.exclude paths = []
for e in elist.split(os.pathsep)] or \ for path in options.path:
config.get('job_builder', 'exclude').split(os.pathsep) if do_recurse and os.path.isdir(path):
paths = [] paths.extend(recurse_path(path, excludes))
for path in options.path: else:
if do_recurse and os.path.isdir(path): paths.append(path)
paths.extend(recurse_path(path, excludes)) options.path = paths
else:
paths.append(path)
options.path = paths
if options.command == 'delete': if options.command == 'delete':
for job in options.name: for job in options.name:

View File

@ -27,6 +27,7 @@ from jenkins_jobs.constants import MAGIC_MANAGE_STRING
from jenkins_jobs.errors import JenkinsJobsException from jenkins_jobs.errors import JenkinsJobsException
from jenkins_jobs.registry import ModuleRegistry from jenkins_jobs.registry import ModuleRegistry
from jenkins_jobs.formatter import deep_format from jenkins_jobs.formatter import deep_format
from jenkins_jobs import utils
from jenkins_jobs.xml_config import XmlJob from jenkins_jobs.xml_config import XmlJob
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@ -69,7 +70,8 @@ class YamlParser(object):
return keep_desc return keep_desc
def parse_fp(self, fp): 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 data:
if not isinstance(data, list): if not isinstance(data, list):
raise JenkinsJobsException( raise JenkinsJobsException(

35
jenkins_jobs/utils.py Normal file
View File

@ -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)

View File

@ -0,0 +1,25 @@
<?xml version="1.0" encoding="utf-8"?>
<project>
<actions/>
<description>&lt;!-- Managed by Jenkins Job Builder --&gt;</description>
<keepDependencies>false</keepDependencies>
<blockBuildWhenDownstreamBuilding>false</blockBuildWhenDownstreamBuilding>
<blockBuildWhenUpstreamBuilding>false</blockBuildWhenUpstreamBuilding>
<concurrentBuild>false</concurrentBuild>
<canRoam>true</canRoam>
<properties/>
<scm class="hudson.scm.NullSCM"/>
<builders/>
<publishers/>
<buildWrappers>
<org.jenkinsci.plugins.preSCMbuildstep.PreSCMBuildStepsWrapper>
<buildSteps>
<hudson.tasks.Shell>
<command>#!/bin/bash
echo &quot;Unicode! ?&quot;
</command>
</hudson.tasks.Shell>
</buildSteps>
</org.jenkinsci.plugins.preSCMbuildstep.PreSCMBuildStepsWrapper>
</buildWrappers>
</project>

View File

@ -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

View File

@ -1,5 +1,5 @@
import os
import io import io
import os
import yaml import yaml
import jenkins import jenkins
@ -51,6 +51,7 @@ class TestTests(CmdTestsBase):
Run test mode and pass a non-existing configuration directory Run test mode and pass a non-existing configuration directory
""" """
args = self.parser.parse_args(['test', 'foo']) args = self.parser.parse_args(['test', 'foo'])
args.output_dir = mock.MagicMock()
self.assertRaises(IOError, cmd.execute, args, self.config) self.assertRaises(IOError, cmd.execute, args, self.config)
def test_non_existing_config_file(self): def test_non_existing_config_file(self):
@ -58,6 +59,7 @@ class TestTests(CmdTestsBase):
Run test mode and pass a non-existing configuration file Run test mode and pass a non-existing configuration file
""" """
args = self.parser.parse_args(['test', 'non-existing.yaml']) args = self.parser.parse_args(['test', 'non-existing.yaml'])
args.output_dir = mock.MagicMock()
self.assertRaises(IOError, cmd.execute, args, self.config) self.assertRaises(IOError, cmd.execute, args, self.config)
def test_non_existing_job(self): def test_non_existing_job(self):
@ -80,7 +82,7 @@ class TestTests(CmdTestsBase):
os.path.join(self.fixtures_path, os.path.join(self.fixtures_path,
'cmd-001.yaml'), 'cmd-001.yaml'),
'foo-job']) '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 cmd.execute(args, self.config) # probably better to fail here
@mock.patch('jenkins_jobs.cmd.Builder.update_job') @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) update_job_mock.assert_called_with(paths, [], output=args.output_dir)
args = self.parser.parse_args(['test', multipath]) args = self.parser.parse_args(['test', multipath])
args.output_dir = mock.MagicMock()
self.config.set('job_builder', 'recursive', 'True') self.config.set('job_builder', 'recursive', 'True')
cmd.execute(args, self.config) cmd.execute(args, self.config)
@ -172,6 +175,59 @@ class TestTests(CmdTestsBase):
'r', encoding='utf-8').read() 'r', encoding='utf-8').read()
self.assertEqual(console_out.getvalue().decode('utf-8'), xml_content) 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): def test_config_with_test(self):
""" """
Run test mode and pass a config file 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 Verify that retrieval of information from Jenkins instance about its
plugins will be skipped when run if no config file provided. 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.main(['test', os.path.join(self.fixtures_path,
'cmd-001.yaml')]) 'cmd-001.yaml')])
self.assertFalse(get_plugins_info_mock.called) 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 plugins will be skipped when run if a config file provided and disables
querying through a config option. querying through a config option.
""" """
with mock.patch('sys.stdout'): with mock.patch('sys.stdout', new_callable=io.BytesIO):
cmd.main(['--conf', cmd.main(['--conf',
os.path.join(self.fixtures_path, os.path.join(self.fixtures_path,
'disable-query-plugins.conf'), 'disable-query-plugins.conf'),