Skip to content

Fix getting subnet of ipv4 mapped ipv6 addresses - #29835

Merged
PVince81 merged 3 commits into
masterfrom
bugfix/noid/fix-ipv4-mapped-ipv6-subnet
Nov 23, 2021
Merged

Fix getting subnet of ipv4 mapped ipv6 addresses#29835
PVince81 merged 3 commits into
masterfrom
bugfix/noid/fix-ipv4-mapped-ipv6-subnet

Conversation

@PVince81

@PVince81PVince81 commented Nov 22, 2021

Copy link
Copy Markdown
Member

⚠️ my knowledge of IPv6 is very limited, so please help double checking whether this makes sense at all

@PVince81PVince81 added the 3. to review Waiting for reviews label Nov 22, 2021
@PVince81PVince81 self-assigned this Nov 22, 2021
@PVince81
Fix getting subnet of ipv4 mapped ipv6 addresses
7e08a4a
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
nickvergessen
nickvergessen reviewed Nov 22, 2021
View reviewed changes
Comment threadlib/private/Security/Normalizer/IpAddress.php Outdated
juliusknorr
juliusknorr reviewed Nov 22, 2021
View reviewed changes
Comment threadtests/lib/Security/Normalizer/IpAddressTest.php Outdated
@PVince81
PVince81force-pushed the bugfix/noid/fix-ipv4-mapped-ipv6-subnet branch from 75fa96c to 266cc7cCompareNovember 22, 2021 14:56
come-nc
come-nc requested changes Nov 22, 2021
View reviewed changes

@come-nccome-nc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please fix the type declarations, it should please psalm as well.

Comment threadlib/private/Security/Normalizer/IpAddress.php Outdated
Comment threadlib/private/Security/Normalizer/IpAddress.php Outdated
juliusknorr
juliusknorr reviewed Nov 22, 2021
View reviewed changes
Comment threadlib/private/Security/Normalizer/IpAddress.php Outdated
juliusknorr
juliusknorr approved these changes Nov 22, 2021
View reviewed changes

@juliusknorrjuliusknorr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good apart from the phpdoc issue that psalm has. LGTM once that is resolved

@PVince81
Improve normalizer detecting IPv4 inside of IPv6
f01ad7b
The subnet for an IPv4 address inside of IPv6 is now returned in its
IPv4 form.
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
@PVince81
PVince81force-pushed the bugfix/noid/fix-ipv4-mapped-ipv6-subnet branch from 266cc7c to f01ad7bCompareNovember 22, 2021 15:46
come-nc
come-nc requested changes Nov 22, 2021
View reviewed changes
Comment threadlib/private/Security/Normalizer/IpAddress.php Outdated
@PVince81@come-nc
Type hint in IpAddress
19f41a6
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com>
@PVince81

PVince81 commented Nov 23, 2021

Copy link
Copy Markdown
MemberAuthor

failing test "log in with invalid user once fixed by admin" might be related: /drone/src/tests/acceptance/features/login.feature:38

I'll investigate

@PVince81

PVince81 commented Nov 23, 2021

Copy link
Copy Markdown
MemberAuthor

the test passes locally with docker, I wonder if it's timing related.
I'll restart CI just in case

@PVince81

PVince81 commented Nov 23, 2021

Copy link
Copy Markdown
MemberAuthor

/backport to stable23

@PVince81

PVince81 commented Nov 23, 2021

Copy link
Copy Markdown
MemberAuthor

/backport to stable22

@PVince81

PVince81 commented Nov 23, 2021

Copy link
Copy Markdown
MemberAuthor

/backport to stable21

@PVince81
PVince81 merged commit 3040386 into masterNov 23, 2021
@PVince81
PVince81 deleted the bugfix/noid/fix-ipv4-mapped-ipv6-subnet branch November 23, 2021 09:36
This was referenced Nov 23, 2021
Merged
Merged
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickvergessennickvergessennickvergessen left review comments
@juliusknorrjuliusknorrjuliusknorr approved these changes
@come-nccome-nccome-nc approved these changes
@LukasReschkeLukasReschkeAwaiting requested review from LukasReschke

Assignees

@PVince81PVince81

Labels

3. to reviewWaiting for reviews

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@PVince81@nickvergessen@juliusknorr@come-nc