Accessing Response.text can fail with an AttributeError

[privatwolke]

Long story short

Handling of payloads in the server response is not consistent and can lead to AttributeErrors since code depends on methods that may not exist. I am willing to try and create a PR that fixes this issue but I'm unsure of what the intended behavior should be.

Expected behaviour

We handle payload setting consistently and don't fail when text is called after setting a string payload using the body property setter.

Actual behaviour

The setter for Response.body has support for automatic type detection via the PayloadRegistry. This means that in the above example, the body will be encoded as a StringPayload. When trying to access this via the text property we fail because StringPayload does not have a decode method.

When you set text directly any string is decoded to bytes and set to _body directly without passing through the PayloadRegistry.

Steps to reproduce

>>> from aiohttp.web_response import Response
>>> r = Response()
>>> r.body = 'asdf'
>>> r.text
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/private/tmp/.ve/lib/python3.6/site-packages/aiohttp/web_response.py", line 539, in text
    return self._body.decode(self.charset or 'utf-8')
AttributeError: 'StringPayload' object has no attribute 'decode'

Your environment

Reproduced on aiohttp 2.3.10 and 3.1.2

[asvetlov]

I see at least a logical error: r.body is a binary property, assignment a str is a bug.

[privatwolke]

Thank you for the quick response! Then the question is why PayloadRegistry has a StringPayload registered that is used if a str is passed to r.body. I agree that r.body feels like a binary property. But it's certainly not implemented that way.

Also, what should the behavior of Response.text be if any other type of payload is set?

According to documentation the body property should return bytes when in reality it returns either bytes or a Payload subclass instance.

[asvetlov]

The life is imperfect.
Let's keep the issue open as an appointment for the problem.

[privatwolke]

Maybe a feasible way would be to avoid the AttributeError thrown by text if not isinstance(Response._body, bytes) and raise a more meaningful exception?

[asvetlov]

Maybe. I need to look into the source deeper.
You can try to make a PR with fix though -- making tests passing may clean up all pitfalls.

[myusko]

@asvetlov @webknjaz someone is working on the issue? otherwise, I can take it.

[webknjaz]

@myuz feel free to give it a try

[myusko]

@webknjaz @asvetlov
I've explored a bit and come up with some suggestions.

1. The documentation said that body property receiving bytes datatype, but in real life, we also allow to pass str data type, the documentation is contradictory in that case, isn't it?

Per my understanding body was designed for bytes and text for str, am I right?

What I suggest.

1.raise an error if body received bad data type (except only bytes) it will allow us to be consistent.
2. It will avoid the problem between text <=> body in the future.

thoughts, guys?

[privatwolke]

@myuz that sounds reasonable to me :-)

[folt]

got the same error

def create_app(args: Namespace) -> Application:
    ..........
    PAYLOAD_REGISTRY.register(JsonPayload, (Mapping, MappingProxyType))
    return app

dumps = partial(json.dumps, default=convert, ensure_ascii=False)
class JsonPayload(BaseJsonPayload):
    def __init__(self,
                 value: Any,
                 encoding: str = 'utf-8',
                 content_type: str = 'application/json',
                 dumps: JSONEncoder = dumps,
                 *args: Any,
                 **kwargs: Any) -> None:
        super().__init__(value, encoding, content_type, dumps, *args, **kwargs)

@middleware
async def error_middleware(request: Request, handler):
    try:
        return await handler(request)
    except HTTPException as err:
        if not isinstance(err.body, JsonPayload):
            err = format_http_error(err.__class__, err.text)
        raise err
    except ValidationError as err:
        raise handle_validation_error(err)
    except Exception:
        log.exception('Unhandled exception')
        raise format_http_error(HTTPInternalServerError)

def format_http_error(http_error_cls, message: Optional[str] = None,
                      fields: Optional[Mapping] = None) -> HTTPException:
    status = HTTPStatus(http_error_cls.status_code)
    error = {
        'code': status.name.lower(),
        'message': message or status.description
    }
    if fields:
        error['fields'] = fields
    return http_error_cls(body={'error': error})

Traceback (most recent call last):
  File "/Users/folt/Documents/Work/geodb/.venv/lib/python3.8/site-packages/aiohttp/web_protocol.py", line 485, in start
    resp, reset = await task
  File "/Users/folt/Documents/Work/geodb/.venv/lib/python3.8/site-packages/aiohttp/web_protocol.py", line 427, in _handle_request
    status=exc.status, reason=exc.reason, text=exc.text, headers=exc.headers
  File "/Users/folt/Documents/Work/geodb/.venv/lib/python3.8/site-packages/aiohttp/web_response.py", line 650, in text
    return self._body.decode(self.charset or "utf-8")
AttributeError: 'JsonPayload' object has no attribute 'decode'