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

Absence of the consent-state cookie is not handled correctly #12

Closed
diesieben07 opened this issue Mar 13, 2023 · 2 comments
Closed

Absence of the consent-state cookie is not handled correctly #12

diesieben07 opened this issue Mar 13, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@diesieben07
Copy link

Node/npm version(s):
Node v18.13.0
NPM 8.19.3

Package(s) version(s):

    @porscheofficial/cookie-consent-banner@3.1.1

Browser(s) version(s):
Chrome 111.0.5563.64

Description of the issue/feature:
Consent banner does not open when cookie is absent, only when cookie is present, but set to the empty string. The cause of this is the following code (source):

const cookieValueString =
    `; ${document.cookie}`.split(`; ${this.cookieName}=`).pop() ??
    "".split(";").shift();

const cookieValues = cookieValueString ? cookieValueString.split(",") : [];

This code does not work properly if this.cookieName does not occur in document.cookie.

The first template string (`; ${document.cookie}`) can never be the empty string. Even if there are no cookies set at all and document.cookie is the empty string, `; ${document.cookie}` will be '; '. Calling split on a non-empty string will never produce an empty array, as such the following pop call will always return a value. As such anything past the ?? is never executed.
Now, if document.cookie doesn't contain this.cookieName, then the result of pop will just be the entire document.cookie with ; prepended to it. This is then assumed to be the contents of the cookie and the code assumes a consent decision has already been made.

@pboeder pboeder added the bug Something isn't working label Mar 15, 2023
@pboeder
Copy link
Member

pboeder commented Mar 15, 2023

Thanks for detailed analysis, @diesieben07!
Will publish a new version in the next hours

@diesieben07
Copy link
Author

Thank you for the quick fix, however your solution only solves half of the problem. The code now checks whether document.cookie is completely empty, but this issue also arises (and does still arise in the new version) if there are other cookies set, but the consent state cookie is not present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants