Skip to content

Conversation

@sxa
Copy link
Member

@sxasxa commented Jul 14, 2020

This is a fix for the issue that is stopping #14500 from being merged due to zone IDs on Linux being network interface names, and the regex for zone ID does not pass the check in net.isIP if it contains non-alphanumeric characters.

We should possibly look at a wider expansion of the characters that can be used as zone IDs but this will trap the more likely ones for now, and allow 14500 to go in - we can create an issue to extend it to a more complete list if desired. dns label added as this may have shown up as part of a lookup call during the new test in the referenced PR.

Cc @bnoordhuis, @indutny, @nodejs/streams @Trott@lpinca

Signed-off-by: Stewart Addison [email protected]

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@sxasxa added dns Issues and PRs related to the dns subsystem. net Issues and PRs related to the net subsystem. labels Jul 14, 2020
@sxasxa self-assigned this Jul 14, 2020
@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@richardlaurichardlau left a comment

Choose a reason for hiding this comment

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

Subsystem could be net over the generic lib (I'm fine either way).

Zone IDs on Linux are network interface names. The regex we use to determine valid IPs does not allow for non-alphanumeric characters in the zone ID suffix. Some machines (including the RHEL Linux/s390x machines from Marist) have zone IDs with a '.' character in them which the regex in net.isIP rejects. This changes the regex. Ref: nodejs#14500 Signed-off-by: Stewart Addison <[email protected]>
@sxa
Copy link
MemberAuthor

sxa commented Jul 14, 2020

Subsystem could be net over the generic lib (I'm fine either way).

Fair comment - changed

@sxasxa changed the title lib: allow wider regex in interface namenet: allow wider regex in interface nameJul 14, 2020
@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@mhdawsonmhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@sxasxa added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 15, 2020
mhdawson pushed a commit that referenced this pull request Jul 16, 2020
Zone IDs on Linux are network interface names. The regex we use to determine valid IPs does not allow for non-alphanumeric characters in the zone ID suffix. Some machines (including the RHEL Linux/s390x machines from Marist) have zone IDs with a '.' character in them which the regex in net.isIP rejects. This changes the regex. Ref: #14500 Signed-off-by: Stewart Addison <[email protected]> PR-URL: #34364 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
@mhdawson
Copy link
Member

CI green, landed as 9cb7293

cjihrig pushed a commit that referenced this pull request Jul 23, 2020
Zone IDs on Linux are network interface names. The regex we use to determine valid IPs does not allow for non-alphanumeric characters in the zone ID suffix. Some machines (including the RHEL Linux/s390x machines from Marist) have zone IDs with a '.' character in them which the regex in net.isIP rejects. This changes the regex. Ref: #14500 Signed-off-by: Stewart Addison <[email protected]> PR-URL: #34364 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 27, 2020
Zone IDs on Linux are network interface names. The regex we use to determine valid IPs does not allow for non-alphanumeric characters in the zone ID suffix. Some machines (including the RHEL Linux/s390x machines from Marist) have zone IDs with a '.' character in them which the regex in net.isIP rejects. This changes the regex. Ref: #14500 Signed-off-by: Stewart Addison <[email protected]> PR-URL: #34364 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
@ruyadornoruyadorno mentioned this pull request Jul 28, 2020
addaleax pushed a commit that referenced this pull request Sep 22, 2020
Zone IDs on Linux are network interface names. The regex we use to determine valid IPs does not allow for non-alphanumeric characters in the zone ID suffix. Some machines (including the RHEL Linux/s390x machines from Marist) have zone IDs with a '.' character in them which the regex in net.isIP rejects. This changes the regex. Ref: #14500 Signed-off-by: Stewart Addison <[email protected]> PR-URL: #34364 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
Zone IDs on Linux are network interface names. The regex we use to determine valid IPs does not allow for non-alphanumeric characters in the zone ID suffix. Some machines (including the RHEL Linux/s390x machines from Marist) have zone IDs with a '.' character in them which the regex in net.isIP rejects. This changes the regex. Ref: #14500 Signed-off-by: Stewart Addison <[email protected]> PR-URL: #34364 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
@codebyterecodebytere mentioned this pull request Sep 28, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.dnsIssues and PRs related to the dns subsystem.netIssues and PRs related to the net subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@sxa@nodejs-github-bot@mhdawson@bnoordhuis@Trott@addaleax@lpinca@willin@richardlau