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

web.Request.clone bug #7481

Closed
1 task done
wallneradam opened this issue Aug 5, 2023 · 7 comments · Fixed by #8990
Closed
1 task done

web.Request.clone bug #7481

wallneradam opened this issue Aug 5, 2023 · 7 comments · Fixed by #8990
Labels

Comments

@wallneradam
Copy link

wallneradam commented Aug 5, 2023

Describe the bug

When I try to create a new Request object from an existing with clone and set new scheme and host parameters. The new request object's url will not change it's scheme and url.

To Reproduce

# Original request.url starts with 'http://'
request = request.clone(scheme='https', host='new_host')
logger.debug(request.url)  # This still starts with 'http://'
logger.debug(request.scheme)  # This will be 'https', so scheme has changed

Expected behavior

The new request object shold start with 'https://'

Logs/tracebacks

logger.debug(request.url)
logger.debug(request.scheme)
2023-08-06 01:05:26,217 [DEBUG] http://ipecho.net/plain
2023-08-06 01:05:26,217 [DEBUG] https

Python Version

3.11

aiohttp Version

3.8.5

multidict Version

6.0.5

yarl Version

$ python -m pip show yarl

OS

macOS

Related component

Server

Additional context

Workaround:

# call clone twice
request = request.clone(scheme='https', host='new_host')  # 1st set scheme and new host
request = request.clone(scheme='https', host='new_host', rel_url=request.rel_url)  # This will update the url of the request

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@wallneradam wallneradam added the bug label Aug 5, 2023
@Dreamsorcerer
Copy link
Member

That code was removed in 3.3, over 5 years ago:
de5fd44

No idea why though... The documentation didn't get changed, so I can't tell if it was deliberate.

@wallneradam
Copy link
Author

Which code is removed? The problem is the url is not change only if the rel_url parameter is used. But it is checked before the scheme parameter, so if you change both scheme and rel_url, or only scheme the url is not refreshed. So it is a logic bug in the clone method.

@Dreamsorcerer
Copy link
Member

The code in the commit linked above. The scheme attribute is no longer used.

@wallneradam
Copy link
Author

Tho clone method has scheme and it is used. The problem is what I described above.

https://docs.aiohttp.org/en/stable/web_reference.html#aiohttp.web.BaseRequest.scheme
This is what I'd like to do. Replace the Request object to a https one. But then if I try to use the request.url, it won't change. Only by a second clone with using rel_url parameter as well.

@Dreamsorcerer
Copy link
Member

Yes, please look at the commit I linked. The code has been removed, it won't work. request.scheme just returns the sslcontext state of the transport. It previously would return the scheme parameter if set.

@wallneradam
Copy link
Author

wallneradam commented Aug 8, 2023

I don't understand. My workaround is working. So scheme property is used. And the documentation says:

The value could be overridden by clone().

And it is true, because I can overwrite it. The problem is the Request objects url property, which is not updated, and cannot be updated, only by the 2nd clone().

@Dreamsorcerer
Copy link
Member

Dreamsorcerer commented Sep 2, 2024

OK, I realise the attribute was just changed to optimise the reify decorator.
The real issue must be that your request had an absolute URL. Not sure under what conditions that would happen, the comment in the code suggests for proxies...

But, the code seems to want the absolute URL to override:

if url.is_absolute():
# absolute URL is given,
# override auto-calculating url, host, and scheme
# all other properties should be good
self._cache["url"] = url
self._cache["host"] = url.host
self._cache["scheme"] = url.scheme
self._rel_url = url.relative()

I'm not which makes more sense, either the scheme/host need to be ignored and not changed, or the absolute url needs to get updated from the scheme/host.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants