Skip to content
This repository was archived by the owner on Feb 7, 2024. It is now read-only.

Conversation

@jhford
Copy link
Contributor

@jhford jhford commented Oct 28, 2014

As the code is currently written, it's impossible to generate a bewit for a URL unless
that URL has a query string. This restriction does not exist in the canonical implementation

https://github.com/hueniverse/hawk/blob/7b050d7920c52d1bb4b5ff815c41d60766a2a108/lib/client.js#L229

I have URLs that I wish to generate a bewit for which do not have query strings. I guess
I could add a bogus ?a=1 query string to the URI, but it looks more likely that the
indentation for this block of code is one level too many.

@jhford
Copy link
Contributor Author

jhford commented Oct 28, 2014

Fwiw, older versions of this file seem to have the same indentation as I'm proposing in this patch

@jhford
Copy link
Contributor Author

jhford commented Oct 28, 2014

What's really strange is that looking at github blame suggests that the commit that introduced this line of code (38663dc) did so with the correct indentation. Not sure what's going on here, unless it's some sort of whitespace collapsing of diffs somewhere that is messing with python indentation scoping.

@jhford
Copy link
Contributor Author

jhford commented Oct 28, 2014

In my code, I can verify that this code does fix the issue that I was having.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks for figuring this out!

@ozten
Copy link
Contributor

ozten commented Oct 29, 2014

Looks like I've been stripped of my merge button-fu.

r+

@mykmelez
Copy link

Looks like I've been stripped of my merge button-fu.

I just created the "PyHawk" team, added you to it, and gave it admin access to this repository. So you should now be able to merge this change (and administer this repository generally).

@jhford
Copy link
Contributor Author

jhford commented Nov 2, 2014

@ozten did what Myk did in #27 (comment) fix your ability to push to the repository?

ozten added a commit that referenced this pull request Nov 3, 2014
fix indentation in the get_bewit function
@ozten ozten merged commit d30ed24 into mozilla:master Nov 3, 2014
@ozten
Copy link
Contributor

ozten commented Nov 3, 2014

@mykmelez thanks for fixing the permissions issue!

@jhford jhford deleted the fix-get_bewit_indent branch November 11, 2014 13:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants