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

Raising a HTTPUnauthorized caused error after upgrading from 3.6.2 to 3.7.4.post0 #5657

Closed
teslafields opened this issue Apr 29, 2021 · 6 comments
Labels
bug regression Something that used to work stopped working "as before" after upgrade reproducer: present This PR or issue contains code, which reproduce the problem described or clearly understandable STR

Comments

@teslafields
Copy link

🐞 Describe the bug

Raising a HTTPUnauthorized with a body="string" is giving an exception, as you can see in the traceback. This started to happen after I've upgraded from 3.6.2 to 3.7.4.post0. Looks to me that, as the body I return is str, it creates a StringPayload and still it tries to decode() it as it was bytes-like object

My piece of code that works in 3.6.2: raise web.HTTPUnauthorized(body='Not logged in')
Workaround to work in 3.7.4.post0: raise web.HTTPUnauthorized(body=b'Not logged in')

💡 To Reproduce

  1. Try to raise a HTTPUnauthorized with a string body

📋 Logs/tracebacks

Traceback (most recent call last):
  File "aiohttp/web_protocol.py", line 422, in _handle_request
  File "aiohttp/web_app.py", line 499, in _handle
  File "aiohttp/web_middlewares.py", line 119, in impl
  File "aiohttp_session/__init__.py", line 172, in factory
  File "aiohttp_session/__init__.py", line 154, in factory
  File "http_server.py", line 1869, in route_get_info
  File "http_server.py", line 461, in validate_identity
aiohttp.web_exceptions.HTTPUnauthorized: Unauthorized

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "aiohttp/web_protocol.py", line 485, in start
  File "aiohttp/web_protocol.py", line 427, in _handle_request
  File "aiohttp/web_response.py", line 650, in text
AttributeError: 'StringPayload' object has no attribute 'decode'

📋 Your version of the Python

$ python --version
Python 3.8.0

📋 Your version of the aiohttp/yarl/multidict distributions

aiohttp==3.7.4.post0
aiohttp-jinja2==1.4.2
aiohttp-security==0.4.0
aiohttp-session==2.9.0
multidict==5.1.0
yarl==1.6.3
@webknjaz webknjaz added regression Something that used to work stopped working "as before" after upgrade reproducer: missing This PR or issue lacks code, which reproduce the problem described or clearly understandable STR labels May 2, 2021
@webknjaz
Copy link
Member

webknjaz commented May 2, 2021

Sounds like a regression. The first step towards addressing this would be adding a regression test marked as xfail per https://pganssle-talks.github.io/xfail-lightning/. The PR should be sent against master and it can be merged with that xfail marker.
Then, somebody needs to bisect / track down what caused this change.

@paulefoe
Copy link
Member

paulefoe commented May 3, 2021

Looks like the change here
https://github.com/aio-libs/aiohttp/pull/3462/files#diff-549d85f88f5240e30a8a85dca61a84007be49b6d267f18bb85a6534a82c0f23eL85

UPD: link is not right, it was introduced earlier. I will continue to search a little bit later.

Here's reproducer:

from aiohttp import web
routes = web.RouteTableDef()
@routes.get('/', name='main')
async def hello(request):
    raise web.HTTPUnauthorized(body='Not logged in')


app = web.Application()
app.add_routes(routes)
web.run_app(app, port=8082)

Works in 3.6.3, does not work in 3.7.0

@paulefoe paulefoe added reproducer: present This PR or issue contains code, which reproduce the problem described or clearly understandable STR and removed reproducer: missing This PR or issue lacks code, which reproduce the problem described or clearly understandable STR labels May 3, 2021
@webknjaz
Copy link
Member

webknjaz commented May 5, 2021

@paulefoe it's best to submit that as a regression test marked as xfail pointing to this issue in the "reason=" arg per https://pganssle-talks.github.io/xfail-lightning/

@webknjaz
Copy link
Member

Alright, with a regression test in place, it's time to have this compatibility issue addressed.

@paulefoe
Copy link
Member

I will be looking into it tomorrow

@paulefoe
Copy link
Member

I think I found it, it's this change:

https://github.com/aio-libs/aiohttp/pull/4271/files#diff-2126b277e07e3fdd05e7a81da456accf24e5515a46c78c48a79db4eb166043e4R383-R386

In both 3.6.3 and 3.7.0 self._body inside text is a StringPayload, but previously there was no call to exc.text when returning a response.

StringPayload, however, is fairly easy to decode, we can do something like:

@property
def text(self) -> Optional[str]:
    if self._body is None:
        return None
    if isinstance(self._body, StringPayload):
        return self._body._value.decode(self._body.encoding)
    return self._body.decode(self.charset or "utf-8")

Although, then we should probably make value a public property inside Payload

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug regression Something that used to work stopped working "as before" after upgrade reproducer: present This PR or issue contains code, which reproduce the problem described or clearly understandable STR
Projects
None yet
Development

No branches or pull requests

3 participants