Repository: smithy-lang/smithy
Pull request #2615 (2025-05-06 20:06:53)

Make IAM resource name validation case-insensitive
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/smithy-aws-iam-traits/src/main/java/software/amazon/smithy/aws/iam/traits/IamResourceTraitConflictingNameValidator.java b/smithy-aws-iam-traits/src/main/java/software/amazon/smithy/aws/iam/traits/IamResourceTraitConflictingNameValidator.java
index c5fcd1918..ae9becf7c 100644
--- a/smithy-aws-iam-traits/src/main/java/software/amazon/smithy/aws/iam/traits/IamResourceTraitConflictingNameValidator.java
+++ b/smithy-aws-iam-traits/src/main/java/software/amazon/smithy/aws/iam/traits/IamResourceTraitConflictingNameValidator.java
@@ -9,6 +9,7 @@ import static software.amazon.smithy.model.validation.ValidationUtils.tickedList
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.stream.Collectors;
 import software.amazon.smithy.model.Model;
@@ -38,7 +39,7 @@ public class IamResourceTraitConflictingNameValidator extends AbstractValidator
         List<ValidationEvent> events = new ArrayList<>();
         Map<String, List<ShapeId>> resourceMap = new HashMap<>();
         for (ResourceShape resource : topDownIndex.getContainedResources(service)) {
-            String resourceName = IamResourceTrait.resolveResourceName(resource);
+            String resourceName = IamResourceTrait.resolveResourceName(resource).toLowerCase(Locale.US);
             resourceMap.computeIfAbsent(resourceName, k -> new ArrayList<>()).add(resource.getId());
         }
         resourceMap.entrySet()
@@ -46,8 +47,9 @@ public class IamResourceTraitConflictingNameValidator extends AbstractValidator
                 .filter(entry -> entry.getValue().size() > 1)
                 .map(entry -> error(service,
                         String.format(
-                                "Multiple IAM resources defined with the same IAM resource name is not allowed in a service "
-                                        + "closure, but found multiple resources named `%s` in the service `%s`: [%s]",
+                                "Multiple IAM resources defined with the same IAM resource name is not allowed in a "
+                                        + "service closure, but found multiple resources case-insensitively named `%s` "
+                                        + "in the service `%s`: [%s]",
                                 entry.getKey(),
                                 service.getId(),
                                 tickedList(entry.getValue()))))
diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/EndpointValue.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/EndpointValue.java
index 701982876..324c71ab9 100644
--- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/EndpointValue.java
+++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/evaluation/value/EndpointValue.java
@@ -25,7 +25,7 @@ import software.amazon.smithy.utils.StringUtils;
  */
 public final class EndpointValue extends Value {
     private static final String PROPERTIES = "properties";
-    private static final String URL = "URL";
+    private static final String URL = "url";
     private static final String HEADERS = "headers";
     private static final List<String> NODE_PROPERTIES = ListUtils.of(PROPERTIES, URL, HEADERS);
 
@@ -60,14 +60,14 @@ public final class EndpointValue extends Value {
         ObjectNode objectNode = node.expectObjectNode("endpoints are object nodes");
         objectNode.expectNoAdditionalProperties(NODE_PROPERTIES);
 
-        objectNode.expectStringMember("url", builder::url);
+        objectNode.expectStringMember(URL, builder::url);
         objectNode.getObjectMember(PROPERTIES, props -> {
             for (Map.Entry<String, Node> entry : props.getStringMap().entrySet()) {
                 builder.putProperty(entry.getKey(), Value.fromNode(entry.getValue()));
             }
         });
 
-        objectNode.getObjectMember("headers", headers -> {
+        objectNode.getObjectMember(HEADERS, headers -> {
             for (Map.Entry<String, Node> entry : headers.getStringMap().entrySet()) {
                 builder.putHeader(entry.getKey(),
                         entry.getValue()
diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/IsValidHostLabel.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/IsValidHostLabel.java
index 2c5ee2635..2abbc12c1 100644
--- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/IsValidHostLabel.java
+++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/IsValidHostLabel.java
@@ -6,6 +6,7 @@ package software.amazon.smithy.rulesengine.language.syntax.expressions.functions
 
 import java.util.Arrays;
 import java.util.List;
+import java.util.regex.Pattern;
 import software.amazon.smithy.rulesengine.language.evaluation.type.Type;
 import software.amazon.smithy.rulesengine.language.evaluation.value.Value;
 import software.amazon.smithy.rulesengine.language.syntax.ToExpression;
@@ -19,6 +20,7 @@ import software.amazon.smithy.utils.SmithyUnstableApi;
 @SmithyUnstableApi
 public final class IsValidHostLabel extends LibraryFunction {
     public static final String ID = "isValidHostLabel";
+    private static final Pattern HOST_LABEL_PATTERN = Pattern.compile("^[a-zA-Z\\d][a-zA-Z\\d\\-]{0,62}$");
     private static final Definition DEFINITION = new Definition();
 
     IsValidHostLabel(FunctionNode functionNode) {
@@ -89,23 +91,30 @@ public final class IsValidHostLabel extends LibraryFunction {
             return Value.booleanValue(isValidHostLabel(hostLabel, allowDots));
         }
 
-        private boolean isValidHostLabel(String hostLabel, boolean allowDots) {
-            if (allowDots) {
-                // ensure that empty matches at the end are included
-                for (String subLabel : hostLabel.split("[.]", -1)) {
-                    if (!isValidHostLabel(subLabel, false)) {
-                        return false;
-                    }
-                }
-                return true;
-            } else {
-                return hostLabel.matches("[a-zA-Z\\d][a-zA-Z\\d\\-]{1,62}");
-            }
-        }
-
         @Override
         public IsValidHostLabel createFunction(FunctionNode functionNode) {
             return new IsValidHostLabel(functionNode);
         }
     }
+
+    /**
+     * Check if a hostLabel is valid host.
+     *
+     * @param hostLabel Host label to check.
+     * @param allowDots Set to true to allow dots.
+     * @return true if the label is valid.
+     */
+    public static boolean isValidHostLabel(String hostLabel, boolean allowDots) {
+        if (!allowDots) {
+            return HOST_LABEL_PATTERN.matcher(hostLabel).matches();
+        } else {
+            // ensure that empty matches at the end are included
+            for (String subLabel : hostLabel.split("[.]", -1)) {
+                if (!HOST_LABEL_PATTERN.matcher(subLabel).matches()) {
+                    return false;
+                }
+            }
+            return true;
+        }
+    }
 }
diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/ParseUrl.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/ParseUrl.java
index 12adedf22..a8a749f00 100644
--- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/ParseUrl.java
+++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/ParseUrl.java
@@ -99,41 +99,9 @@ public final class ParseUrl extends LibraryFunction {
                     return Value.emptyValue();
                 }
 
-                boolean isIpAddr = false;
-                String host = parsed.getHost();
-                if (host.startsWith("[") && host.endsWith("]")) {
-                    isIpAddr = true;
-                }
-                String[] dottedParts = host.split("\\.");
-                if (dottedParts.length == 4) {
-                    isIpAddr = true;
-                    for (String dottedPart : dottedParts) {
-                        try {
-                            int value = Integer.parseInt(dottedPart);
-                            if (value < 0 || value > 255) {
-                                isIpAddr = false;
-                            }
-                        } catch (NumberFormatException ex) {
-                            isIpAddr = false;
-                        }
-                    }
-                }
-
+                boolean isIpAddr = isIpAddr(parsed.getHost());
                 String path = parsed.getPath();
-                String normalizedPath;
-                if (StringUtils.isBlank(path)) {
-                    normalizedPath = "/";
-                } else {
-                    StringBuilder builder = new StringBuilder();
-                    if (!path.startsWith("/")) {
-                        builder.append("/");
-                    }
-                    builder.append(path);
-                    if (!path.endsWith("/")) {
-                        builder.append("/");
-                    }
-                    normalizedPath = builder.toString();
-                }
+                String normalizedPath = normalizePath(path);
 
                 Map<Identifier, Value> values = new LinkedHashMap<>();
                 values.put(SCHEME, Value.stringValue(parsed.getProtocol()));
@@ -152,4 +120,92 @@ public final class ParseUrl extends LibraryFunction {
             return new ParseUrl(functionNode);
         }
     }
+
+    /**
+     * Checks if a host is an IP address for use with endpoint rules.
+     *
+     * @param host Host to check.
+     * @return true if it is an IP address.
+     */
+    public static boolean isIpAddr(String host) {
+        if (host == null || host.length() < 2) {
+            return false;
+        }
+
+        // Simple check for IPv6 (enclosed in square brackets)
+        if (host.charAt(0) == '[' && host.charAt(host.length() - 1) == ']') {
+            return true;
+        }
+
+        int from = 0;
+        int segments = 0;
+        boolean done = false;
+        while (!done) {
+            int index = host.indexOf('.', from);
+            if (index == -1) {
+                if (segments != 3) {
+                    // E.g., 123.com
+                    return false;
+                }
+                index = host.length();
+                done = true;
+            } else if (segments == 3) {
+                // E.g., 1.2.3.4.5
+                return false;
+            }
+            int length = index - from;
+            if (length == 1) {
+                char ch0 = host.charAt(from);
+                if (ch0 < '0' || ch0 > '9') {
+                    return false;
+                }
+            } else if (length == 2) {
+                char ch0 = host.charAt(from);
+                char ch1 = host.charAt(from + 1);
+                if ((ch0 <= '0' || ch0 > '9') || (ch1 < '0' || ch1 > '9')) {
+                    return false;
+                }
+            } else if (length == 3) {
+                char ch0 = host.charAt(from);
+                char ch1 = host.charAt(from + 1);
+                char ch2 = host.charAt(from + 2);
+                if ((ch0 <= '0' || ch0 > '9')
+                        || (ch1 < '0' || ch1 > '9')
+                        || (ch2 < '0' || ch2 > '9')) {
+                    return false;
+                }
+                int value = ((ch0 - '0') * 100) + ((ch1 - '0') * 10) + (ch2 - '0');
+                if (value > 255) {
+                    return false;
+                }
+            } else {
+                return false;
+            }
+            from = index + 1;
+            segments += 1;
+        }
+        return true;
+    }
+
+    /**
+     * Normalizes a path string according to the ParseURL function.
+     *
+     * @param path Path string to normalize.
+     * @return the normalized path.
+     */
+    public static String normalizePath(String path) {
+        if (StringUtils.isBlank(path)) {
+            return "/";
+        } else {
+            StringBuilder builder = new StringBuilder();
+            if (!path.startsWith("/")) {
+                builder.append("/");
+            }
+            builder.append(path);
+            if (!path.endsWith("/")) {
+                builder.append("/");
+            }
+            return builder.toString();
+        }
+    }
 }
diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/Substring.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/Substring.java
index f0c0e2378..846eba9a0 100644
--- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/Substring.java
+++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/Substring.java
@@ -97,30 +97,42 @@ public final class Substring extends LibraryFunction {
             int startIndex = arguments.get(1).expectIntegerValue().getValue();
             int stopIndex = arguments.get(2).expectIntegerValue().getValue();
             boolean reverse = arguments.get(3).expectBooleanValue().getValue();
+            String result = getSubstring(str, startIndex, stopIndex, reverse);
+            return result == null ? Value.emptyValue() : Value.stringValue(result);
+        }
 
-            for (int i = 0; i < str.length(); i++) {
-                char ch = str.charAt(i);
-                if (!(ch <= 127)) {
-                    return Value.emptyValue();
-                }
-            }
+        @Override
+        public Substring createFunction(FunctionNode functionNode) {
+            return new Substring(functionNode);
+        }
+    }
 
-            if (startIndex >= stopIndex || str.length() < stopIndex) {
-                return Value.emptyValue();
+    /**
+     * Get the substring of {@code value} using the same logic as the substring function.
+     *
+     * @param value Value to get the substring of.
+     * @param startIndex Start index.
+     * @param stopIndex Stop index.
+     * @param reverse True if the slice is from the end.
+     * @return the substring value or null.
+     */
+    public static String getSubstring(String value, int startIndex, int stopIndex, boolean reverse) {
+        for (int i = 0; i < value.length(); i++) {
+            if (!(value.charAt(i) <= 127)) {
+                return null;
             }
+        }
 
-            if (!reverse) {
-                return Value.stringValue(str.substring(startIndex, stopIndex));
-            } else {
-                int revStart = str.length() - stopIndex;
-                int revStop = str.length() - startIndex;
-                return Value.stringValue(str.substring(revStart, revStop));
-            }
+        if (startIndex >= stopIndex || value.length() < stopIndex) {
+            return null;
         }
 
-        @Override
-        public Substring createFunction(FunctionNode functionNode) {
-            return new Substring(functionNode);
+        if (!reverse) {
+            return value.substring(startIndex, stopIndex);
+        } else {
+            int revStart = value.length() - stopIndex;
+            int revStop = value.length() - startIndex;
+            return value.substring(revStart, revStop);
         }
     }
 }


Test patch:
diff --git a/smithy-aws-iam-traits/src/test/resources/software/amazon/smithy/aws/iam/traits/errorfiles/iam-resources/duplicate-iam-resources-name.errors b/smithy-aws-iam-traits/src/test/resources/software/amazon/smithy/aws/iam/traits/errorfiles/iam-resources/duplicate-iam-resources-name.errors
index 976e0c872..9b71b6d93 100644
--- a/smithy-aws-iam-traits/src/test/resources/software/amazon/smithy/aws/iam/traits/errorfiles/iam-resources/duplicate-iam-resources-name.errors
+++ b/smithy-aws-iam-traits/src/test/resources/software/amazon/smithy/aws/iam/traits/errorfiles/iam-resources/duplicate-iam-resources-name.errors
@@ -1,1 +1,1 @@
-[ERROR] smithy.example#MyService: Multiple IAM resources defined with the same IAM resource name is not allowed in a service closure, but found multiple resources named `Beer` in the service `smithy.example#MyService`: [`smithy.example#BadIamResourceName1`, `smithy.example#BadIamResourceName2`, `smithy.example#Beer`] | IamResourceTraitConflictingName
+[ERROR] smithy.example#MyService: Multiple IAM resources defined with the same IAM resource name is not allowed in a service closure, but found multiple resources case-insensitively named `beer` in the service `smithy.example#MyService`: [`smithy.example#BadIamResourceName1`, `smithy.example#BadIamResourceName2`, `smithy.example#Beer`] | IamResourceTraitConflictingName
diff --git a/smithy-aws-iam-traits/src/test/resources/software/amazon/smithy/aws/iam/traits/errorfiles/iam-resources/duplicate-iam-resources-name.smithy b/smithy-aws-iam-traits/src/test/resources/software/amazon/smithy/aws/iam/traits/errorfiles/iam-resources/duplicate-iam-resources-name.smithy
index 9833e42db..2d97355ef 100644
--- a/smithy-aws-iam-traits/src/test/resources/software/amazon/smithy/aws/iam/traits/errorfiles/iam-resources/duplicate-iam-resources-name.smithy
+++ b/smithy-aws-iam-traits/src/test/resources/software/amazon/smithy/aws/iam/traits/errorfiles/iam-resources/duplicate-iam-resources-name.smithy
@@ -21,7 +21,7 @@ resource Beer {}
 @aws.iam#iamResource(name: "Beer")
 resource BadIamResourceName1 {}
 
-@aws.iam#iamResource(name: "Beer")
+@aws.iam#iamResource(name: "beer")
 resource BadIamResourceName2 {}
 
 @aws.iam#iamResource(name: "shouldNotThrowErrorSecond")
diff --git a/smithy-rules-engine/src/test/java/software/amazon/smithy/rulesengine/language/value/EndpointValueTest.java b/smithy-rules-engine/src/test/java/software/amazon/smithy/rulesengine/language/value/EndpointValueTest.java
new file mode 100644
index 000000000..11734bc96
--- /dev/null
+++ b/smithy-rules-engine/src/test/java/software/amazon/smithy/rulesengine/language/value/EndpointValueTest.java
@@ -0,0 +1,20 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package software.amazon.smithy.rulesengine.language.value;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.junit.jupiter.api.Test;
+import software.amazon.smithy.model.node.Node;
+import software.amazon.smithy.rulesengine.language.evaluation.value.Value;
+
+public class EndpointValueTest {
+    @Test
+    public void loadsFromNode() {
+        Value value = Value.endpointValue(Node.objectNode().withMember("url", "https://foo.com"));
+
+        assertEquals("https://foo.com", value.toNode().expectObjectNode().expectStringMember("url").getValue());
+    }
+}