Merge pull request #2049 from pjanouse/JENKINS-33037

[FIXED JENKINS-33037] Fixed malformed ranges in RangeSet.fromString()
jenkinsci · Mar 16, 2016 · 8ba5919 · 8ba5919
2 parents eeea5ee + c1621a2
commit 8ba5919
Show file tree

Hide file tree

Showing 2 changed files with 334 additions and 4 deletions.
diff --git a/core/src/main/java/hudson/model/Fingerprint.java b/core/src/main/java/hudson/model/Fingerprint.java
@@ -50,6 +50,7 @@
 import jenkins.model.FingerprintFacet;
 import jenkins.model.Jenkins;
 import jenkins.model.TransientFingerprintFacetFactory;
+import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.export.ExportedBean;
 
@@ -687,21 +688,77 @@ public synchronized boolean isSmallerThan(int n) {
          */
         public static RangeSet fromString(String list, boolean skipError) {
             RangeSet rs = new RangeSet();
-            for (String s : Util.tokenize(list,",")) {
+
+            // Reject malformed ranges like "1---10", "1,,,,3" etc.
+            if (list.contains("--") || list.contains(",,")) {
+                if (!skipError) {
+                    throw new IllegalArgumentException(
+                            String.format("Unable to parse '%s', expected correct notation M,N or M-N", list));
+                }
+                // ignore malformed notation
+                return rs;
+            }
+
+            String[] items = Util.tokenize(list,",");
+            if(items.length > 1 && items.length <= StringUtils.countMatches(list, ",")) {
+                if (!skipError) {
+                    throw new IllegalArgumentException(
+                            String.format("Unable to parse '%s', expected correct notation M,N or M-N", list));
+                }
+                // ignore malformed notation like ",1,2" or "1,2,"
+                return rs;
+            }
+
+            for (String s : items) {
                 s = s.trim();
                 // s is either single number or range "x-y".
                 // note that the end range is inclusive in this notation, but not in the Range class
                 try {
+                    if (s.isEmpty()) {
+                        if (!skipError) {
+                            throw new IllegalArgumentException(
+                                    String.format("Unable to parse '%s', expected number", list));                        }
+                        // ignore "" element
+                        continue;
+                    }
+
                     if(s.contains("-")) {
+                        if(StringUtils.countMatches(s, "-") > 1) {
+                            if (!skipError) {
+                                throw new IllegalArgumentException(String.format(
+                                        "Unable to parse '%s', expected correct notation M,N or M-N", list));
+                            }
+                            // ignore malformed ranges like "-5-2" or "2-5-"
+                            continue;
+                        }
                         String[] tokens = Util.tokenize(s,"-");
                         if (tokens.length == 2) {
-                            rs.ranges.add(new Range(Integer.parseInt(tokens[0]), Integer.parseInt(tokens[1]) + 1));
+                            int left = Integer.parseInt(tokens[0]);
+                            int right = Integer.parseInt(tokens[1]);
+                            if(left < 0 || right < 0) {
+                                if (!skipError) {
+                                    throw new IllegalArgumentException(
+                                            String.format("Unable to parse '%s', expected number above zero", list));
+                                }
+                                // ignore a range which starts or ends under zero like "-5-3"
+                                continue;
+                            }
+                            if(left > right) {
+                                if (!skipError) {
+                                    throw new IllegalArgumentException(String.format(
+                                            "Unable to parse '%s', expected string with a range M-N where M<N", list));
+                                }
+                                // ignore inverse range like "10-5"
+                                continue;
+                            }
+                            rs.ranges.add(new Range(left, right+1));
                         } else {
                             if (!skipError) {
                                 throw new IllegalArgumentException(
-                                        String.format("Unable to parse %s, expected string with a range M-N", list));
+                                        String.format("Unable to parse '%s', expected string with a range M-N", list));
                             }
                             // ignore malformed text like "1-10-50"
+                            continue;
                         }
                     } else {
                         int n = Integer.parseInt(s);
@@ -710,7 +767,7 @@ public static RangeSet fromString(String list, boolean skipError) {
                 } catch (NumberFormatException e) {
                     if (!skipError)
                         throw new IllegalArgumentException(
-                                String.format("Unable to parse %s, expected number", list));
+                                String.format("Unable to parse '%s', expected number", list));
                     // ignore malformed text
                 }
             }