On Thu, Jan 07, 2016 at 08:00:51PM +1000, Fraser Tweedale wrote: > On Thu, Jan 07, 2016 at 07:56:15AM +0100, Jan Cholasta wrote: > > Hi, > > > > On 6.1.2016 05:26, Fraser Tweedale wrote: > > >Happy new year, all. > > > > > >The attached patch fixes a unicode decode error triggered in some > > >locales, which causes failure of installation (and probably other > > >oprations, if locale is changed under an existing server). > > > > > >https://fedorahosted.org/freeipa/ticket/5578 > > > > It seems like this fixes only part of the issue - the installer won't crash > > anymore. But what happens if the reason phrase uses characters which are not > > in iso-8859-1 (e.g. "č", a character commonly used in Czech)? Shouldn't we > > always specify the encoding in requests, so that Dogtag does not have to > > guess? > > > In this case it will not throw an exception, but it will decode > nonsense. However, in my investigation just now of how Tomcat > decides what to send in the reason phrase, it turns out that in > future releases they will not send a reason phrase[1] at all! > > [1] > https://github.com/apache/tomcat/commit/707ab1c77f3bc189e1c3f29b641506db4c8bce37 > > (Nice to know about this in advance - I will not be surprised if > some clients break) > > I'll cut a new patch tomorrow that just ignores the reason phrase > rather than trying to decode and log it. All the info is in the > status code, after all. > > Thanks for reviewing, > Fraser > Promised new patch attached - removes the reason phrase and updates call sites accordingly.
Cheers, Fraser > > > Honza > > > > -- > > Jan Cholasta > > -- > Manage your subscription for the Freeipa-devel mailing list: > https://www.redhat.com/mailman/listinfo/freeipa-devel > Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
From 3d860dbb660844b76b9343d3c0908af89f0327ac Mon Sep 17 00:00:00 2001 From: Fraser Tweedale <ftwee...@redhat.com> Date: Wed, 6 Jan 2016 14:50:42 +1100 Subject: [PATCH] Do not decode HTTP reason phrase from Dogtag The HTTP reason phrase sent by Dogtag is assumed to be encoded in UTF-8, but the encoding used by Tomcat is dependent on system locale, causing decode errors in some locales. The reason phrase is optional and will not be sent in a future version of Tomcat[1], so do not bother decoding and returning it. [1] https://github.com/apache/tomcat/commit/707ab1c77f3bc189e1c3f29b641506db4c8bce37 Fixes: https://fedorahosted.org/freeipa/ticket/5578 --- ipapython/dogtag.py | 23 +++++++++++------------ ipaserver/install/certs.py | 7 +++---- ipaserver/plugins/dogtag.py | 44 ++++++++++++++++++++++---------------------- 3 files changed, 36 insertions(+), 38 deletions(-) diff --git a/ipapython/dogtag.py b/ipapython/dogtag.py index 010e49652687680444d18e2e8f784fb6167a0df5..1cb74719c4ce2cc97c54dc7bebfa4b32ceee14a1 100644 --- a/ipapython/dogtag.py +++ b/ipapython/dogtag.py @@ -118,14 +118,14 @@ def ca_status(ca_host=None, use_proxy=True): ca_port = 443 else: ca_port = 8443 - status, reason, headers, body = unauthenticated_https_request( + status, headers, body = unauthenticated_https_request( ca_host, ca_port, '/ca/admin/ca/getStatus') if status == 503: # Service temporarily unavailable - return reason + return status elif status != 200: raise errors.RemoteRetrieveError( - reason=_("Retrieving CA status failed: %s") % reason) + reason=_("Retrieving CA status failed with status %d") % status) return _parse_ca_status(body) @@ -136,8 +136,8 @@ def https_request(host, port, url, secdir, password, nickname, :param url: The path (not complete URL!) to post to. :param body: The request body (encodes kw if None) :param kw: Keyword arguments to encode into POST body. - :return: (http_status, http_reason_phrase, http_headers, http_body) - as (integer, unicode, dict, str) + :return: (http_status, http_headers, http_body) + as (integer, dict, str) Perform a client authenticated HTTPS request """ @@ -165,8 +165,8 @@ def http_request(host, port, url, **kw): """ :param url: The path (not complete URL!) to post to. :param kw: Keyword arguments to encode into POST body. - :return: (http_status, http_reason_phrase, http_headers, http_body) - as (integer, unicode, dict, str) + :return: (http_status, http_headers, http_body) + as (integer, dict, str) Perform an HTTP request. """ @@ -179,8 +179,8 @@ def unauthenticated_https_request(host, port, url, **kw): """ :param url: The path (not complete URL!) to post to. :param kw: Keyword arguments to encode into POST body. - :return: (http_status, http_reason_phrase, http_headers, http_body) - as (integer, unicode, dict, str) + :return: (http_status, http_headers, http_body) + as (integer, dict, str) Perform an unauthenticated HTTPS request. """ @@ -219,15 +219,14 @@ def _httplib_request( res = conn.getresponse() http_status = res.status - http_reason_phrase = unicode(res.reason, 'utf-8') http_headers = res.msg.dict http_body = res.read() conn.close() except Exception as e: raise NetworkError(uri=uri, error=str(e)) - root_logger.debug('response status %d %s', http_status, http_reason_phrase) + root_logger.debug('response status %d', http_status) root_logger.debug('response headers %s', http_headers) root_logger.debug('response body %r', http_body) - return http_status, http_reason_phrase, http_headers, http_body + return http_status, http_headers, http_body diff --git a/ipaserver/install/certs.py b/ipaserver/install/certs.py index 1591362b4b37946aa390a8a5bfd455419382a199..f74b76090bfe2670a998373e3c7cdc3c5727c465 100644 --- a/ipaserver/install/certs.py +++ b/ipaserver/install/certs.py @@ -403,12 +403,11 @@ class CertDB(object): result = dogtag.https_request( self.host_name, 8443, "/ca/ee/ca/profileSubmitSSLClient", self.secdir, password, "ipaCert", **params) - http_status, http_reason_phrase, http_headers, http_body = result + http_status, http_headers, http_body = result if http_status != 200: raise CertificateOperationError( - error=_('Unable to communicate with CMS (%s)') % - http_reason_phrase) + error=_('Unable to communicate with CMS (status %d)') % http_status) # The result is an XML blob. Pull the certificate out of that doc = xml.dom.minidom.parseString(http_body) @@ -457,7 +456,7 @@ class CertDB(object): result = dogtag.https_request( self.host_name, 8443, "/ca/ee/ca/profileSubmitSSLClient", self.secdir, password, "ipaCert", **params) - http_status, http_reason_phrase, http_headers, http_body = result + http_status, http_headers, http_body = result if http_status != 200: raise RuntimeError("Unable to submit cert request") diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py index 1a1172a3807fe2585d15cb2363e68318e83cb09f..2549aae3d7d99364cc78f44bd96897e3738ef00d 100644 --- a/ipaserver/plugins/dogtag.py +++ b/ipaserver/plugins/dogtag.py @@ -1351,8 +1351,8 @@ class ra(rabase.rabase): """ :param url: The URL to post to. :param kw: Keyword arguments to encode into POST body. - :return: (http_status, http_reason_phrase, http_headers, http_body) - as (integer, unicode, dict, str) + :return: (http_status, http_headers, http_body) + as (integer, dict, str) Perform an HTTP request. """ @@ -1362,8 +1362,8 @@ class ra(rabase.rabase): """ :param url: The URL to post to. :param kw: Keyword arguments to encode into POST body. - :return: (http_status, http_reason_phrase, http_headers, http_body) - as (integer, unicode, dict, str) + :return: (http_status, http_headers, http_body) + as (integer, dict, str) Perform an HTTPS request """ @@ -1423,7 +1423,7 @@ class ra(rabase.rabase): self.debug('%s.check_request_status()', self.fullname) # Call CMS - http_status, http_reason_phrase, http_headers, http_body = \ + http_status, http_headers, http_body = \ self._request('/ca/ee/ca/checkRequest', self.env.ca_port, requestId=request_id, @@ -1432,7 +1432,7 @@ class ra(rabase.rabase): # Parse and handle errors if http_status != 200: self.raise_certificate_operation_error('check_request_status', - detail=http_reason_phrase) + detail=http_status) parse_result = self.get_parse_result_xml(http_body, parse_check_request_result_xml) request_status = parse_result['request_status'] @@ -1508,7 +1508,7 @@ class ra(rabase.rabase): serial_number = int(serial_number, 0) # Call CMS - http_status, http_reason_phrase, http_headers, http_body = \ + http_status, http_headers, http_body = \ self._sslget('/ca/agent/ca/displayBySerial', self.env.ca_agent_port, serialNumber=str(serial_number), @@ -1518,7 +1518,7 @@ class ra(rabase.rabase): # Parse and handle errors if http_status != 200: self.raise_certificate_operation_error('get_certificate', - detail=http_reason_phrase) + detail=http_status) parse_result = self.get_parse_result_xml(http_body, parse_display_cert_xml) request_status = parse_result['request_status'] @@ -1576,7 +1576,7 @@ class ra(rabase.rabase): self.debug('%s.request_certificate()', self.fullname) # Call CMS - http_status, http_reason_phrase, http_headers, http_body = \ + http_status, http_headers, http_body = \ self._sslget('/ca/eeca/ca/profileSubmitSSLClient', self.env.ca_ee_port, profileId=profile_id, @@ -1586,7 +1586,7 @@ class ra(rabase.rabase): # Parse and handle errors if http_status != 200: self.raise_certificate_operation_error('request_certificate', - detail=http_reason_phrase) + detail=http_status) parse_result = self.get_parse_result_xml(http_body, parse_profile_submit_result_xml) # Note different status return, it's not request_status, it's error_code @@ -1655,7 +1655,7 @@ class ra(rabase.rabase): serial_number = int(serial_number, 0) # Call CMS - http_status, http_reason_phrase, http_headers, http_body = \ + http_status, http_headers, http_body = \ self._sslget('/ca/agent/ca/doRevoke', self.env.ca_agent_port, op='revoke', @@ -1667,7 +1667,7 @@ class ra(rabase.rabase): # Parse and handle errors if http_status != 200: self.raise_certificate_operation_error('revoke_certificate', - detail=http_reason_phrase) + detail=http_status) parse_result = self.get_parse_result_xml(http_body, parse_revoke_cert_xml) request_status = parse_result['request_status'] @@ -1718,7 +1718,7 @@ class ra(rabase.rabase): serial_number = int(serial_number, 0) # Call CMS - http_status, http_reason_phrase, http_headers, http_body = \ + http_status, http_headers, http_body = \ self._sslget('/ca/agent/ca/doUnrevoke', self.env.ca_agent_port, serialNumber=str(serial_number), @@ -1727,7 +1727,7 @@ class ra(rabase.rabase): # Parse and handle errors if http_status != 200: self.raise_certificate_operation_error('take_certificate_off_hold', - detail=http_reason_phrase) + detail=http_status) parse_result = self.get_parse_result_xml(http_body, parse_unrevoke_cert_xml) @@ -2030,7 +2030,7 @@ class RestClient(Backend): """Log into the REST API""" if self.cookie is not None: return - status, status_text, resp_headers, resp_body = dogtag.https_request( + status, resp_headers, resp_body = dogtag.https_request( self.ca_host, self.override_port or self.env.ca_agent_port, '/ca/rest/account/login', self.sec_dir, self.password, self.ipa_certificate_nickname, @@ -2056,8 +2056,8 @@ class RestClient(Backend): """ :param url: The URL to post to. :param kw: Keyword arguments to encode into POST body. - :return: (http_status, http_reason_phrase, http_headers, http_body) - as (integer, unicode, dict, str) + :return: (http_status, http_headers, http_body) + as (integer, dict, str) Perform an HTTPS request """ @@ -2071,7 +2071,7 @@ class RestClient(Backend): resource = os.path.join('/ca/rest', self.path, path) # perform main request - status, status_text, resp_headers, resp_body = dogtag.https_request( + status, resp_headers, resp_body = dogtag.https_request( self.ca_host, self.override_port or self.env.ca_agent_port, resource, self.sec_dir, self.password, self.ipa_certificate_nickname, @@ -2080,10 +2080,10 @@ class RestClient(Backend): if status < 200 or status >= 300: explanation = self._parse_dogtag_error(resp_body) or '' raise errors.RemoteRetrieveError( - reason=_('Non-2xx response from CA REST API: %(status)d %(status_text)s. %(explanation)s') - % {'status': status, 'status_text': status_text, 'explanation': explanation} + reason=_('Non-2xx response from CA REST API: %(status)d. %(explanation)s') + % {'status': status, 'explanation': explanation} ) - return (status, status_text, resp_headers, resp_body) + return (status, resp_headers, resp_body) class ra_certprofile(RestClient): @@ -2108,7 +2108,7 @@ class ra_certprofile(RestClient): """ Read the profile configuration from Dogtag """ - status, status_text, resp_headers, resp_body = self._ssldo( + status, resp_headers, resp_body = self._ssldo( 'GET', profile_id + '/raw') return resp_body -- 2.5.0
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
