Minor refactoring to improve tests and documentation

JLLeitschuh · JLLeitschuh · commit a21992ade9dc · 2022-03-07T18:40:53.000-05:00
diff --git a/java/ql/lib/semmle/code/java/environment/SystemProperty.qll b/java/ql/lib/semmle/code/java/environment/SystemProperty.qll
@@ -70,7 +70,7 @@ private class FieldFilePathSeparator extends Field {
  * See: https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/SystemUtils.html
  */
 private FieldAccess getSystemPropertyFromApacheSystemUtils(string propertyName) {
-  exists(Field f | f = result.getField() and f.getDeclaringType() instanceof ApacheSystemUtils |
+  exists(Field f | f = result.getField() and f.getDeclaringType() instanceof TypeApacheSystemUtils |
     f.hasName("AWT_TOOLKIT") and propertyName = "awt.toolkit"
     or
     f.hasName("FILE_ENCODING") and propertyName = "file.encoding"
diff --git a/java/ql/lib/semmle/code/java/frameworks/apache/Lang.qll b/java/ql/lib/semmle/code/java/frameworks/apache/Lang.qll
@@ -41,8 +41,8 @@ private class ApacheStrBuilderFluentMethod extends FluentMethod {
 /**
  * The class `org.apache.commons.lang.SystemUtils` or `org.apache.commons.lang3.SystemUtils`.
  */
-class ApacheSystemUtils extends Class {
-  ApacheSystemUtils() {
+class TypeApacheSystemUtils extends Class {
+  TypeApacheSystemUtils() {
     this.hasQualifiedName(["org.apache.commons.lang", "org.apache.commons.lang3"], "SystemUtils")
   }
 }
diff --git a/java/ql/lib/semmle/code/java/os/OSCheck.qll b/java/ql/lib/semmle/code/java/os/OSCheck.qll
@@ -40,19 +40,18 @@ abstract class IsSpecificUnixVariant extends Guard { }
 /**
  * Holds when `ma` compares the current OS against the string constant `osString`.
  */
-bindingset[osString]
 private predicate isOsFromSystemProp(MethodAccess ma, string osString) {
   TaintTracking::localExprTaint(getSystemProperty("os.name"), ma.getQualifier()) and // Call from System.getProperty (or equivalent) to some partial match method
   exists(StringPartialMatchMethod m, CompileTimeConstantExpr matchedStringConstant |
     m = ma.getMethod() and
-    matchedStringConstant.getStringValue().toLowerCase().matches(osString)
+    matchedStringConstant.getStringValue().toLowerCase() = osString
   |
     DataFlow::localExprFlow(matchedStringConstant, ma.getArgument(m.getMatchParameterIndex()))
   )
 }
 
 private class IsWindowsFromSystemProp extends IsWindowsGuard instanceof MethodAccess {
-  IsWindowsFromSystemProp() { isOsFromSystemProp(this, "window%") }
+  IsWindowsFromSystemProp() { isOsFromSystemProp(this, any(string s | s.regexpMatch("windows?"))) }
 }
 
 /**
@@ -99,13 +98,15 @@ private class IsUnixFromCharSeparator extends IsUnixGuard {
 }
 
 private class IsUnixFromSystemProp extends IsSpecificUnixVariant instanceof MethodAccess {
-  IsUnixFromSystemProp() { isOsFromSystemProp(this, ["mac%", "linux%"]) }
+  IsUnixFromSystemProp() {
+    isOsFromSystemProp(this, any(string s | s.regexpMatch(["mac.*", "linux.*"])))
+  }
 }
 
 bindingset[fieldNamePattern]
 private predicate isOsFromApacheCommons(FieldAccess fa, string fieldNamePattern) {
   exists(Field f | f = fa.getField() |
-    f.getDeclaringType() instanceof ApacheSystemUtils and
+    f.getDeclaringType() instanceof TypeApacheSystemUtils and
     f.getName().matches(fieldNamePattern)
   )
 }
diff --git a/java/ql/test/library-tests/os/Test.java b/java/ql/test/library-tests/os/Test.java
@@ -25,6 +25,10 @@ void testWindows() {
             onlyOnWindows();
         }
 
+        if (System.getProperty("os.name").toLowerCase().contains("window")) {
+            onlyOnWindows();
+        }
+
         if (System.getProperty("os.name").toUpperCase().contains("WINDOWS")) {
             onlyOnWindows();
         }
diff --git a/java/ql/test/library-tests/os/specific-unix-variant-test.expected b/java/ql/test/library-tests/os/specific-unix-variant-test.expected
@@ -27,11 +27,11 @@
 | ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1415:5:1415:84 | IS_OS_SOLARIS |
 | ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1427:5:1427:83 | IS_OS_SUN_OS |
 | ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1625:5:1625:80 | IS_OS_ZOS |
-| Test.java:103:13:103:73 | contains(...) |
-| Test.java:107:13:107:59 | contains(...) |
-| Test.java:111:13:111:35 | SystemUtils.IS_OS_LINUX |
-| Test.java:117:14:117:36 | SystemUtils.IS_OS_LINUX |
-| Test.java:125:13:125:62 | contains(...) |
-| Test.java:129:14:129:72 | contains(...) |
-| Test.java:133:14:133:34 | SystemUtils.IS_OS_MAC |
-| Test.java:139:14:139:45 | SystemUtils.IS_OS_MAC_OSX_MOJAVE |
+| Test.java:107:13:107:73 | contains(...) |
+| Test.java:111:13:111:59 | contains(...) |
+| Test.java:115:13:115:35 | SystemUtils.IS_OS_LINUX |
+| Test.java:121:14:121:36 | SystemUtils.IS_OS_LINUX |
+| Test.java:129:13:129:62 | contains(...) |
+| Test.java:133:14:133:72 | contains(...) |
+| Test.java:137:14:137:34 | SystemUtils.IS_OS_MAC |
+| Test.java:143:14:143:45 | SystemUtils.IS_OS_MAC_OSX_MOJAVE |
diff --git a/java/ql/test/library-tests/os/specific-windows-variant-test.expected b/java/ql/test/library-tests/os/specific-windows-variant-test.expected
@@ -11,4 +11,4 @@
 | ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1584:5:1584:86 | IS_OS_WINDOWS_7 |
 | ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1596:5:1596:86 | IS_OS_WINDOWS_8 |
 | ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1608:5:1608:87 | IS_OS_WINDOWS_10 |
-| Test.java:38:13:38:40 | SystemUtils.IS_OS_WINDOWS_XP |
+| Test.java:42:13:42:40 | SystemUtils.IS_OS_WINDOWS_XP |
diff --git a/java/ql/test/library-tests/os/unix-test.expected b/java/ql/test/library-tests/os/unix-test.expected
@@ -1,9 +1,9 @@
 | ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1439:5:1439:81 | IS_OS_UNIX |
-| Test.java:66:13:66:95 | contains(...) |
-| Test.java:70:13:70:84 | contains(...) |
-| Test.java:74:13:74:34 | SystemUtils.IS_OS_UNIX |
-| Test.java:81:13:81:41 | ... == ... |
-| Test.java:85:13:85:37 | ... == ... |
+| Test.java:70:13:70:95 | contains(...) |
+| Test.java:74:13:74:84 | contains(...) |
+| Test.java:78:13:78:34 | SystemUtils.IS_OS_UNIX |
+| Test.java:85:13:85:41 | ... == ... |
 | Test.java:89:13:89:37 | ... == ... |
-| Test.java:93:13:93:33 | ... == ... |
-| Test.java:97:13:97:60 | equals(...) |
+| Test.java:93:13:93:37 | ... == ... |
+| Test.java:97:13:97:33 | ... == ... |
+| Test.java:101:13:101:60 | equals(...) |
diff --git a/java/ql/test/library-tests/os/windows-test.expected b/java/ql/test/library-tests/os/windows-test.expected
@@ -1,10 +1,11 @@
 | ../../stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/SystemUtils.java:1451:5:1451:84 | IS_OS_WINDOWS |
 | Test.java:20:13:20:61 | contains(...) |
 | Test.java:24:13:24:75 | contains(...) |
-| Test.java:28:13:28:75 | contains(...) |
-| Test.java:32:13:32:37 | SystemUtils.IS_OS_WINDOWS |
-| Test.java:44:13:44:41 | ... == ... |
-| Test.java:48:13:48:37 | ... == ... |
-| Test.java:52:13:52:38 | ... == ... |
-| Test.java:56:13:56:34 | ... == ... |
-| Test.java:60:13:60:60 | equals(...) |
+| Test.java:28:13:28:74 | contains(...) |
+| Test.java:32:13:32:75 | contains(...) |
+| Test.java:36:13:36:37 | SystemUtils.IS_OS_WINDOWS |
+| Test.java:48:13:48:41 | ... == ... |
+| Test.java:52:13:52:37 | ... == ... |
+| Test.java:56:13:56:38 | ... == ... |
+| Test.java:60:13:60:34 | ... == ... |
+| Test.java:64:13:64:60 | equals(...) |

Original file line number	Diff line number	Diff line change
`@@ -41,8 +41,8 @@ private class ApacheStrBuilderFluentMethod extends FluentMethod {`
`41`	`41`	`/**`
`42`	`42`	* The class `org.apache.commons.lang.SystemUtils` or `org.apache.commons.lang3.SystemUtils`.
`43`	`43`	`*/`
`44`		`-class ApacheSystemUtils extends Class {`
`45`		`- ApacheSystemUtils() {`
	`44`	`+class TypeApacheSystemUtils extends Class {`
	`45`	`+ TypeApacheSystemUtils() {`
`46`	`46`	`this.hasQualifiedName(["org.apache.commons.lang", "org.apache.commons.lang3"], "SystemUtils")`
`47`	`47`	`}`
`48`	`48`	`}`
Original file line number	Diff line number	Diff line change
`@@ -40,19 +40,18 @@ abstract class IsSpecificUnixVariant extends Guard { }`
`40`	`40`	`/**`
`41`	`41`	* Holds when `ma` compares the current OS against the string constant `osString`.
`42`	`42`	`*/`
`43`		`-bindingset[osString]`
`44`	`43`	`private predicate isOsFromSystemProp(MethodAccess ma, string osString) {`
`45`	`44`	`TaintTracking::localExprTaint(getSystemProperty("os.name"), ma.getQualifier()) and // Call from System.getProperty (or equivalent) to some partial match method`
`46`	`45`	`exists(StringPartialMatchMethod m, CompileTimeConstantExpr matchedStringConstant \|`
`47`	`46`	`m = ma.getMethod() and`
`48`		`- matchedStringConstant.getStringValue().toLowerCase().matches(osString)`
	`47`	`+ matchedStringConstant.getStringValue().toLowerCase() = osString`
`49`	`48`	`\|`
`50`	`49`	`DataFlow::localExprFlow(matchedStringConstant, ma.getArgument(m.getMatchParameterIndex()))`
`51`	`50`	`)`
`52`	`51`	`}`
`53`	`52`
`54`	`53`	`private class IsWindowsFromSystemProp extends IsWindowsGuard instanceof MethodAccess {`
`55`		`- IsWindowsFromSystemProp() { isOsFromSystemProp(this, "window%") }`
	`54`	`+ IsWindowsFromSystemProp() { isOsFromSystemProp(this, any(string s \| s.regexpMatch("windows?"))) }`
`56`	`55`	`}`
`57`	`56`
`58`	`57`	`/**`
`@@ -99,13 +98,15 @@ private class IsUnixFromCharSeparator extends IsUnixGuard {`
`99`	`98`	`}`
`100`	`99`
`101`	`100`	`private class IsUnixFromSystemProp extends IsSpecificUnixVariant instanceof MethodAccess {`
`102`		`- IsUnixFromSystemProp() { isOsFromSystemProp(this, ["mac%", "linux%"]) }`
	`101`	`+ IsUnixFromSystemProp() {`
	`102`	`+ isOsFromSystemProp(this, any(string s \| s.regexpMatch(["mac.", "linux."])))`
	`103`	`+ }`
`103`	`104`	`}`
`104`	`105`
`105`	`106`	`bindingset[fieldNamePattern]`
`106`	`107`	`private predicate isOsFromApacheCommons(FieldAccess fa, string fieldNamePattern) {`
`107`	`108`	`exists(Field f \| f = fa.getField() \|`
`108`		`- f.getDeclaringType() instanceof ApacheSystemUtils and`
	`109`	`+ f.getDeclaringType() instanceof TypeApacheSystemUtils and`
`109`	`110`	`f.getName().matches(fieldNamePattern)`
`110`	`111`	`)`
`111`	`112`	`}`
Original file line number	Diff line number	Diff line change
`@@ -25,6 +25,10 @@ void testWindows() {`
`25`	`25`	`onlyOnWindows();`
`26`	`26`	`}`
`27`	`27`
	`28`	`+ if (System.getProperty("os.name").toLowerCase().contains("window")) {`
	`29`	`+ onlyOnWindows();`
	`30`	`+ }`
	`31`	`+`
`28`	`32`	`if (System.getProperty("os.name").toUpperCase().contains("WINDOWS")) {`
`29`	`33`	`onlyOnWindows();`
`30`	`34`	`}`