commit 6748b39f87de76f0ad1a4a192f3ee3965a14bc71
author cparsons <cparsons@google.com> Mon Apr 17 21:16:13 2017 +0200
committer Klaus Aehlig <aehlig@google.com> Tue Apr 18 11:33:40 2017 +0200
tree e2139f249d6002db847ebc4a79b99eb35539e303
parent 391d5d9d06f18d49baab037ebc10e4ec5efdb233

Improve DottedVersion validation for minimum_os attribute, and fix error message.

RELNOTES: None.
PiperOrigin-RevId: 153372617

src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java

diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java b/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java
index 9bdb5a8..2a38bfe 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java
@@ -188,6 +188,27 @@
     return Joiner.on('.').join(stringComponents.build());
   }
 
+  /**
+   * Returns true if this version number has any alphabetic characters, such as 'alpha' in
+   * "7.3alpha.2".
+   */
+  public boolean hasAlphabeticCharacters() {
+    for (Component component : components) {
+      if (!Objects.equals(component.alphaSequence, NO_ALPHA_SEQUENCE)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  /**
+   * Returns the number of components in this version number. For example, "7.3.0" has three
+   * components.
+   */
+  public int numComponents() {
+    return components.size();
+  }
+
   @Override
   public String toString() {
     return stringRepresentation;

src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java b/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java
index de2c187..5cbb4d9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java
@@ -46,6 +46,11 @@
   static final String UNSUPPORTED_PLATFORM_TYPE_ERROR_FORMAT =
       "Unsupported platform type \"%s\"";
   
+  @VisibleForTesting
+  static final String INVALID_VERSION_STRING_ERROR_FORMAT =
+      "Invalid version string \"%s\". Version must be of the form 'x.y' without alphabetic "
+          + "characters, such as '4.3'.";
+  
   private static final ImmutableSet<PlatformType> SUPPORTED_PLATFORM_TYPES =
       ImmutableSet.of(
           PlatformType.IOS, PlatformType.WATCHOS, PlatformType.TVOS, PlatformType.MACOS);
@@ -79,12 +84,14 @@
     // TODO(b/37096178): This should be a mandatory attribute.
     if (!Strings.isNullOrEmpty(attributeValue)) {
       try {
-        // TODO(cparsons): Do more rigorous validation. For example, "8.2beta.3" is invalid, but
-        // will pass this validation.
-        DottedVersion.fromString(attributeValue);
+        DottedVersion minimumOsVersion = DottedVersion.fromString(attributeValue);
+        if (minimumOsVersion.hasAlphabeticCharacters() || minimumOsVersion.numComponents() > 2) {
+          throw ruleContext.throwWithAttributeError(MultiArchPlatformRule.MINIMUM_OS_VERSION,
+              String.format(INVALID_VERSION_STRING_ERROR_FORMAT, attributeValue));
+        }
       } catch (IllegalArgumentException exception) {
-        throw ruleContext.throwWithAttributeError(MultiArchPlatformRule.PLATFORM_TYPE_ATTR_NAME,
-            String.format(UNSUPPORTED_PLATFORM_TYPE_ERROR_FORMAT, attributeValue));
+        throw ruleContext.throwWithAttributeError(MultiArchPlatformRule.MINIMUM_OS_VERSION,
+            String.format(INVALID_VERSION_STRING_ERROR_FORMAT, attributeValue));
       }
     }
   }