From a21992ade9dc9bc95f620576d4ff38ca1d168dda Mon Sep 17 00:00:00 2001 From: Jonathan Leitschuh Date: Mon, 7 Mar 2022 18:40:53 -0500 Subject: [PATCH] Minor refactoring to improve tests and documentation --- .../code/java/environment/SystemProperty.qll | 2 +- .../semmle/code/java/frameworks/apache/Lang.qll | 4 ++-- java/ql/lib/semmle/code/java/os/OSCheck.qll | 11 ++++++----- java/ql/test/library-tests/os/Test.java | 4 ++++ .../os/specific-unix-variant-test.expected | 16 ++++++++-------- .../os/specific-windows-variant-test.expected | 2 +- java/ql/test/library-tests/os/unix-test.expected | 14 +++++++------- .../test/library-tests/os/windows-test.expected | 15 ++++++++------- 8 files changed, 37 insertions(+), 31 deletions(-) diff --git a/java/ql/lib/semmle/code/java/environment/SystemProperty.qll b/java/ql/lib/semmle/code/java/environment/SystemProperty.qll index 847de2a75b7..7f4cb6d35d8 100644 --- 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 index d7e2eca8396..84db672e935 100644 --- 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 index 2393597db62..ea82fab4f5b 100644 --- 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 index 7b9a4cb8fd9..bae22e423e0 100644 --- a/java/ql/test/library-tests/os/Test.java +++ b/java/ql/test/library-tests/os/Test.java @@ -25,6 +25,10 @@ public class Test { 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 index 56044f3bb74..a81e7d8e5e3 100644 --- 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 index ff43cd30463..89630563d13 100644 --- 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 index 003488d83a5..826f7980e66 100644 --- 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 index 60a88e3b9dc..c06de71c594 100644 --- 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(...) |