Skip to content

OAuth1: Allow IPv6 addresses being parsed by signature#818

Merged
JonathanHuot merged 5 commits intooauthlib:masterfrom
dasm:master
Sep 6, 2022
Merged

OAuth1: Allow IPv6 addresses being parsed by signature#818
JonathanHuot merged 5 commits intooauthlib:masterfrom
dasm:master

Conversation

@dasm
Copy link
Copy Markdown
Contributor

@dasm dasm commented May 19, 2022

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

This PR addresses issue with incorrectly parsing IPv6 address,
described here: oauthlib#817
@dasm
Copy link
Copy Markdown
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?

Comment thread oauthlib/oauth1/rfc5849/signature.py Outdated
Comment thread tests/oauth1/rfc5849/test_signatures.py
@dasm
Copy link
Copy Markdown
Contributor Author

dasm commented Jun 15, 2022

Bandit failure seems to be unrelated with my changes.

@auvipy
Copy link
Copy Markdown
Member

auvipy commented Jun 22, 2022

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

@dasm
Copy link
Copy Markdown
Contributor Author

dasm commented Jun 22, 2022

@auvipy how can I rekick CI?

Copy link
Copy Markdown

@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
Copy Markdown
Contributor Author

dasm commented Jul 6, 2022

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

Copy link
Copy Markdown
Member

@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
Copy Markdown
Contributor Author

dasm commented Jul 6, 2022

Sure. Do you need any help?

@dasm
Copy link
Copy Markdown
Contributor Author

dasm commented Jul 20, 2022

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

@auvipy
Copy link
Copy Markdown
Member

auvipy commented Jul 20, 2022

we need to setup a new CI

@dasm
Copy link
Copy Markdown
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?

Copy link
Copy Markdown

@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
Copy Markdown
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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants