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

Strip leading . from cookie domain names. #1041

Merged
merged 6 commits into from
May 20, 2022
Merged

Conversation

icemac
Copy link
Member

@icemac icemac commented May 17, 2022

If the value of the domain attribute in the cookie starts with . Zope currently breaks.
But according to https://www.rfc-editor.org/rfc/rfc6265#section-4.1.2.3 it should be allowed.

I added a test and a fix, without the fix the new test breaks with:

Error in test test_domain (ZPublisher.tests.test_cookie.CookieParameterRegistryTests)
Traceback (most recent call last):
  File "/.../lib/python3.7/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/.../lib/python3.7/unittest/case.py", line 628, in run
    testMethod()
  File "/.../Zope/src/ZPublisher/tests/test_cookie.py", line 150, in test_domain
    _, v = convertCookieParameter("domain", ".zope.dev")
  File "/.../Zope/src/ZPublisher/cookie.py", line 152, in convert
    return nn, (value if value is None else self[nn](value))
  File "/.../Zope/src/ZPublisher/cookie.py", line 249, in domain_converter
    value = ".".join(to_str(ToASCII(nameprep(c))) for c in u_value.split("."))
  File "/.../Zope/src/ZPublisher/cookie.py", line 249, in <genexpr>
    value = ".".join(to_str(ToASCII(nameprep(c))) for c in u_value.split("."))
  File "/.../lib/python3.7/encodings/idna.py", line 73, in ToASCII
    raise UnicodeError("label empty or too long")
UnicodeError: label empty or too long

@icemac icemac added the bug label May 17, 2022
@icemac icemac requested a review from d-maurer May 17, 2022 15:42
@icemac icemac self-assigned this May 17, 2022
src/ZPublisher/cookie.py Outdated Show resolved Hide resolved
src/ZPublisher/cookie.py Outdated Show resolved Hide resolved
@d-maurer
Copy link
Contributor

d-maurer commented May 18, 2022 via email

@icemac icemac changed the title Allow leading . in cookie domain names. Strip leading . from cookie domain names. May 19, 2022
@dataflake
Copy link
Member

I'm going to merge this, #1033 is a separate issue for a separate PR.

@dataflake dataflake merged commit bc81ab4 into master May 20, 2022
@dataflake dataflake deleted the fix-cookie-leading-dot branch May 20, 2022 10:21
dataflake added a commit that referenced this pull request May 20, 2022
* Allow leading `.` in cookie domain names.

* Add PR reference.

* Strip leading dot as suggested by @d-maurer.

* Make linter happy.

* - small cleanups

Co-authored-by: Jens Vagelpohl <jens@plyp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants