CookieJar - return 'best-match' and not LIFO #7577

marq24 · 2023-09-04T12:19:27Z

What do these changes do?

The CookieJAR can contain multiple cookies with the identical name (e.g. JSESSIONID) for different paths of the same domain

the filter_cookies have to make sure that the cookie with the best-matching path will be returned. In the current implementation that last matching cookie that was inserted into the JAR will return...

I came across this issue in the situation, that a request to /auth set a SESSION_A, then redirected to / set another (different SESSION_B), redirected me again to /auth where the current implementation returned the SESSION_B for / (instead of the SESSION_A for /auth

I might do things here completely wrong - please excuse - I am far from being a python expert (I consider myself more a newbie)

Tests?

I added an additional test in the cookiejar.py test_path_filter_diff_folder_same_name_return_best_match_independent_from_put_order that fail with the current implementation, but with the change/fix it test is passed

Are there changes in behavior for the user?

IMHO - nope

Related issue number

PRNUM: #7577

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
- The format is <Name> <Surname>.
- Please keep alphabetical order, the file is sorted by names.
Add a new news fragment into the CHANGES folder
- name it <issue_id>.<type> for example (588.bugfix)
- if you don't have an issue_id change it to the pr id after creating the pr
- ensure type is one of the following:
  - .feature: Signifying a new feature.
  - .bugfix: Signifying a bug fix.
  - .doc: Signifying a documentation improvement.
  - .removal: Signifying a deprecation or removal of public API.
  - .misc: A ticket has been closed, but it is not of interest to users.
- Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

…ESSIONID) in different paths The filtered result should always return the best-matching cookie for a given (in contrast to the last inserted)

for more information, see https://pre-commit.ci

aiohttp/cookiejar.py

codecov · 2023-09-05T14:28:36Z

Codecov Report

Merging #7577 (f5ae6da) into master (81c4de1) will increase coverage by 0.09%.
Report is 3 commits behind head on master.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #7577      +/-   ##
==========================================
+ Coverage   97.26%   97.35%   +0.09%     
==========================================
  Files         106      106              
  Lines       31485    31502      +17     
  Branches     2917     3586     +669     
==========================================
+ Hits        30623    30668      +45     
+ Misses        646      630      -16     
+ Partials      216      204      -12

Flag	Coverage Δ
CI-GHA	`97.27% <100.00%> (+0.06%)`	⬆️
OS-Linux	`96.94% <100.00%> (+0.20%)`	⬆️
OS-Windows	`95.40% <100.00%> (+<0.01%)`	⬆️
OS-macOS	`96.60% <100.00%> (?)`
Py-3.10.11	`95.33% <100.00%> (+<0.01%)`	⬆️
Py-3.10.12	`?`
Py-3.10.13	`96.81% <100.00%> (?)`
Py-3.11.5	`96.51% <100.00%> (?)`
Py-3.8.10	`95.30% <100.00%> (+<0.01%)`	⬆️
Py-3.8.18	`96.74% <100.00%> (?)`
Py-3.9.13	`95.29% <100.00%> (+<0.01%)`	⬆️
Py-3.9.17	`?`
Py-3.9.18	`96.77% <100.00%> (?)`
Py-pypy7.3.11	`96.31% <100.00%> (+<0.01%)`	⬆️
VM-macos	`96.60% <100.00%> (?)`
VM-ubuntu	`96.94% <100.00%> (+0.20%)`	⬆️
VM-windows	`95.40% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
aiohttp/cookiejar.py	`98.82% <100.00%> (ø)`
tests/test_cookiejar.py	`99.15% <100.00%> (+0.04%)`	⬆️

... and 10 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

aiohttp/cookiejar.py

patchback · 2023-09-07T15:51:20Z

Backport to 3.8: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 9c932f7 on top of patchback/backports/3.8/9c932f71ec5a450954cee92ff9450974414ac1d8/pr-7577

Backporting merged PR #7577 into master

Ensure you have a local repo clone of your fork. Unless you cloned it
from the upstream, this would be your origin remote.
Make sure you have an upstream repo added as a remote too. In these
instructions you'll refer to it by the name upstream. If you don't
have it, here's how you can add it:
```
$ git remote add upstream https://github.com/aio-libs/aiohttp.git
```

Ensure you have the latest copy of upstream and prepare a branch
that will hold the backported code:

$ git fetch upstream
$ git checkout -b patchback/backports/3.8/9c932f71ec5a450954cee92ff9450974414ac1d8/pr-7577 upstream/3.8

Now, cherry-pick PR CookieJar - return 'best-match' and not LIFO #7577 contents into that branch:
```
$ git cherry-pick -x 9c932f71ec5a450954cee92ff9450974414ac1d8
```
If it'll yell at you with something like fatal: Commit 9c932f71ec5a450954cee92ff9450974414ac1d8 is a merge but no -m option was given., add -m 1 as follows instead:
```
$ git cherry-pick -m1 -x 9c932f71ec5a450954cee92ff9450974414ac1d8
```
At this point, you'll probably encounter some merge conflicts. You must
resolve them in to preserve the patch from PR CookieJar - return 'best-match' and not LIFO #7577 as close to the
original as possible.

Push this branch to your fork on GitHub:

$ git push origin patchback/backports/3.8/9c932f71ec5a450954cee92ff9450974414ac1d8/pr-7577

Create a PR, ensure that the CI is green. If it's not — update it so that
the tests and any other checks pass. This is it!
Now relax and wait for the maintainers to process your pull request
when they have some cycles to do reviews. Don't worry — they'll tell you if
any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback · 2023-09-07T15:51:25Z

Backport to 3.9: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 9c932f7 on top of patchback/backports/3.9/9c932f71ec5a450954cee92ff9450974414ac1d8/pr-7577

Backporting merged PR #7577 into master

Ensure you have a local repo clone of your fork. Unless you cloned it
from the upstream, this would be your origin remote.
Make sure you have an upstream repo added as a remote too. In these
instructions you'll refer to it by the name upstream. If you don't
have it, here's how you can add it:
```
$ git remote add upstream https://github.com/aio-libs/aiohttp.git
```

Ensure you have the latest copy of upstream and prepare a branch
that will hold the backported code:

$ git fetch upstream
$ git checkout -b patchback/backports/3.9/9c932f71ec5a450954cee92ff9450974414ac1d8/pr-7577 upstream/3.9

Now, cherry-pick PR CookieJar - return 'best-match' and not LIFO #7577 contents into that branch:
```
$ git cherry-pick -x 9c932f71ec5a450954cee92ff9450974414ac1d8
```
If it'll yell at you with something like fatal: Commit 9c932f71ec5a450954cee92ff9450974414ac1d8 is a merge but no -m option was given., add -m 1 as follows instead:
```
$ git cherry-pick -m1 -x 9c932f71ec5a450954cee92ff9450974414ac1d8
```
At this point, you'll probably encounter some merge conflicts. You must
resolve them in to preserve the patch from PR CookieJar - return 'best-match' and not LIFO #7577 as close to the
original as possible.

Push this branch to your fork on GitHub:

$ git push origin patchback/backports/3.9/9c932f71ec5a450954cee92ff9450974414ac1d8/pr-7577

Create a PR, ensure that the CI is green. If it's not — update it so that
the tests and any other checks pass. This is it!
Now relax and wait for the maintainers to process your pull request
when they have some cycles to do reviews. Don't worry — they'll tell you if
any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

Co-authored-by: marq24 <marq24@emac.de> (cherry picked from commit 9c932f7)

Dreamsorcerer · 2023-09-07T16:13:50Z

Thanks a lot. I've sorted out the backports, so it'll be in the next release.

Co-authored-by: Matthias Marquardt <marquardt24@gmail.com>

Co-authored-by: marq24 <marq24@emac.de> (cherry picked from commit 9c932f7) Co-authored-by: Matthias Marquardt <marquardt24@gmail.com>

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [aiohttp](https://togithub.com/aio-libs/aiohttp) | `==3.8.5` -> `==3.8.6` | [![age](https://developer.mend.io/api/mc/badges/age/pypi/aiohttp/3.8.6?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/aiohttp/3.8.6?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/aiohttp/3.8.5/3.8.6?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/aiohttp/3.8.5/3.8.6?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>aio-libs/aiohttp (aiohttp)</summary> ### [`v3.8.6`](https://togithub.com/aio-libs/aiohttp/blob/HEAD/CHANGES.rst#386-2023-10-07) [Compare Source](https://togithub.com/aio-libs/aiohttp/compare/v3.8.5...v3.8.6) \================== ## Security bugfixes - Upgraded the vendored copy of llhttp\_ to v9.1.3 -- by :user:`Dreamsorcerer` Thanks to :user:`kenballus` for reporting this, see GHSA-pjjw-qhg8-p2p9. .. \_llhttp: https://llhttp.org `#7647 <https://github.com/aio-libs/aiohttp/issues/7647>`\_ - Updated Python parser to comply with RFCs 9110/9112 -- by :user:`Dreamorcerer` Thanks to :user:`kenballus` for reporting this, see GHSA-gfw2-4jvh-wgfg. `#7663 <https://github.com/aio-libs/aiohttp/issues/7663>`\_ ## Deprecation - Added `fallback_charset_resolver` parameter in `ClientSession` to allow a user-supplied character set detection function. Character set detection will no longer be included in 3.9 as a default. If this feature is needed, please use `fallback_charset_resolver <https://docs.aiohttp.org/en/stable/client_advanced.html#character-set-detection>`\_. `#7561 <https://github.com/aio-libs/aiohttp/issues/7561>`\_ ## Features - Enabled lenient response parsing for more flexible parsing in the client (this should resolve some regressions when dealing with badly formatted HTTP responses). -- by :user:`Dreamsorcerer` `#7490 <https://github.com/aio-libs/aiohttp/issues/7490>`\_ ## Bugfixes - Fixed `PermissionError` when `.netrc` is unreadable due to permissions. `#7237 <https://github.com/aio-libs/aiohttp/issues/7237>`\_ - Fixed output of parsing errors pointing to a `\n`. -- by :user:`Dreamsorcerer` `#7468 <https://github.com/aio-libs/aiohttp/issues/7468>`\_ - Fixed `GunicornWebWorker` max_requests_jitter not working. `#7518 <https://github.com/aio-libs/aiohttp/issues/7518>`\_ - Fixed sorting in `filter_cookies` to use cookie with longest path. -- by :user:`marq24`. `#7577 <https://github.com/aio-libs/aiohttp/issues/7577>`\_ - Fixed display of `BadStatusLine` messages from llhttp\_. -- by :user:`Dreamsorcerer` `#7651 <https://github.com/aio-libs/aiohttp/issues/7651>`\_ *** </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/allenporter/pyrainbird).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

the jar might contain multiple matching cookies for a domain (e.g. JS…

48ef848

…ESSIONID) in different paths The filtered result should always return the best-matching cookie for a given (in contrast to the last inserted)

marq24 requested a review from asvetlov as a code owner September 4, 2023 12:19

pre-commit-ci bot and others added 2 commits September 4, 2023 12:19

[pre-commit.ci] auto fixes from pre-commit.com hooks

63f4681

for more information, see https://pre-commit.ci

added infos (after PR)

bc4626e

marq24 requested a review from webknjaz as a code owner September 4, 2023 12:25

psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Sep 4, 2023

marq24 changed the title ~~the jar might contain multiple matching cookies for a domain (e.g. JS…~~ CookieJar - return 'best-match' and not LIFO Sep 4, 2023

marq24 mentioned this pull request Sep 4, 2023

Webapi stopped pulling Data after 401 from Senecs Website marq24/ha-senec-v3#7

Closed

Dreamsorcerer reviewed Sep 4, 2023

View reviewed changes

aiohttp/cookiejar.py Outdated Show resolved Hide resolved

simplified code based on discussion in PR

9b5d5f2

Dreamsorcerer reviewed Sep 5, 2023

View reviewed changes

aiohttp/cookiejar.py Outdated Show resolved Hide resolved

Dreamsorcerer mentioned this pull request Sep 6, 2023

CookieJar improvements #7583

Open

3 tasks

feedback fom Sam Bull

10856c5

Dreamsorcerer reviewed Sep 7, 2023

View reviewed changes

aiohttp/cookiejar.py Show resolved Hide resolved

Dreamsorcerer and others added 2 commits September 7, 2023 16:23

Update aiohttp/cookiejar.py

7c3ff00

Update 7577.bugfix

f5ae6da

Dreamsorcerer added backport-3.8 backport-3.9 Trigger automatic backporting to the 3.9 release branch by Patchback robot labels Sep 7, 2023

Dreamsorcerer enabled auto-merge (squash) September 7, 2023 15:25

Dreamsorcerer merged commit 9c932f7 into aio-libs:master Sep 7, 2023
25 of 30 checks passed

Dreamsorcerer pushed a commit that referenced this pull request Sep 7, 2023

CookieJar - return 'best-match' and not LIFO (#7577)

a75c3e6

Co-authored-by: marq24 <marq24@emac.de> (cherry picked from commit 9c932f7)

Dreamsorcerer pushed a commit that referenced this pull request Sep 7, 2023

CookieJar - return 'best-match' and not LIFO (#7577)

675eab9

Co-authored-by: marq24 <marq24@emac.de> (cherry picked from commit 9c932f7)

Dreamsorcerer added a commit that referenced this pull request Sep 7, 2023

CookieJar - return 'best-match' and not LIFO (#7577) (#7587)

96c415c

Co-authored-by: Matthias Marquardt <marquardt24@gmail.com>

marq24 deleted the return-best-match branch September 7, 2023 17:08

Dreamsorcerer added a commit that referenced this pull request Sep 7, 2023

CookieJar - return 'best-match' and not LIFO (#7577) (#7588)

5946c74

Co-authored-by: marq24 <marq24@emac.de> (cherry picked from commit 9c932f7) Co-authored-by: Matthias Marquardt <marquardt24@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CookieJar - return 'best-match' and not LIFO #7577

CookieJar - return 'best-match' and not LIFO #7577

marq24 commented Sep 4, 2023 •

edited

codecov bot commented Sep 5, 2023 •

edited

patchback bot commented Sep 7, 2023 •

edited

patchback bot commented Sep 7, 2023 •

edited

Dreamsorcerer commented Sep 7, 2023

CookieJar - return 'best-match' and not LIFO #7577

CookieJar - return 'best-match' and not LIFO #7577

Conversation

marq24 commented Sep 4, 2023 • edited

What do these changes do?

Tests?

Are there changes in behavior for the user?

Related issue number

Checklist

codecov bot commented Sep 5, 2023 • edited

Codecov Report

patchback bot commented Sep 7, 2023 • edited

Backport to 3.8: 💔 cherry-picking failed — conflicts found

Backporting merged PR #7577 into master

patchback bot commented Sep 7, 2023 • edited

Backport to 3.9: 💔 cherry-picking failed — conflicts found

Backporting merged PR #7577 into master

Dreamsorcerer commented Sep 7, 2023

marq24 commented Sep 4, 2023 •

edited

codecov bot commented Sep 5, 2023 •

edited

patchback bot commented Sep 7, 2023 •

edited

patchback bot commented Sep 7, 2023 •

edited