-
Notifications
You must be signed in to change notification settings - Fork 295
Don't allow loopback IP addresses for hosts #2920
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.
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
Conversation
CydeWeys
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CydeWeys made 1 comment.
Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @gbrodman).
core/src/main/java/google/registry/flows/host/HostFlowUtils.java line 116 at r1 (raw file):
public static void validateInetAddresses(ImmutableSet<InetAddress> inetAddresses) throws EppException { if (inetAddresses == null) {
No need for this. You're always passing in a minimum of an empty set. (See implementation of getInetAddresses())
1eecb36 to
37a4ee2
Compare
gbrodman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gbrodman made 1 comment.
Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @CydeWeys).
core/src/main/java/google/registry/flows/host/HostFlowUtils.java line 116 at r1 (raw file):
Previously, CydeWeys (Ben McIlwain) wrote…
No need for this. You're always passing in a minimum of an empty set. (See implementation of
getInetAddresses())
sure
CydeWeys
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CydeWeys resolved 1 discussion.
Reviewable status: 0 of 5 files reviewed, all discussions resolved.
CydeWeys
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CydeWeys reviewed 5 files and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @gbrodman).
I don't know where in the spec these are explicitly disallowed, but it seems like good practice and we'll fail the RST tests if we don't disallow them.
37a4ee2 to
f887a50
Compare
gbrodman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually inetAddresses can be null, e.g. https://github.com/google/nomulus/blob/master/core/src/main/java/google/registry/model/host/HostCommand.java#L57
@gbrodman made 1 comment.
Reviewable status: 4 of 5 files reviewed, all discussions resolved (waiting on @CydeWeys).
CydeWeys
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CydeWeys made 1 comment.
Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @gbrodman).
core/src/main/java/google/registry/flows/host/HostFlowUtils.java line 114 at r3 (raw file):
/** Makes sure that no provided IP addresses are local / loopback addresses. */ public static void validateInetAddresses(ImmutableSet<InetAddress> inetAddresses)
Mark param with @Nullable then.
I don't know where in the spec these are explicitly disallowed, but it seems like good practice and we'll fail the RST tests if we don't disallow them.
This change is