Repository: bitcoin/bitcoin
File: test/functional/test_framework/authproxy.py (Python)

Code before:
    def batch(self, rpc_call_list):
        postdata = json.dumps(list(rpc_call_list), default=serialization_fallback, ensure_ascii=self.ensure_ascii)
        log.debug("--> " + postdata)
        response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
        if status != HTTPStatus.OK:
            raise JSONRPCException({
                'code': -342, 'message': 'non-200 HTTP status code but no JSON-RPC error'}, status)
        return response

    def _get_response(self):
        req_start_time = time.time()
        try:
            http_response = self.__conn.getresponse()
        except socket.timeout:
            raise JSONRPCException({
                'code': -344,
                'message': '%r RPC took longer than %f seconds. Consider '
                           'using larger timeout for calls that take '
                           'longer to return.' % (self._service_name,
                                                  self.__conn.timeout)})
        if http_response is None:
            raise JSONRPCException({
                'code': -342, 'message': 'missing HTTP response from server'})

        content_type = http_response.getheader('Content-Type')
        if content_type != 'application/json':
            raise JSONRPCException(
                {'code': -342, 'message': 'non-JSON HTTP response with \'%i %s\' from server' % (http_response.status, http_response.reason)},
                http_response.status)

        responsedata = http_response.read().decode('utf8')
        response = json.loads(responsedata, parse_float=decimal.Decimal)
        elapsed = time.time() - req_start_time
        if "error" in response and response["error"] is None:
            log.debug("<-%s- [%.6f] %s" % (response["id"], elapsed, json.dumps(response["result"], default=serialization_fallback, ensure_ascii=self.ensure_ascii)))
        else:
            log.debug("<-- [%.6f] %s" % (elapsed, responsedata))
        return response, http_response.status

    def __truediv__(self, relative_uri):
        return AuthServiceProxy("{}/{}".format(self.__service_url, relative_uri), self._service_name, connection=self.__conn)

    def _set_conn(self, connection=None):
        port = 80 if self.__url.port is None else self.__url.port
        if connection:
            self.__conn = connection
            self.timeout = connection.timeout
        elif self.__url.scheme == 'https':
            self.__conn = http.client.HTTPSConnection(self.__url.hostname, port, timeout=self.timeout)
        else:
            self.__conn = http.client.HTTPConnection(self.__url.hostname, port, timeout=self.timeout)

Review comment (security):
In commit "test: handle 204 NO_CONTENT correctly in authproxy" (c85c9998dc98d15a7e42a0d2eb142cb516126243)

The logic in this function seems too permissive:
- If an empty "application/json" response is returned, now it returns None instead of treating the missing JSON as an error
- It does not treat responses that have NO_CONTENT status code but do have content as errors. It will even parse json and return from NO_CONTENT responses and ignore the content_type value.

The code is also more confusing now that these cases are mixed together instead of being handled separately. Would suggest just checking for the NO_CONTENT status up front, actually verifying that no content is present, and not mixing up the no content code with the json parsing code.

Code after:
    def batch(self, rpc_call_list):
        postdata = json.dumps(list(rpc_call_list), default=serialization_fallback, ensure_ascii=self.ensure_ascii)
        log.debug("--> " + postdata)
        response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
        if status != HTTPStatus.OK:
            raise JSONRPCException({
                'code': -342, 'message': 'non-200 HTTP status code but no JSON-RPC error'}, status)
        return response

    def _get_response(self):
        req_start_time = time.time()
        try:
            http_response = self.__conn.getresponse()
        except socket.timeout:
            raise JSONRPCException({
                'code': -344,
                'message': '%r RPC took longer than %f seconds. Consider '
                           'using larger timeout for calls that take '
                           'longer to return.' % (self._service_name,
                                                  self.__conn.timeout)})
        if http_response is None:
            raise JSONRPCException({
                'code': -342, 'message': 'missing HTTP response from server'})

        # Check for no-content HTTP status code, which can be returned when an
        # RPC client requests a JSON-RPC 2.0 "notification" with no response.
        # Currently this is only possible if clients call the _request() method
        # directly to send a raw request.
        if http_response.status == HTTPStatus.NO_CONTENT:
            if len(http_response.read()) != 0:
                raise JSONRPCException({'code': -342, 'message': 'Content received with NO CONTENT status code'})
            return None, http_response.status

        content_type = http_response.getheader('Content-Type')
        if content_type != 'application/json':
            raise JSONRPCException(
                {'code': -342, 'message': 'non-JSON HTTP response with \'%i %s\' from server' % (http_response.status, http_response.reason)},
                http_response.status)

        responsedata = http_response.read().decode('utf8')
        response = json.loads(responsedata, parse_float=decimal.Decimal)
        elapsed = time.time() - req_start_time
        if "error" in response and response["error"] is None:
            log.debug("<-%s- [%.6f] %s" % (response["id"], elapsed, json.dumps(response["result"], default=serialization_fallback, ensure_ascii=self.ensure_ascii)))
        else:
            log.debug("<-- [%.6f] %s" % (elapsed, responsedata))
        return response, http_response.status

    def __truediv__(self, relative_uri):
        return AuthServiceProxy("{}/{}".format(self.__service_url, relative_uri), self._service_name, connection=self.__conn)