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.

Sign up for GitHub

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

Jump to bottom

OAuth1: Allow IPv6 addresses being parsed by signature #818

Merged
merged 5 commits into from Sep 6, 2022
Merged

OAuth1: Allow IPv6 addresses being parsed by signature #818

merged 5 commits into from Sep 6, 2022

Conversation

dasm
Copy link
Contributor

@dasm dasm commented May 19, 2022

This PR addresses issue with incorrectly parsing IPv6 address,
described here: #817

@dasm
OAuth1: Allow IPv6 addresses being parsed by signature
49294a6 
This PR addresses issue with incorrectly parsing IPv6 address,
described here: oauthlib#817
@dasm
Copy link
Contributor Author

dasm commented May 19, 2022

It seems like it won't be so easy fix.
If I understand RFC correctly:https://www.rfc-editor.org/rfc/rfc3986#section-3.2 authority should be allowed to have IPs as well.

Does it mean base_string_uri should be checking only for domain names and not IPs?

@dasm
Copy link
Contributor Author

dasm commented Jun 15, 2022 •
edited

Bandit failure seems to be unrelated with my changes.

@auvipy
Copy link
Contributor

auvipy commented Jun 22, 2022

it is apparent that the CI is not working, right?

@dasm
Copy link
Contributor Author

dasm commented Jun 22, 2022

@auvipy how can I rekick CI?

xek
xek approved these changes Jun 28, 2022
View reviewed changes
Copy link

@xek xek left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks!

@auvipy auvipy self-requested a review June 28, 2022 12:00
@dasm
Copy link
Contributor Author

dasm commented Jul 6, 2022

Hey @auvipy ! Do you have any feedback on this?
Thanks!

auvipy
auvipy reviewed Jul 6, 2022
View reviewed changes
Copy link
Contributor

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

I would like to introduce new CI

@dasm
Copy link
Contributor Author

dasm commented Jul 6, 2022

Sure. Do you need any help?

@dasm
Copy link
Contributor Author

dasm commented Jul 20, 2022

Hi @auvipy
Can I help somehow? How is it going?

@auvipy
Copy link
Contributor

auvipy commented Jul 20, 2022

we need to setup a new CI

@dasm
Copy link
Contributor Author

dasm commented Aug 25, 2022

Hi @auvipy!
How is it going with a new CI? Do you need any help with setting that up?

d34dh0r53
d34dh0r53 approved these changes Aug 30, 2022
View reviewed changes
Copy link

@d34dh0r53 d34dh0r53 left a comment

Choose a reason for hiding this comment

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

LGTM

@auvipy auvipy added this to the 3.2.1 milestone Aug 31, 2022
@JonathanHuot JonathanHuot merged commit b4bdd09 into oauthlib:master Sep 6, 2022
@dasm
Copy link
Contributor Author

dasm commented Sep 6, 2022

Thanks @JonathanHuot !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@auvipy auvipy auvipy left review comments

@xek xek xek approved these changes

@d34dh0r53 d34dh0r53 d34dh0r53 approved these changes

@JonathanHuot JonathanHuot JonathanHuot approved these changes

Assignees
No one assigned
Labels
OAuth1
Projects
None yet
Milestone
3.2.1
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@dasm @auvipy @xek @d34dh0r53 @JonathanHuot