[issue26302] cookies module allows commas in keys

Jason R. Coombs added the comment: Thanks Anish for the patch, now slated for Python 3.5.2 and Python 3.6. -- resolution: -> fixed status: open -> closed ___ Python tracker ___

[issue26302] cookies module allows commas in keys

Roundup Robot added the comment: New changeset 758cb13aaa2c by Anish Shah in branch '3.5': Issue #26302: Correctly identify comma as an invalid character for a cookie (correcting regression in Python 3.5). https://hg.python.org/cpython/rev/758cb13aaa2c New changeset 91eb7ae951a1 by Jason R. Co

[issue26302] cookies module allows commas in keys

Serhiy Storchaka added the comment: LGTM. Do you want to commit the patch Jason? -- ___ Python tracker ___ ___ Python-bugs-list mailin

[issue26302] cookies module allows commas in keys

Jason R. Coombs added the comment: Looks good to me. -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https:/

[issue26302] cookies module allows commas in keys

Anish Shah added the comment: Is this patch ready to merge? -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue26302] cookies module allows commas in keys

Martin Panter added the comment: The patch looks okay to me. The inconsistency between silently rejecting cookie “morsels” and raising an exception from load() also exists in 2.7, so maybe it is not a big deal. -- ___ Python tracker

[issue26302] cookies module allows commas in keys

Anish Shah added the comment: _LegalKeyChars contains "\-" whereas _LegalChars just contains "-". On Sun, Feb 7, 2016 at 4:33 PM, Martin Panter wrote: > > Martin Panter added the comment: > > I take that back about _CookiePattern having the same bug; it uses a > different input variable. But i

[issue26302] cookies module allows commas in keys

Martin Panter added the comment: I take that back about _CookiePattern having the same bug; it uses a different input variable. But it is strange that _LegalKeyChars lists a comma, but _LegalChars omits it. -- ___ Python tracker

[issue26302] cookies module allows commas in keys

Martin Panter added the comment: The same bug is in the _CookiePattern regular expression. For illegal characters other than a comma, the CookieError does not actually seem to be raised. -- ___ Python tracker ___

[issue26302] cookies module allows commas in keys

Anish Shah added the comment: I ran regex and issuperset version on a random string. The regex one gives better performance. So, I have included the re.escape in the patch. >>> random_str = ''.join(random.choice(_LegalChars) for _ in range(10 ** 8)) >>> is_legal_key = re.compile('[%s]+' % re.e

[issue26302] cookies module allows commas in keys

Serhiy Storchaka added the comment: The regular expression is used for performance. -- ___ Python tracker ___ ___ Python-bugs-list mai

[issue26302] cookies module allows commas in keys

Martin Panter added the comment: Another option might be to do away with the regular expression (personally I like to avoid REs and code generation where practical): def _is_legal_key(key): return key and set(_LegalChars).issuperset(key) -- nosy: +martin.panter stage: needs patch -

[issue26302] cookies module allows commas in keys

Serhiy Storchaka added the comment: _LegalChars contained only characters which don't require quoting, as documented in the comment above. If _LegalChars was only used to create _is_legal_key, we would just wrote the regular expression. But it is used also in other places. In this particular c

[issue26302] cookies module allows commas in keys

Anish Shah added the comment: @serhiy.storchaka OK, I have used re.escape instead of '\'. And I have added a test too. Also, may I know why '\' can not be in _LegalChars, so that I can remember this for future purpose? -- Added file: http://bugs.python.org/file41837/issue26302_2016020

[issue26302] cookies module allows commas in keys

Serhiy Storchaka added the comment: No, _LegalChars shouldn't include "\". -- ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue26302] cookies module allows commas in keys

Jason R. Coombs added the comment: A test would be much appreciated. It's worthwhile capturing the rejection of at least one invalid character, if not several common ones. -- ___ Python tracker ___

[issue26302] cookies module allows commas in keys

Anish Shah added the comment: We just need to use '\-' instead of '-'. ``` >>> regex = re.compile("[a-z]") >>> bool(regex.match('b')) True >>> regex = re.compile("[a\-z]") >>> bool(regex.match('b')) False ``` I have uploaded a patch. Let me know if this needs some tests too? -- keyword

[issue26302] cookies module allows commas in keys

Changes by Anish Shah : -- nosy: +anish.shah ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.pytho

[issue26302] cookies module allows commas in keys

Kunal Grover added the comment: Hi, I am a newcomer here, I am interested to work on this issue. -- nosy: +Kunal Grover ___ Python tracker ___ ___

[issue26302] cookies module allows commas in keys

Changes by Serhiy Storchaka : -- keywords: +3.5regression ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https

[issue26302] cookies module allows commas in keys

Serhiy Storchaka added the comment: Ah, how can I missed this catch? The simplest way is just move "-" to the start or the end of character list. The most error-proof way is to use re.escape(). -- components: +Library (Lib) keywords: +easy -3.5regression nosy: +demian.brecht stage: ->

[issue26302] cookies module allows commas in keys

New submission from Jason R. Coombs: Commas aren't legal characters in cookie keys, yet in Python 3.5, they're allowed: >>> bool(http.cookies._is_legal_key(',')) True The issue lies in the use of _LegalChars constructing a regular expression. "Some people, when confronted with a problem, thin