Add URL with fragment to ClientResponse

[hdk5]

What do these changes do?

ClientResponse and RequestInfo now have real_url property, which is request url without fragment part being stripped

Checklist

• I think the code is well written
• Unit tests for the changes exist
• Documentation reflects the changes
• If you provide code modification, please add yourself to CONTRIBUTORS.txt
• Add a new news fragment into the CHANGES folder

[hdk5]

I had a trouble retrieving URL fragment from response URL. I am aware that request should be sent without URL fragment, but I don't think that URL from response should be modified.

[codecov-io]

Codecov Report

Merging #2925 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2925      +/-   ##
==========================================
+ Coverage   97.99%   97.99%   +<.01%     
==========================================
  Files          40       40              
  Lines        7514     7520       +6     
  Branches     1318     1318              
==========================================
+ Hits         7363     7369       +6     
  Misses         48       48              
  Partials      103      103

Impacted Files	Coverage Δ
aiohttp/client_reqrep.py	97.27% <100%> (+0.02%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 544716c...aac2dc4. Read the comment docs.

[asvetlov]

HTTP response has no URL (unless it is not 3xx redirection).
ClientResponse.url is an URL of request.
I think that keeping a fragment part in response:

1. Makes inconsistency mess.
2. Breaks existing API

[hdk5]

unless it is not 3xx redirection

This is what i meant. I need to know full URL I am getting redirected to because this is how I get application token.

Here is a little example: https://dry-meadow-80563.herokuapp.com/tokenask

It redirects to /tokengive#token=hereisyourtoken, and now it is not possible to get #token=hereisyourtoken part from response object.

[hdk5]

Also, this code before my commit looks strange to me:

        self.url = url.with_fragment(None)
        self.original_url = url

Both url and original_url are not modified after (at least by the object itself), so they are always the same.

And then original_url is passed as url for ClientResponse.

So, wasn't it actually intended to keep URL fragment in response object?

[asvetlov]

Let's consider ClientRequest as private API. I don't care too much about its attributes.
The library needs a public client request object but it is another story.

The public API is ClientResponse and resp.history for looking on redirection URLs.
The history is a list of RequestInfo data structures.
I propose adding a new attribute called real_url (unstripped_url or whatever) to save URL with a fragment.

By this, we will keep the behavior of existing code and provide a way for getting original unmodified URLs.

Opinions?

[hdk5]

I propose adding a new attribute called real_url

This might be a solution.

But can you please explain my previous comment on code and this commit?

[asvetlov]

Looks like @fafhrd91 wanted to implement exactly your initial intention, later we broke the behavior again.
I see two reasons for it:

1. Logical complexity: there is no obvious answer to the question: what is response's URL. I have a feeling that we need two properties.
2. The mentioned commit has no tests/documentation update, that's why we did break it insensibly.

[hdk5]

Should I just put real url in real url attribute then, like you suggested, and update docs?

[asvetlov]

I think we need both ClientResponse.real_url and RequestInfo.real_url.

Documentation and tests are mandatory.

[asvetlov]

Please add .. versionadded:: 3.2 tag

[asvetlov]

.. versionadded:: 3.2

[asvetlov]

Thanks!

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new