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

Failed test with curl build with --with-libpsl #26

Closed
remicollet opened this issue Feb 15, 2016 · 8 comments
Closed

Failed test with curl build with --with-libpsl #26

remicollet opened this issue Feb 15, 2016 · 8 comments

Comments

@remicollet
Copy link
Collaborator

See https://apps.fedoraproject.org/koschei/package/php-pecl-http

TEST 29/182 [tests/client021.phpt]
========DIFF========
007+ Set-Cookie: counter=1;
007- Set-Cookie: counter=2;
011+ Set-Cookie: counter=1;
011- Set-Cookie: counter=3;
015+ Set-Cookie: counter=1;
015- Set-Cookie: counter=4;
019+ Set-Cookie: counter=1;
019- Set-Cookie: counter=5;
023+ Set-Cookie: counter=1;
024+ Etag: ""
025+ X-Original-Transfer-Encoding: chunked
026+ HTTP/1.1 200 OK
027+ Set-Cookie: counter=1;
028+ Etag: ""
029+ X-Original-Transfer-Encoding: chunked
030+ HTTP/1.1 200 OK
031+ Set-Cookie: counter=1;
023- Set-Cookie: counter=6;
039- Set-Cookie: counter=2;
042- HTTP/1.1 200 OK
047+ Set-Cookie: counter=1;
050+ ===DONE===
043- Set-Cookie: counter=3;
044- Etag: ""
045- X-Original-Transfer-Encoding: chunked
046- HTTP/1.1 200 OK
047- Set-Cookie: counter=4;
048- Etag: ""
049- X-Original-Transfer-Encoding: chunked
050- ===DONE===
========DONE========
FAIL client cookies [tests/client021.phpt] 

Explan,ation from curl maintainer in Fedora:

The cookie is dropped by libcurl because the domain "localhost" is recognized
as a public suffix by libpsl. You can have a look at the attached minimal
example.

@m6w6
Copy link
Owner

m6w6 commented Feb 15, 2016

bagder: setting verbose to curl will of course tell if it rejects cookies based on PSL

I'll give that a try when I come around building a libcurl with libpsl support.

@bagder
Copy link

bagder commented Feb 15, 2016

From what I can tell, the latest public suffix list is this: https://github.com/publicsuffix/list/blob/master/public_suffix_list.dat and it doesn't contain "localhost" as far as I can see, so that explanation is puzzling to me.

@remicollet
Copy link
Collaborator Author

IIRC reading https://github.com/rockdaboot/libpsl/blob/master/src/psl.c#L790

Suffix without any dot (suffix.nlabels == 1) are considered as public...

@m6w6
Copy link
Owner

m6w6 commented Feb 15, 2016

Yup.

<m6w6> okay: `cookie 'counter' dropped, domain 'localhost' is a public suffix`
<bagder> ah ok
<m6w6> https://github.com/rockdaboot/libpsl/blob/master/src/psl.c#L810
<bagder> so it says any TLD is PSL if there's only one part?
<m6w6> seems so
<bagder> that seems wrong
<bagder> I mean, you could argue that you should treat cookies in some way in that case, but saying that it is a PSL must be wrong
<bagder> surprising at least
<m6w6> yup

So either curl's usage of psl is too naive, or one could consider it a bug, that psl treats everything being just a single label as TLD.

@kdudka
Copy link

kdudka commented Feb 15, 2016

I am not sure if it is already attached -- this is the minimal example I have tried:

#if 0
gcc -lpsl $0 || exit $?
./a.out
exit $?
#endif

#include <libpsl.h>
#include <stdio.h>

static const char domain[] = "localhost";

int main()
{
    const psl_ctx_t *psl = psl_builtin();
    if (!psl)
        return 1;

    printf("psl_is_public_suffix(psl, \"%s\") = %d\n", domain,
            psl_is_public_suffix(psl, domain));

    return 0;
}

I do not know libpsl enough to tell whether it is a bug or intended behavior.

@m6w6
Copy link
Owner

m6w6 commented Feb 15, 2016

Yeah, I'm not sure, either. See rockdaboot/libpsl#48

@m6w6
Copy link
Owner

m6w6 commented Jun 21, 2016

Fixed in libcurl IIRC.

@m6w6 m6w6 closed this as completed Jun 21, 2016
@m6w6 m6w6 removed the in progress label Jun 21, 2016
@kdudka
Copy link

kdudka commented Jun 21, 2016

Yes, this was fixed via: curl/curl@curl-7_47_1-103-gc140bd7

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

No branches or pull requests

4 participants