kumar303 / mohawk

Python library for Hawk HTTP authorization
BSD 3-Clause "New" or "Revised" License
47 stars 20 forks source link

Use explicit matching for a-z, A-Z and 0-9 #33

Closed thusoy closed 8 years ago

thusoy commented 8 years ago

Swaps \w for the more explicit a-zA-Z0-9. \w also includes _, but that is already covered in the regex.

This also drops the re.LOCALE flag, which would cause whatever characters defined by the current locale as alphanumeric to be accepted as valid header values, thus causing potential differences between deployments based on locale settings.

Ref #32

kumar303 commented 8 years ago

Thanks for getting this started. Could you address the failing tests? They seem legitimate. I will take a look at your patch shortly.

thusoy commented 8 years ago

Certainly, I thought the underscore was already matched, but apparently it wasn't. This didn't show up when I ran the tests locally, since there's no explicit test for underscores in the header attributes, but it might occur as part of the nonce, which happened when run on Travis. So I figured we should probably have an explicit test for all valid characters, which led me to a different bug: Commas in the header breaks the parser. I didn't want to start re-writing the parser, but here's a test you can use:

def test_ext_with_all_valid_characters(self):
        valid_characters = "!#$%&'()*+,-./:;<=>?@[]^_`{|}~ azAZ09"
        sender = self.Sender(ext=valid_characters)
        self.receive(sender.request_header)
        parsed = parse_authorization_header(sender.request_header)
        eq_(parsed['ext'], valid_characters)

This patch should be good to go though, I added back the underscore.

kumar303 commented 8 years ago

Commas in the header breaks the parser.

Hmm, this sounds vaguely familiar but could you file a new bug for this?

kumar303 commented 8 years ago

Looks good, thanks!

thusoy commented 8 years ago

Sure, done.

escapewindow commented 7 years ago

This fixes usage of mohawk with python 3.6, which now disallows the use of re.LOCALE with non-binary strings.

It looks like this landed after the most recent mohawk release. Is there another release expected soon? I'd love to add py36 support to scriptworker, but I'm hitting this issue because scriptworker depends on taskcluster-client.py which depends on mohawk.

kumar303 commented 7 years ago

Hi @escapewindow , this change has been released in 0.3.4 https://pypi.python.org/pypi/mohawk

escapewindow commented 7 years ago

Thank you!

On Jan 7, 2017, at 12:46, Kumar McMillan notifications@github.com wrote:

Hi @escapewindow , this change has been released in 0.3.4 https://pypi.python.org/pypi/mohawk

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.