bpo-38976: Add support for HTTP Only flag in MozillaCookieJar #17471

jacobneiltaylor · 2019-12-05T05:14:24Z

This PR adds support for the HttpOnly flag as encoded in CURL cookiejars.

This PR was mainly designed to allow the MozillaCookieJar to parse in the cookies, as previously they were considered comments and ignored.

As HttpOnly is considered a non-standard attribute, the nonstandard attribute dict was considered the most appropriate place to persist this information.

https://bugs.python.org/issue38976

jacobneiltaylor · 2020-03-24T05:07:56Z

For more information:

This is effectively the same issue discussed in https://bugs.python.org/issue2190
While Mozilla browsers no longer use this format, it is often used to store cookies for CLI tools
- An example of this is cURL, which supports this functionality: https://github.com/curl/curl/blob/curl-7_39_0/lib/cookie.c#L644

orsenthil

Thank you for your contribution, @jacobneiltaylor

I only had two brief comments. Please see if you like to address them.

I also wonder why a very old issue got resurfaced again? Is there a practical change in the curl and other clients that we missed noticing? (ref: https://bugs.python.org/msg300920)

orsenthil · 2020-03-31T02:41:09Z

Lib/http/cookiejar.py

@@ -2004,6 +2004,7 @@ class MozillaCookieJar(FileCookieJar):
    header by default (Mozilla can cope with that).

    """
+    httponly_prefix = "#HttpOnly_"


We haven't visited this module often. Do you think, we could move this class attributes to a __init__ method - The new httponly_prefix as well as the previous magic_re and header.

Just to clarify, you are asking for the class variables to be removed and the values moved into the constructor as literals?

Do you think, we could move this class attributes to a __init__ method - The new httponly_prefix as well as the previous magic_re and header.

Hi Jacob,

My original thinking was - we add a __init__ method and we move those variables to the __init__ method, then our usage with self.attr_name becomes consistent with instance variable usages.

Move them to module-level headers is fine too.

I notice that moving.compile` to the module header will add a slight performance to the module which was not present. I am fine with it, but I will try to measure the performance of the test suite, before and after to ensure there is no drastic difference with the change.

orsenthil · 2020-03-31T02:50:59Z

Lib/http/cookiejar.py

                if line == "": break

+                # detect httponly flag if present


Will a comment or a reference to an RFC help to explain why we are doing this?

jacobneiltaylor · 2020-03-31T05:08:02Z

Thanks for the feedback @orsenthil - I'll work on those code changes.

With regard to the "why", I can share some limited details on the specific use case that prompted this contribution. An in-house SSO system I work with allows users to periodically vend libcurl-compatible cookiejar files. These files are explicitly for use with console scripts that interface with internal web APIs, usually for purposes of testing, experimentation and automation. They currently work with cURL-based BASH scripts, however there is increased interest in using them with more sophisticated Python-based automation.

The cookiejar files are generated with the #HttpOnly_ prefix, which are ignored as comments under the current implementation. We have an internal workaround, but we thought it would be prudent to share a fix with the community in case others have the same or a similar use case (as niche as the use case may be).

Rebase

…ython into mozcookiejar-httponly

orsenthil · 2020-04-02T12:35:54Z

Azure Pipelines PR — #20200402.24 failed

this is make patchcheck failure, perhaps there is a extra-space or a newline some in the patch. Running it locally could help us figure it out.

jacobneiltaylor · 2020-04-07T10:50:29Z

Thanks Senthil, that is now fixed by the looks of things.

With regard to the changes made, I opted to move all the class variables to module constants instead of literals. This was because they were either relatively complex declarations (multiline strings/compiled regex objects) or were referenced in multiple locations.

Additionally it is important to note that while HTTPOnly is a standard attribute in RFC6265, these modules are designed to model the cookies as defined in RFC2965.

As such, I still believe the nonstandard attributes dictionary is the correct location to persist this information.

orsenthil · 2020-10-23T22:48:20Z

Lib/http/cookiejar.py

                if line == "": break

+                # httponly is a cookie flag as defined in rfc6265
+                # when encoded in a netscape cookie file,
+                # the line is prepended with "#HttpOnly_"


Verified this information here - https://curl.haxx.se/libcurl/c/CURLOPT_COOKIELIST.html

bedevere-bot · 2020-10-23T22:48:58Z

@orsenthil: Please replace # with GH- in the commit message next time. Thanks!

dlenski · 2020-11-15T20:42:42Z

This issue is a duplicate of bpo-2190, which means that my PR (#22798) for that one is probably now unnecessary.

…#17471) Add support for HTTP Only flag in MozillaCookieJar Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>

dlenski · 2023-09-26T21:30:18Z

Lib/http/cookiejar.py

@@ -50,10 +50,18 @@ def _debug(*args):
        logger = logging.getLogger("http.cookiejar")
    return logger.debug(*args)

-
+HTTPONLY_ATTR = "HTTPOnly"


This implementation is similar to my alternative approach in #22798 (and see bpo-2190), except:

This version:

Check if a cookie is "HTTP-only" with cookie.has_nonstandard_attr(http.cookiejar.HTTPONLY_ATTR))

Set a cookie as "HTTP-only" with cookie.set_nonstandard_attr("HTTPOnly", "")

My version:

Check with cookie.httponly.

Set with cookie.httponly = True

Add support for HTTP Only flag in MozillaCookieJar

a9c548f

the-knights-who-say-ni added the CLA signed label Dec 5, 2019

bedevere-bot added the awaiting review label Dec 5, 2019

📜🤖 Added by blurb_it.

96ce45e

csabella requested a review from orsenthil December 15, 2019 12:58

orsenthil reviewed Mar 31, 2020

View reviewed changes

orsenthil self-assigned this Mar 31, 2020

jacobneiltaylor and others added 10 commits April 2, 2020 19:43

Cleanup class variables and update comments

ef15aba

Fix reference to header variable

cded0a7

Add the HttpOnly prefix to module constants

a8a0b51

Merge pull request #1 from python/master

bc77978

Rebase

Add support for HTTP Only flag in MozillaCookieJar

48c2a8d

📜🤖 Added by blurb_it.

5013677

Cleanup class variables and update comments

4de40c1

Fix reference to header variable

a42631e

Add the HttpOnly prefix to module constants

de66b4a

Merge branch 'mozcookiejar-httponly' of github.com:jacobneiltaylor/cp…

466e0d0

…ython into mozcookiejar-httponly

Fixed whitespace

1dc6baa

orsenthil reviewed Oct 23, 2020

View reviewed changes

orsenthil approved these changes Oct 23, 2020

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Oct 23, 2020

orsenthil merged commit 16ee68d into python:master Oct 23, 2020

bedevere-bot removed the awaiting merge label Oct 23, 2020

dlenski mentioned this pull request Nov 15, 2020

bpo-2190: make MozillaCookieJar handle HTTP-only cookies #22798

Closed

dlenski reviewed Sep 26, 2023

View reviewed changes

kairi003 mentioned this pull request Jan 6, 2024

HttpOnly prefix appending feature of MozillaCookieJar almost never works #113775

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-38976: Add support for HTTP Only flag in MozillaCookieJar #17471

bpo-38976: Add support for HTTP Only flag in MozillaCookieJar #17471

jacobneiltaylor commented Dec 5, 2019 •

edited by bedevere-bot

Loading

jacobneiltaylor commented Mar 24, 2020 •

edited

Loading

orsenthil left a comment

orsenthil Mar 31, 2020

jacobneiltaylor Mar 31, 2020

orsenthil Mar 31, 2020

orsenthil Apr 2, 2020

orsenthil Mar 31, 2020

jacobneiltaylor commented Mar 31, 2020

orsenthil commented Apr 2, 2020

jacobneiltaylor commented Apr 7, 2020

orsenthil Oct 23, 2020

bedevere-bot commented Oct 23, 2020

dlenski commented Nov 15, 2020 •

edited by bedevere-bot

Loading

dlenski Sep 26, 2023 •

edited

Loading

bpo-38976: Add support for HTTP Only flag in MozillaCookieJar #17471

bpo-38976: Add support for HTTP Only flag in MozillaCookieJar #17471

Conversation

jacobneiltaylor commented Dec 5, 2019 • edited by bedevere-bot Loading

jacobneiltaylor commented Mar 24, 2020 • edited Loading

orsenthil left a comment

Choose a reason for hiding this comment

orsenthil Mar 31, 2020

Choose a reason for hiding this comment

jacobneiltaylor Mar 31, 2020

Choose a reason for hiding this comment

orsenthil Mar 31, 2020

Choose a reason for hiding this comment

orsenthil Apr 2, 2020

Choose a reason for hiding this comment

orsenthil Mar 31, 2020

Choose a reason for hiding this comment

jacobneiltaylor commented Mar 31, 2020

orsenthil commented Apr 2, 2020

jacobneiltaylor commented Apr 7, 2020

orsenthil Oct 23, 2020

Choose a reason for hiding this comment

bedevere-bot commented Oct 23, 2020

dlenski commented Nov 15, 2020 • edited by bedevere-bot Loading

dlenski Sep 26, 2023 • edited Loading

Choose a reason for hiding this comment

jacobneiltaylor commented Dec 5, 2019 •

edited by bedevere-bot

Loading

jacobneiltaylor commented Mar 24, 2020 •

edited

Loading

dlenski commented Nov 15, 2020 •

edited by bedevere-bot

Loading

dlenski Sep 26, 2023 •

edited

Loading