Skip to content

Commit ce235a9

Browse files
sugmanuesugmanue
authored
Improve the IP addresses detection (#5911)
* Improve the IP addresses detection The previous code relied on the exception thrown by `Integer.parseInt` to detect when the given segment was not a integer number. This uses exception as control flow and it can be very inefficient as creating an exception is costly. Instead of using `Integer.parseInt` we just handle the few cases that we care about depending on the length of the segment. If the length is one then check that the char is a digit, if the length is two then check that both chars are digits, if the length is 3, check that all the chars are digits and that the converted number is less than 255. * Address PR comments * Remove unused imports
1 parent 57af9bc commit ce235a9

File tree

4 files changed

+254
-25
lines changed

4 files changed

+254
-25
lines changed

‎codegen/src/main/resources/software/amazon/awssdk/codegen/rules2/RuleUrl.java.resource

+72-25
Original file line numberDiff line numberDiff line change
@@ -48,41 +48,88 @@ public final class RuleUrl {
4848
String path = parsed.getPath();
4949
if (parsed.getQuery() != null) {
5050
return null;
51-
5251
}
53-
boolean isIpAddr = false;
5452
String host = parsed.getHost();
53+
boolean isIpAddr = isIpAddr(host);
54+
String normalizedPath = normalizePath(path);
55+
return new RuleUrl(parsed.getProtocol(), parsed.getAuthority(), path, normalizedPath, isIpAddr);
56+
}
57+
58+
static boolean isIpAddr(String host) {
5559
if (host.startsWith("[") && host.endsWith("]")) {
56-
isIpAddr = true;
60+
return true;
5761
}
58-
String[] dottedParts = host.split("\\.");
59-
if (dottedParts.length == 4) {
60-
if (Arrays.stream(dottedParts).allMatch(part -> {
61-
try {
62-
int value = Integer.parseInt(part);
63-
return value >= 0 && value <= 255;
64-
} catch (NumberFormatException ex) {
62+
int from = 0;
63+
int segments = 0;
64+
boolean done = false;
65+
while (!done) {
66+
int index = host.indexOf('.', from);
67+
if (index == -1) {
68+
if (segments != 3) {
69+
// E.g., 123.com
6570
return false;
6671
}
67-
})) {
68-
isIpAddr = true;
72+
index = host.length();
73+
done = true;
74+
} else if (segments == 3) {
75+
// E.g., 1.2.3.4.5
76+
return false;
6977
}
78+
int length = index - from;
79+
if (length == 1) {
80+
char ch0 = host.charAt(from);
81+
if (ch0 < '0' || ch0 > '9') {
82+
return false;
83+
}
84+
} else if (length == 2) {
85+
char ch0 = host.charAt(from);
86+
char ch1 = host.charAt(from + 1);
87+
if ((ch0 <= '0' || ch0 > '9') || (ch1 < '0' || ch1 > '9')) {
88+
return false;
89+
}
90+
} else if (length == 3) {
91+
char ch0 = host.charAt(from);
92+
char ch1 = host.charAt(from + 1);
93+
char ch2 = host.charAt(from + 2);
94+
if ((ch0 <= '0' || ch0 > '9')
95+
|| (ch1 < '0' || ch1 > '9')
96+
|| (ch2 < '0' || ch2 > '9')
97+
) {
98+
return false;
99+
}
100+
int value = ((ch0 - '0') * 100) + ((ch1 - '0') * 10) + (ch2 - '0');
101+
if (value > 255) {
102+
return false;
103+
}
104+
} else {
105+
return false;
106+
}
107+
from = index + 1;
108+
segments += 1;
70109
}
71-
String normalizedPath;
110+
return true;
111+
}
112+
113+
static String normalizePath(String path) {
72114
if (StringUtils.isBlank(path)) {
73-
normalizedPath = "/";
74-
} else {
75-
StringBuilder builder = new StringBuilder();
76-
if (!path.startsWith("/")) {
77-
builder.append("/");
78-
}
79-
builder.append(path);
80-
if (!path.endsWith("/")) {
81-
builder.append("/");
82-
}
83-
normalizedPath = builder.toString();
115+
return "/";
84116
}
85117

86-
return new RuleUrl(parsed.getProtocol(), parsed.getAuthority(), path, normalizedPath, isIpAddr);
118+
boolean startsWithSlash = path.startsWith("/");
119+
boolean endsWithSlash = path.endsWith("/");
120+
121+
if (startsWithSlash && endsWithSlash) {
122+
return path;
123+
}
124+
125+
StringBuilder builder = new StringBuilder();
126+
if (!startsWithSlash) {
127+
builder.append("/");
128+
}
129+
builder.append(path);
130+
if (!endsWithSlash) {
131+
builder.append("/");
132+
}
133+
return builder.toString();
87134
}
88135
}

‎test/codegen-generated-classes-test/src/main/resources/codegen-resources/compiledrules/customization.config

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"enableGenerateCompiledEndpointRules": true
3+
}

‎test/codegen-generated-classes-test/src/main/resources/codegen-resources/compiledrules/service-2.json

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
{
2+
"version":"2.0",
3+
"metadata":{
4+
"apiVersion":"2016-03-11",
5+
"endpointPrefix":"compiledendpointrules",
6+
"jsonVersion":"1.1",
7+
"protocol":"rest-json",
8+
"serviceAbbreviation":"CompiledEndpointRulesService",
9+
"serviceFullName":"Compiled Endpoin tRules",
10+
"serviceId":"CompiledEndpointRulesService",
11+
"signatureVersion":"v4",
12+
"targetPrefix":"compiled-endpoint-rules-service",
13+
"timestampFormat":"unixTimestamp",
14+
"uid":"restjson-2016-03-11"
15+
},
16+
"operations":{
17+
"OneOperation":{
18+
"name":"OneOperation",
19+
"http":{
20+
"method":"POST",
21+
"requestUri":"/test/uri"
22+
},
23+
"input":{"shape":"OneShape"}
24+
}
25+
},
26+
"shapes": {
27+
"OneShape": {
28+
"type": "structure",
29+
"members": {
30+
"StringMember": {
31+
"shape": "String"
32+
}
33+
}
34+
},
35+
"String":{"type":"string"}
36+
}
37+
}

‎test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/compiledendpointrules/endpoints/internal/RuleUrlTest.java

+142
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.services.compiledendpointrules.endpoints.internal;
17+
18+
import static org.junit.jupiter.api.Assertions.assertEquals;
19+
20+
import java.util.Arrays;
21+
import java.util.Collection;
22+
import org.junit.jupiter.params.ParameterizedTest;
23+
import org.junit.jupiter.params.provider.MethodSource;
24+
25+
class RuleUrlTest {
26+
27+
public static Collection<TestIsIpCase> testIsIpCases() {
28+
return Arrays.asList(
29+
new TestIsIpCase("0.0.0.0", true)
30+
, new TestIsIpCase("0.1.1.1", true)
31+
, new TestIsIpCase("1.0.1.1", true)
32+
, new TestIsIpCase("1.1.0.1", true)
33+
, new TestIsIpCase("1.1.1.0", true)
34+
, new TestIsIpCase("127.0.0.1", true)
35+
, new TestIsIpCase("132.248.181.171", true)
36+
37+
// Starts [ and ends with ]
38+
// No much validation other than that.
39+
, new TestIsIpCase("[::1]", true)
40+
, new TestIsIpCase("[no-much-validation-inside]", true)
41+
42+
// Segment value is > 255
43+
, new TestIsIpCase("256.1.1.1", false)
44+
, new TestIsIpCase("1.256.1.1", false)
45+
, new TestIsIpCase("1.1.256.1", false)
46+
, new TestIsIpCase("1.1.1.256", false)
47+
48+
, new TestIsIpCase("399.1.1.1", false)
49+
, new TestIsIpCase("1.399.1.1", false)
50+
, new TestIsIpCase("1.1.399.1", false)
51+
, new TestIsIpCase("1.1.1.399", false)
52+
53+
// Zero-prefix
54+
, new TestIsIpCase("010.1.1.1", false)
55+
, new TestIsIpCase("1.010.1.1", false)
56+
, new TestIsIpCase("1.1.010.1", false)
57+
, new TestIsIpCase("1.1.1.010", false)
58+
59+
, new TestIsIpCase("01.1.1.1", false)
60+
, new TestIsIpCase("1.01.1.1", false)
61+
, new TestIsIpCase("1.1.01.1", false)
62+
, new TestIsIpCase("1.1.1.01", false)
63+
64+
// Not numeric segment
65+
, new TestIsIpCase("foo.1.1.1", false)
66+
, new TestIsIpCase("1.foo.1.1", false)
67+
, new TestIsIpCase("1.1.foo.1", false)
68+
, new TestIsIpCase("1.1.1.foo", false)
69+
, new TestIsIpCase("10x.1.1.1", false)
70+
, new TestIsIpCase("1.10x.1.1", false)
71+
, new TestIsIpCase("1.1.10x.1", false)
72+
, new TestIsIpCase("1.1.1.10x", false)
73+
74+
// More than 4 segments
75+
, new TestIsIpCase("127.0.0.1.1", false)
76+
, new TestIsIpCase("127.0.0.1.1mm", false)
77+
, new TestIsIpCase("1mm.127.0.0.1", false)
78+
, new TestIsIpCase("127.1mm.0.0.1", false)
79+
, new TestIsIpCase("127.0.1mm.0.1", false)
80+
, new TestIsIpCase("127.0.0.1mm.1", false)
81+
82+
// name
83+
, new TestIsIpCase("amazon.com", false)
84+
, new TestIsIpCase("localhost", false)
85+
);
86+
}
87+
88+
public static Collection<TestNormalizePathCase> testNormalizePathCases() {
89+
return Arrays.asList(
90+
new TestNormalizePathCase(null, "/")
91+
, new TestNormalizePathCase("", "/")
92+
, new TestNormalizePathCase("/", "/")
93+
, new TestNormalizePathCase("foo", "/foo/")
94+
, new TestNormalizePathCase("/foo", "/foo/")
95+
, new TestNormalizePathCase("foo/", "/foo/")
96+
, new TestNormalizePathCase("/foo/", "/foo/")
97+
);
98+
}
99+
100+
@ParameterizedTest
101+
@MethodSource("testIsIpCases")
102+
void testIsIp(TestIsIpCase testCase) {
103+
assertEquals(testCase.isIp, RuleUrl.isIpAddr(testCase.host));
104+
}
105+
106+
@ParameterizedTest
107+
@MethodSource("testNormalizePathCases")
108+
void testNormalizePath(TestNormalizePathCase testCase) {
109+
assertEquals(testCase.normalized, RuleUrl.normalizePath(testCase.path));
110+
}
111+
112+
113+
static class TestIsIpCase {
114+
final String host;
115+
final boolean isIp;
116+
117+
TestIsIpCase(String host, boolean isIp) {
118+
this.host = host;
119+
this.isIp = isIp;
120+
}
121+
122+
@Override
123+
public String toString() {
124+
return this.host + " -> " + this.isIp;
125+
}
126+
}
127+
128+
static class TestNormalizePathCase {
129+
final String path;
130+
final String normalized;
131+
132+
TestNormalizePathCase(String path, String normalized) {
133+
this.path = path;
134+
this.normalized = normalized;
135+
}
136+
137+
@Override
138+
public String toString() {
139+
return this.path + " -> " + this.normalized;
140+
}
141+
}
142+
}

0 commit comments

Comments
 (0)