Skip to content

Commit 87e21d6

Browse files
authored
fix(ec2-alpha): do not use string comparison in rangesOverlap (#32269)
### Issue #32145 Closes #32145. ### Reason for this change The rangesOverlap method was using string comparison to check if IP ranges overlapped. ### Description of changes The rangesOverlap method was updated to compare IP ranges using the ip-num library ### Description of how you validated changes Added two unit tests to verify correct behavior ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent d4f6946 commit 87e21d6

File tree

7 files changed

+103
-41
lines changed

7 files changed

+103
-41
lines changed

packages/@aws-cdk/aws-ec2-alpha/lib/util.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ export class CidrBlock {
260260
}
261261

262262
/**
263-
* Checks if two IP address ranges overlap.
263+
* Checks if two IPv4 address ranges overlap.
264264
*
265265
* @param range1 The first IP address range represented as an array [start, end].
266266
* @param range2 The second IP address range represented as an array [start, end].
@@ -269,9 +269,8 @@ export class CidrBlock {
269269
* Note: This method assumes that the start and end addresses are valid IPv4 addresses.
270270
*/
271271
public rangesOverlap(range1: [string, string], range2: [string, string]): boolean {
272-
const [start1, end1] = range1;
273-
const [start2, end2] = range2;
274-
272+
const [start1, end1] = range1.map(ip => NetworkUtils.ipToNum(ip));
273+
const [start2, end2] = range2.map(ip => NetworkUtils.ipToNum(ip));
275274
// Check if ranges overlap
276275
return start1 <= end2 && start2 <= end1;
277276
}

packages/@aws-cdk/aws-ec2-alpha/test/integ.subnet-v2.js.snapshot/aws-cdk-vpcv2-alpha-new.assets.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/aws-ec2-alpha/test/integ.subnet-v2.js.snapshot/aws-cdk-vpcv2-alpha-new.template.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"SubnetTest3296A161": {
44
"Type": "AWS::EC2::VPC",
55
"Properties": {
6-
"CidrBlock": "10.0.0.0/16",
6+
"CidrBlock": "10.1.0.0/16",
77
"EnableDnsHostnames": true,
88
"EnableDnsSupport": true,
99
"InstanceTenancy": "default"
@@ -26,7 +26,7 @@
2626
"Properties": {
2727
"AssignIpv6AddressOnCreation": false,
2828
"AvailabilityZone": "us-west-2a",
29-
"CidrBlock": "10.0.0.0/24",
29+
"CidrBlock": "10.1.0.0/20",
3030
"VpcId": {
3131
"Fn::GetAtt": [
3232
"SubnetTest3296A161",
@@ -221,7 +221,7 @@
221221
"Properties": {
222222
"AssignIpv6AddressOnCreation": false,
223223
"AvailabilityZone": "us-west-2a",
224-
"CidrBlock": "10.0.1.0/24",
224+
"CidrBlock": "10.1.128.0/20",
225225
"VpcId": {
226226
"Fn::GetAtt": [
227227
"SubnetTest3296A161",

packages/@aws-cdk/aws-ec2-alpha/test/integ.subnet-v2.js.snapshot/manifest.json

Lines changed: 49 additions & 24 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/aws-ec2-alpha/test/integ.subnet-v2.js.snapshot/tree.json

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/aws-ec2-alpha/test/integ.subnet-v2.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ const app = new cdk.App();
2020
const stack = new cdk.Stack(app, 'aws-cdk-vpcv2-alpha-new');
2121

2222
const vpc = new vpc_v2.VpcV2(stack, 'SubnetTest', {
23-
primaryAddressBlock: vpc_v2.IpAddresses.ipv4('10.0.0.0/16'),
23+
primaryAddressBlock: vpc_v2.IpAddresses.ipv4('10.1.0.0/16'),
2424
secondaryAddressBlocks: [vpc_v2.IpAddresses.amazonProvidedIpv6( {
2525
cidrBlockName: 'SecondaryTest',
2626
})],
@@ -36,7 +36,7 @@ const vpc = new vpc_v2.VpcV2(stack, 'SubnetTest', {
3636
new SubnetV2(stack, 'testSubnet1', {
3737
vpc,
3838
availabilityZone: 'us-west-2a',
39-
ipv4CidrBlock: new IpCidr('10.0.0.0/24'),
39+
ipv4CidrBlock: new IpCidr('10.1.0.0/20'),
4040
//defined on the basis of allocation done in IPAM console
4141
//ipv6CidrBlock: new Ipv6Cidr('2a05:d02c:25:4000::/60'),
4242
subnetType: SubnetType.PRIVATE_ISOLATED,
@@ -64,7 +64,7 @@ routeTable.addRoute('eigwRoute', '0.0.0.0/0', { gateway: igw });
6464
new SubnetV2(stack, 'testSubnet2', {
6565
vpc,
6666
availabilityZone: 'us-west-2a',
67-
ipv4CidrBlock: new IpCidr('10.0.1.0/24'),
67+
ipv4CidrBlock: new IpCidr('10.1.128.0/20'),
6868
routeTable: routeTable,
6969
subnetType: SubnetType.PUBLIC,
7070
});
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { CidrBlock } from '../lib/util';
2+
3+
describe('Tests for the CidrBlock.rangesOverlap method to check if IPv4 ranges overlap', () =>{
4+
test('Should return false for non-overlapping IP ranges', () => {
5+
const testCidr = new CidrBlock('10.0.0.0/16');
6+
const range1 = ['10.0.0.0', '10.0.15.255'] as [string, string];
7+
const range2 = ['10.0.128.0', '10.0.143.255'] as [string, string];
8+
expect(testCidr.rangesOverlap(range1, range2)).toBe(false);
9+
});
10+
11+
test('Should return true for overlapping IP ranges', () => {
12+
const testCidr = new CidrBlock('54.0.0.0/17');
13+
const range1 = ['54.0.0.0', '54.0.127.255'] as [string, string];
14+
const range2 = ['54.0.100.0', '54.0.192.255'] as [string, string];
15+
expect(testCidr.rangesOverlap(range1, range2)).toBe(true);
16+
});
17+
18+
test('Should return true for overlapping IP ranges where one range is completely inside the other', () => {
19+
const testCidr = new CidrBlock('10.0.0.0/16');
20+
const range1 = ['10.0.0.0', '10.0.127.255'] as [string, string];
21+
const range2 = ['10.0.64.0', '10.0.65.255'] as [string, string];
22+
expect(testCidr.rangesOverlap(range1, range2)).toBe(true);
23+
});
24+
25+
test('Should return true for overlapping IP ranges where the last IP of one range is the first IP of the other', () => {
26+
const testCidr = new CidrBlock('10.0.0.0/16');
27+
const range1 = ['10.0.0.0', '10.0.15.255'] as [string, string];
28+
const range2 = ['10.0.15.255', '10.0.255.255'] as [string, string];
29+
expect(testCidr.rangesOverlap(range1, range2)).toBe(true);
30+
});
31+
32+
test('Should return false for non-overlapping IP ranges where one range starts immediately after the other ends', () => {
33+
const testCidr = new CidrBlock('10.0.0.0/16');
34+
const range1 = ['10.0.0.0', '10.0.15.255'] as [string, string];
35+
const range2 = ['10.0.16.0', '10.0.19.255'] as [string, string];
36+
expect(testCidr.rangesOverlap(range1, range2)).toBe(false);
37+
});
38+
});

0 commit comments

Comments
 (0)