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.

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

feat(internet): Add RFC 5737 testing IPv4 support #2460

Open
wants to merge 2 commits into
base: next
Choose a base branch
from

Conversation

thomas-phillips-nz
Copy link

@thomas-phillips-nz thomas-phillips-nz commented Oct 10, 2023

Adds support for generating random testing-safe IPv4 addresses according to RFC 5737

Sometimes you need to generate an IP address but don't want the system being tested to try and resolve or send traffic to the IP

I guess this could be added as an option to the existing ipv4() function?

@ST-DDT ST-DDT added c: feature Request for new feature p: 1-normal Nothing urgent m: internet Something is referring to the internet module s: waiting for user interest Waiting for more users interested in this feature labels Oct 10, 2023
@ST-DDT ST-DDT added this to the vFuture milestone Oct 10, 2023
@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

Merging #2460 (cc9c202) into next (2d591de) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #2460   +/-   ##
=======================================
  Coverage   99.58%   99.58%           
=======================================
  Files        2823     2823           
  Lines      255497   255518   +21     
  Branches     1102     1103    +1     
=======================================
+ Hits       254434   254458   +24     
+ Misses       1035     1032    -3     
  Partials       28       28           
Files Coverage Δ
src/modules/internet/index.ts 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Oct 10, 2023

[...] Sometimes you need to generate an IP address but don't want the system being tested to try and resolve or send traffic to the IP [...]

Please don't do that with ANY data generated by Faker (emails, phone numbers, etc). This should include IP addresses as well.
See our README:

faker/README.md

Lines 46 to 48 in 350dfbf

> **Note**: Faker tries to generate realistic data and not obvious fake data.
> The generated names, addresses, emails, phone numbers, and/or other data might be coincidentally valid information.
> Please do not send any of your messages/calls to them from your test setup.

@thomas-phillips-nz
Copy link
Author

Totally fine for this to be closed if you think it's unnecessary with that guidance in the readme

@xDivisionByZerox xDivisionByZerox added the s: needs decision Needs team/maintainer decision label Oct 10, 2023
@ST-DDT
Copy link
Member

ST-DDT commented Oct 17, 2023

Team Proposal

How about the following api?

ipv4(options: {ipRange: LiteralUnion<'TEST-NET-1' | 'TEST-NET-2' | 'TEST-NET-3'>}) {
  let { ipRange } = options;
  if (ipRange != null) {
    if (ipRange === 'TEST-NET-1')
      ipRange  = '192.0.2.0/24'
    ...

    const [ipBase, mask] = ipRange.split('/');
    const maskInt = +mask;
    const maskBit = 2 ** mask -1;
    const ipBaseInt = ipToInt32(ipBase) & ^maskBit;
    const ipSuffix = faker.number.int(maskBit);
    return toIpv4(ipBaseInt + ipSuffix)
  } else {
    // current impl
  }
}

@ST-DDT ST-DDT added s: awaiting more info Additional information are requested and removed s: needs decision Needs team/maintainer decision labels Oct 17, 2023
@ST-DDT
Copy link
Member

ST-DDT commented Oct 30, 2023

@thomas-phillips-nz What do you think of our proposal?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature m: internet Something is referring to the internet module p: 1-normal Nothing urgent s: awaiting more info Additional information are requested s: waiting for user interest Waiting for more users interested in this feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants