Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Review 4] Fixes #1072: Ensuring ASCII-only in exceptions, warnings and repr() #1397

Merged
merged 1 commit into from
Sep 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,13 @@ This version contains all fixes up to pywbem 0.12.4.
is set to the built-in default namespace "root/cimv2", instead. Previously,
that was done only when not specifying the `default_namespace` argument.

* All exception and warning messages produced by pywbem now are guaranteed to
contain only ASCII characters. Unicode characters in the messages are
represented using an escape syntax such as `\\uXXXX` or `\\U00XXXXXX`.
That was also done for the result of any `__repr__()` methods of pywbem.
This is important in order to avoid secondary Unicode encoding exceptions
while a first exception or warning is processed. See issue #1072.

**Cleanup:**

* Moved class `NocaseDict` into its own module (Issue #848).
Expand Down
162 changes: 87 additions & 75 deletions pywbem/_listener.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ def main():
from .tupleparse import TupleParser
from .tupletree import xml_to_tupletree_sax
from .exceptions import ParseError, VersionError
from ._utils import _format

# CIM-XML protocol related versions implemented by the WBEM listener.
# These are returned in export message responses.
Expand Down Expand Up @@ -252,10 +253,10 @@ def do_POST(self):
# Accept header check described in DSP0200
accept = self.headers.get('Accept', 'text/xml')
if accept not in ('text/xml', 'application/xml', '*/*'):
self.send_http_error(406, 'header-mismatch',
'Invalid Accept header value: %s '
'(need text/xml, application/xml or */*)' %
accept)
self.send_http_error(
406, 'header-mismatch',
_format("Invalid Accept header value: {0} (need text/xml, "
"application/xml or */*)", accept))
return

# Accept-Charset header check described in DSP0200
Expand All @@ -268,10 +269,10 @@ def do_POST(self):
found = True
break
if not found:
self.send_http_error(406, 'header-mismatch',
'Invalid Accept-Charset header value: %s '
'(need UTF-8 or *)' %
accept_charset)
self.send_http_error(
406, 'header-mismatch',
_format("Invalid Accept-Charset header value: {0} "
"(need UTF-8 or *)", accept_charset))
return

# Accept-Encoding header check described in DSP0200
Expand All @@ -294,10 +295,10 @@ def do_POST(self):
identity_acceptable = True
break
if not identity_acceptable:
self.send_http_error(406, 'header-mismatch',
'Invalid Accept-Encoding header value: %s '
'(need Identity to be acceptable)' %
accept_encoding)
self.send_http_error(
406, 'header-mismatch',
_format("Invalid Accept-Encoding header value: {0} "
"(need Identity to be acceptable)", accept_encoding))
return

# Accept-Language header check described in DSP0200.
Expand All @@ -307,16 +308,18 @@ def do_POST(self):
# Accept-Range header check described in DSP0200
accept_range = self.headers.get('Accept-Range', None)
if accept_range is not None:
self.send_http_error(406, 'header-mismatch',
'Accept-Range header is not permitted %s' %
accept_range)
self.send_http_error(
406, 'header-mismatch',
_format("Accept-Range header is not permitted {0}",
accept_range))
return

# Content-Type header check described in DSP0200
content_type = self.headers.get('Content-Type', None)
if content_type is None:
self.send_http_error(406, 'header-mismatch',
'Content-Type header is required')
self.send_http_error(
406, 'header-mismatch',
"Content-Type header is required")
return
tc_list = re.findall(TOKEN_CHARSET_FINDALL_PATTERN, content_type)
found = False
Expand All @@ -327,20 +330,22 @@ def do_POST(self):
found = True
break
if not found:
self.send_http_error(406, 'header-mismatch',
'Invalid Content-Type header value: %s '
'(need text/xml or application/xml with '
'charset=utf-8 or empty)' %
content_type)
self.send_http_error(
406, 'header-mismatch',
_format("Invalid Content-Type header value: {0} "
"(need text/xml or application/xml with "
"charset=utf-8 or empty)",
content_type))
return

# Content-Encoding header check described in DSP0200
content_encoding = self.headers.get('Content-Encoding', 'identity')
if content_encoding.lower() != 'identity':
self.send_http_error(406, 'header-mismatch',
'Invalid Content-Encoding header value: '
'%s (listener supports only identity)' %
content_encoding)
self.send_http_error(
406, 'header-mismatch',
_format("Invalid Content-Encoding header value: {0}"
"(listener supports only identity)",
content_encoding))
return

# Content-Language header check described in DSP0200.
Expand Down Expand Up @@ -374,21 +379,19 @@ def do_POST(self):
if methodname == 'ExportIndication':

if len(params) != 1 or 'NewIndication' not in params:
self.send_error_response(msgid, methodname,
CIM_ERR_INVALID_PARAMETER,
'Expecting one parameter '
'NewIndication, got %s' %
','.join(params.keys()))
self.send_error_response(
msgid, methodname, CIM_ERR_INVALID_PARAMETER,
_format("Expecting one parameter NewIndication, got {0!A}",
params.keys()))
return

indication_inst = params['NewIndication']

if not isinstance(indication_inst, CIMInstance):
self.send_error_response(msgid, methodname,
CIM_ERR_INVALID_PARAMETER,
'NewIndication parameter is not '
'a CIM instance, but %r' %
indication_inst)
self.send_error_response(
msgid, methodname, CIM_ERR_INVALID_PARAMETER,
_format("NewIndication parameter is not a CIM instance, "
"but {0!A}", indication_inst))
return
# server.listener created in WBEMListener.start function
self.server.listener.deliver_indication(indication_inst,
Expand All @@ -397,9 +400,9 @@ def do_POST(self):
self.send_success_response(msgid, methodname)

else:
self.send_error_response(msgid, methodname,
CIM_ERR_NOT_SUPPORTED,
'Unknown export method: %s' % methodname)
self.send_error_response(
msgid, methodname, CIM_ERR_NOT_SUPPORTED,
_format("Unknown export method: {0!A}", methodname))

def send_http_error(self, http_code, cim_error=None,
cim_error_details=None, headers=None):
Expand All @@ -417,8 +420,7 @@ def send_http_error(self, http_code, cim_error=None,
for header, value in headers:
self.send_header(header, value)
self.end_headers()
self.log('%s: HTTP status %s; CIMError: %s, '
'CIMErrorDetails: %s',
self.log('%s: HTTP status %s; CIMError: %s, CIMErrorDetails: %s',
(self._get_log_prefix(), http_code, cim_error,
cim_error_details),
logging.WARNING)
Expand Down Expand Up @@ -501,35 +503,38 @@ def parse_export_request(request_str):
# Check the tuple tree

if tup_tree[0] != 'CIM':
raise ParseError('Expecting CIM element, got %s' %
tup_tree[0])
raise ParseError(
_format("Expecting CIM element, got {0}", tup_tree[0]))
dtd_version = tup_tree[1]['DTDVERSION']
if not re.match(SUPPORTED_DTD_VERSION_PATTERN, dtd_version):
raise VersionError('DTD version %s not supported. '
'Supported versions are: %s' %
(dtd_version, SUPPORTED_DTD_VERSION_STR))
raise VersionError(
_format("DTD version {0} not supported. Supported versions "
"are: {1!A}",
dtd_version, SUPPORTED_DTD_VERSION_STR))
tup_tree = tup_tree[2]

if tup_tree[0] != 'MESSAGE':
raise ParseError('Expecting MESSAGE element, got %s' %
tup_tree[0])
raise ParseError(
_format("Expecting MESSAGE element, got {0}", tup_tree[0]))
msgid = tup_tree[1]['ID']
protocol_version = tup_tree[1]['PROTOCOLVERSION']
if not re.match(SUPPORTED_PROTOCOL_VERSION_PATTERN, protocol_version):
raise VersionError('Protocol version %s not supported. '
'Supported versions are: %s' %
(protocol_version,
SUPPORTED_PROTOCOL_VERSION_STR))
raise VersionError(
_format("Protocol version {0} not supported. "
"Supported versions are: {1!A}",
protocol_version, SUPPORTED_PROTOCOL_VERSION_STR))
tup_tree = tup_tree[2]

if tup_tree[0] != 'SIMPLEEXPREQ':
raise ParseError('Expecting SIMPLEEXPREQ element, got %s' %
tup_tree[0])
raise ParseError(
_format("Expecting SIMPLEEXPREQ element, got {0}",
tup_tree[0]))
tup_tree = tup_tree[2]

if tup_tree[0] != 'EXPMETHODCALL':
raise ParseError('Expecting EXPMETHODCALL element, '
'got %s' % tup_tree[0])
raise ParseError(
_format("Expecting EXPMETHODCALL element, got {0}",
tup_tree[0]))

methodname = tup_tree[1]['NAME']
params = {}
Expand Down Expand Up @@ -575,15 +580,16 @@ def log_request(self, code='-', size='-'):

def _get_log_prefix(self):
"""Return the prefix components for a log entry"""
return '%s %s from %s' % \
(self.request_version, self.command, self.client_address[0])
return _format("{0} {1} from {2}",
self.request_version, self.command,
self.client_address[0])

def version_string(self):
"""
Overrides the inherited method to add the pywbem listener version.
"""
return 'pywbem-listener/%s %s %s ' % \
(__version__, self.server_version, self.sys_version)
return _format("pywbem-listener/{0} {1} {2} ",
__version__, self.server_version, self.sys_version)


# pylint: disable=too-many-instance-attributes
Expand Down Expand Up @@ -652,8 +658,8 @@ def __init__(self, host, http_port=None, https_port=None,
elif http_port is None:
self._http_port = http_port
else:
raise TypeError("Invalid type for http_port: %s" %
type(http_port))
raise TypeError(
_format("Invalid type for http_port: {0}", type(http_port)))

if isinstance(https_port, six.integer_types):
self._https_port = int(https_port) # Convert Python 2 long to int
Expand All @@ -662,8 +668,8 @@ def __init__(self, host, http_port=None, https_port=None,
elif https_port is None:
self._https_port = https_port
else:
raise TypeError("Invalid type for https_port: %s" %
type(https_port))
raise TypeError(
_format("Invalid type for https_port: {0}", type(https_port)))

if self._https_port is not None:
if certfile is None:
Expand All @@ -677,14 +683,15 @@ def __init__(self, host, http_port=None, https_port=None,
self._keyfile = None

if self._http_port is None and self._https_port is None:
ValueError('Listener requires at least one active port')
ValueError("Listener requires at least one active port")

self._http_server = None # ThreadedHTTPServer for HTTP
self._http_thread = None # Thread for HTTP
self._https_server = None # ThreadedHTTPServer for HTTPS
self._https_thread = None # Thread for HTTPS

self._logger = logging.getLogger('pywbem.listener.%s' % id(self))
self._logger = logging.getLogger(
_format("pywbem.listener.{0}", id(self)))

self._callbacks = [] # Registered callback functions

Expand All @@ -693,11 +700,16 @@ def __repr__(self):
Return a representation of the :class:`~pywbem.WBEMListener` object
with all attributes, that is suitable for debugging.
"""
return "%s(host=%r, http_port=%s, https_port=%s, " \
"certfile=%r, keyfile=%r, logger=%r, _callbacks=%r)" % \
(self.__class__.__name__, self.host, self.http_port,
self.https_port, self.certfile, self.keyfile, self.logger,
self._callbacks)
return _format(
"WBEMListener("
"_host={s._host!A}, "
"_http_port={s._http_port!A}, "
"_https_port={s._https_port!A}, "
"_certfile={s._certfile!A}, "
"_keyfile={s._keyfile!A}, "
"_logger={s._logger!A}, "
"_callbacks={s._callbacks!A})",
s=self)

def __enter__(self):
"""
Expand Down Expand Up @@ -847,8 +859,8 @@ def start(self):
# Windows does not raise any exception.
if getattr(exc, 'errno', None) == errno.EADDRINUSE:
# Reraise with improved error message
msg = "WBEM listener port %s already in use" % \
self._http_port
msg = _format("WBEM listener port {0} already in use",
self._http_port)
exc_type = OSError
six.reraise(exc_type, exc_type(errno.EADDRINUSE, msg),
sys.exc_info()[2])
Expand Down Expand Up @@ -876,8 +888,8 @@ def start(self):
# Windows does not raise any exception.
if getattr(exc, 'errno', None) == errno.EADDRINUSE:
# Reraise with improved error message
msg = "WBEM listener port %s already in use" % \
self._http_port
msg = _format("WBEM listener port {0} already in use",
self._http_port)
exc_type = OSError
six.reraise(exc_type, exc_type(errno.EADDRINUSE, msg),
sys.exc_info()[2])
Expand Down
17 changes: 11 additions & 6 deletions pywbem/_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@
""" # noqa: E501
# pylint: enable=line-too-long

from ._utils import _format

__all__ = ['configure_logger', 'configure_loggers_from_string',
'LOGGER_API_CALLS_NAME', 'LOGGER_HTTP_NAME', 'LOGGER_SIMPLE_NAMES',
'LOG_DESTINATIONS', 'DEFAULT_LOG_DESTINATION', 'LOG_DETAIL_LEVELS',
Expand Down Expand Up @@ -466,8 +468,9 @@ def configure_loggers_from_string(log_configuration_str,
spec_split = log_spec.strip('=').split("=")
simple_name = spec_split[0]
if not simple_name:
raise ValueError("Simple logger name missing in log spec: %r" %
log_spec)
raise ValueError(
_format("Simple logger name missing in log spec: {0}",
log_spec))
if len(spec_split) == 1:
log_dest = DEFAULT_LOG_DESTINATION
detail_level = DEFAULT_LOG_DETAIL_LEVEL
Expand All @@ -479,11 +482,13 @@ def configure_loggers_from_string(log_configuration_str,
elif len(val_split) == 2:
detail_level = val_split[1] or None
else: # len(val_split) > 2
raise ValueError("Too many components separated by : in log "
"spec: %r" % log_spec)
raise ValueError(
_format("Too many components separated by : in log spec: "
"{0}", log_spec))
else: # len(spec_split) > 2:
raise ValueError("Too many components separated by = in log spec: "
"%r" % log_spec)
raise ValueError(
_format("Too many components separated by = in log spec: "
"{0}", log_spec))

# Convert to integer, if possible
if detail_level:
Expand Down
Loading