libcurl does not accept cookies for localhost when built --with-libpsl #658
Comments
|
Agreed. Let's first work out the issue in the libpsl end and then based on that we figure what if anything we need to do here. |
|
One thing that was made clear with this: we need a way for specific requests/handles to disable the use of PSL even if that libcurl version was built to enable it by default. |
|
Better to write here ;) Looking at other consumer of libpsl (e.g. wget), it seems using psl_is_cookie_domain_acceptable could be a better solution than psl_is_public_suffix. psl_is_cookie_domain_acceptable(psl, localhost, localhost) => 1 See: http://git.savannah.gnu.org/cgit/wget.git/tree/src/cookies.c#n527 |
|
Could you try this patch and give feedback ? I am sorry, off-office in 5 mins... back in a few hours. diff --git a/lib/cookie.c b/lib/cookie.c
index c542476..3033837 100644
--- a/lib/cookie.c
+++ b/lib/cookie.c
@@ -798,9 +798,9 @@ Curl_cookie_add(struct SessionHandle *data,
/* Check if the domain is a Public Suffix and if yes, ignore the cookie.
This needs a libpsl compiled with builtin data. */
if(co->domain && !isip(co->domain) && (psl = psl_builtin()) != NULL) {
- if(psl_is_public_suffix(psl, co->domain)) {
- infof(data, "cookie '%s' dropped, domain '%s' is a public suffix\n",
- co->name, co->domain);
+ if(!psl_is_cookie_domain_acceptable(psl, domain, co->domain)) {
+ infof(data, "cookie '%s' dropped, domain '%s' must not set cookies for '%s'\n",
+ co->name, domain, co->domain);
freecookie(co);
return NULL;
} |
|
Curl_cookie_add() is sometimes called with domain=NULL - in this case we have to skip the call to psl_is_cookie_domain_acceptable(), the check should be: if(domain && co->domain && !isip(co->domain) && (psl = psl_builtin()) != NULL) {In this case test1136 fails - I think it is a failure in the test itself: In tests/log/trace1136: It is correct to accept cookies from z-1.compute-1.amazonaws.com if the cookie-domain is also z-1.compute-1.amazonaws.com. |
|
curl tests 46, 62, 179, 506, and 1216 also fail with the above patch applied. Is this expected? |
|
I still have not updated curl since I made the above change (just freshly compiled via 'make check'): |
|
Hmmm, curl --version says PSL, but no libpsl is linked here: What is going on ? |
|
Ups, why this ? I would expect when compiling curl (incl. libcurl), it would be linked with it's own library version by default... |
|
I am pretty sure that curl is linked with its own library at build time but the run-time shared library resolution is something else. If you run |
|
As for the failing test-cases, they can be actually fixed by adding the NULL-check for |
|
I have applied the following patch to fix curl in Fedora: Unless anybody objects, I would apply it upstream, too. I am not against fixing |
|
psl_is_public_suffix() needs no fix. It works regarding the rules of the Mozilla PSL. |
|
The function is said to return true if a domain is a public suffix (that's even how it is named), but it returns true even for suffixes that aren't listed as public suffixes. And according to @rockdaboot, that's how it is supposed to work. But yes. we can certainly work-around this by using a different function. |
|
psl_is_public_suffix() alone isn't enough to check whether a cookie domain is valid or not - you need some code around it, and that is what psl_is_cookie_domain_acceptable() which in turn calls psl_unregistrable_domain(). Even if I would change the behavior of psl_is_public_suffix(), the prior code would still be buggy. |
|
The problem I think you refer to, is about handling cookies with a "single label", not about public suffixes. How we should deal with them is one issue. I believe we introduced this when we removed the dot-counting for accepting cookies in commit 85b9dc8. Another issue (this one) is that libpsl says that the domain is a public suffix, when it isn't. For that, we have a work-around with another function call. Do not conflate PSL and single-label domain names. |
|
The usage of psl_is_public_suffix() prior to the above patch was a mistake/bug. The bug was recognized due to the 'single label issue' (which is not an issue regarding the PSL rules - basic discussion should be done with the maintainers/inventors of the PSL, I am not affiliated with Mozilla/PSL and thus simply the wrong person). Originally, @m6w6 asked two questions (that's what this issue is about):
And these questions are adressed with the above patch. psl_is_cookie_domain_acceptable() cares for cookie-domain==http-host and also for http-host being a single label. This may be redundant regarding the existing cookie code (and might be optimized in another patch), but it should work for the issuers cases. If you think this is wrong, please give some domain/cookie-domain examples to show that and we can further think about it. |
|
I already explained that you don't consider this to be a libpsl bug, but I do. |
|
Yes I know. But you ignore my answer. libpsl follows the PSL rules. If you consider the PSL rules to be wrong (that's how I understood you), you should discuss this with the right people. If you think the PSL rules are ok (including the implicit '*' rule that you were talking about), but the libpsl implementation is buggy, give an example and I can work on it. |
|
I don't ignore your answer, I just disagree with your take on this. I don't think Mozilla/PSL dictates how your library's function call with a specific name is supposed to work. |
Cross-filing from rockdaboot/libpsl#48
Whatever the outcome in above issue will be, I think curl needs to address this issue, too.
Currently libcurl won't accept any cookies from single-label domains, i.e. bare hostnames like
localhost, when built --with-libpsl support.Two questions which come immediately to mind are:
The text was updated successfully, but these errors were encountered: