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

cookie_manager save Response cookie will error when header location not domain info #1759

Closed
huafsud opened this issue Mar 25, 2023 · 2 comments · Fixed by #1760
Closed

cookie_manager save Response cookie will error when header location not domain info #1759

huafsud opened this issue Mar 25, 2023 · 2 comments · Fixed by #1760
Labels
fixed p: cookie_manager Targeting `cookie_manager` package s: bug Something isn't working

Comments

@huafsud
Copy link

huafsud commented Mar 25, 2023

Package

cookie_manager

Version

2.1.3

Output of flutter doctor -v

NO

Dart Version

Steps to Reproduce

final locations = response.headers[HttpHeaders.locationHeader] ?? [];
// We don't want to explicitly consider recursive redirections
// cookie handling here, because when `followRedirects` is set to false,
// users will be available to handle cookies themselves.
final isRedirectRequest = statusCode >= 300 && statusCode < 400;
if (isRedirectRequest && locations.isNotEmpty) {
await Future.wait(
locations.map(
(location) => cookieJar.saveFromResponse(
Uri.parse(location),
cookies,
),
),
);

有些网站的302 location 它是没有返回域名的, 比如返回 search.php?mod=forum ,
如果这时候 cookieJar.saveFromResponse( Uri.parse("search.php?mod=forum), cookies, ), cookieJar就会报错. 因为没有domain

Some websites may not return a domain in their 302 location header, for example, it may return search.php?mod=forum. If you use cookieJar.saveFromResponse(Uri.parse("search.php?mod=forum"), cookies) in this case, it will cause an error in cookieJar because url not contain domain

Expected Result

NO

Actual Result

uri = Uri.parse(response.realUri.origin!).resolve(location);

@huafsud huafsud added h: need triage This issue needs to be categorized s: bug Something isn't working labels Mar 25, 2023
@AlexV525 AlexV525 added p: cookie_manager Targeting `cookie_manager` package and removed h: need triage This issue needs to be categorized labels Mar 25, 2023
@huafsud
Copy link
Author

huafsud commented Mar 25, 2023

@AlexV525 有个难点在于

因为在我代码里面返回的是 'search.php?mod=forum' 结构,

很不容易判断search.php是不是个正常的 域名构造,

如果返回的是'google.com?mod=forum' ,可能很容易区分, google.com 是domain

但是google.com 和 search.php 是一样的构造.

很不容易判断 location 是否包含了domain


The difficulty lies in the fact that the returned value in my code is in the structure of 'search.php?mod=forum'.

It is not easy to determine whether 'search.php' is a valid domain construct.

If it returns 'google.com?mod=forum', it may be easy to distinguish that 'google.com' is the domain.

But 'google.com' and 'search.php' have the same construct.

It is not easy to determine whether the location contains the domain.

@AlexV525
Copy link
Member

AlexV525 commented Mar 25, 2023

Uri.parse(response.realUri.origin!).resolve(location)

目前来看这个应该是正确的解法,我会在 PR 里补充多几个测试用例。

This is the correct usage so far, I'll try to add Uri cases in the coming PR as much as possible.

AlexV525 added a commit that referenced this issue Apr 3, 2023
Fixes #1759
Fixes #1761

### New Pull Request Checklist

- [x] I have read the
[Documentation](https://pub.dev/documentation/dio/latest/)
- [x] I have searched for a similar pull request in the
[project](https://github.com/cfug/dio/pulls) and found none
- [x] I have updated this branch with the latest `main` branch to avoid
conflicts (via merge from master or rebase)
- [x] I have added the required tests to prove the fix/feature I'm
adding
- [x] I have updated the documentation (if necessary)
- [x] I have run the tests without failures
- [x] I have updated the `CHANGELOG.md` in the corresponding package

---------

Signed-off-by: Alex Li <github@alexv525.com>
@AlexV525 AlexV525 added the fixed label Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed p: cookie_manager Targeting `cookie_manager` package s: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants