fix: support both Gradle 2.x and configuration attributes in Gradle 3+ (

#80)

.travis.yml

@@ -12,6 +12,9 @@ node_js:
   - "8"
   - "6"
 env:
+  - GRADLE_VERSION=2.14 JAVA_VERSION=openjdk8
+  # java 9 and gradle 2 not supported
+
   - GRADLE_VERSION=3.5.1 JAVA_VERSION=openjdk8
   # java 9 and gradle 3 not supported
 

lib/index.ts

@@ -286,61 +286,66 @@ function leftPad(s: string, n: number) {
   return ' '.repeat(Math.max(n - s.length, 0)) + s;
 }
 
-async function getAllDeps(root, targetFile, options: SingleRootInspectOptions | MultiRootsInspectOptions):
-    Promise<JsonDepsScriptResult> {
-
-  let initGradlePath: string | null = null;
+async function getInjectedScriptPath(): Promise<{injectedScripPath: string, cleanupCallback?: () => void}> {
+  let initGradleAsset: string | null = null;
   if (/index.js$/.test(__filename)) {
     // running from ./dist
-    initGradlePath = path.join(__dirname, '../lib/init.gradle');
+    // path.join call has to be exactly in this format, needed by "pkg" to build a standalone Snyk CLI binary:
+    // https://www.npmjs.com/package/pkg#detecting-assets-in-source-code
+    initGradleAsset = path.join(__dirname, '../lib/init.gradle');
   } else if (/index.ts$/.test(__filename)) {
     // running from ./lib
-    initGradlePath = path.join(__dirname, 'init.gradle');
+    initGradleAsset = path.join(__dirname, 'init.gradle');
   } else {
     throw new Error('Cannot locate Snyk init.gradle script');
   }
 
-  const args = buildArgs(
-    root, targetFile, initGradlePath,
-    options);
-
   // We could be running from a bundled CLI generated by `pkg`.
   // The Node filesystem in that case is not real: https://github.com/zeit/pkg#snapshot-filesystem
   // Copying the injectable script into a temp file.
-  let tmpInitGradle: tmp.SynchrounousResult | null = null;
   try {
-    tmpInitGradle = tmp.fileSync({postfix: '-init.gradle'});
-    await fs.createReadStream(initGradlePath).pipe(fs.createWriteStream('', {fd: tmpInitGradle!.fd}));
-    initGradlePath = tmpInitGradle.name;
+    const tmpInitGradle = tmp.fileSync({postfix: '-init.gradle'});
+    fs.createReadStream(initGradleAsset).pipe(fs.createWriteStream('', {fd: tmpInitGradle!.fd}));
+    return { injectedScripPath: tmpInitGradle.name, cleanupCallback: tmpInitGradle.removeCallback };
   }  catch (error) {
     error.message = error.message + '\n\n' +
       'Failed to create a temporary file to host Snyk init script for Gradle build analysis.';
     throw error;
   }
+}
+
+async function getAllDeps(root, targetFile, options: SingleRootInspectOptions | MultiRootsInspectOptions):
+    Promise<JsonDepsScriptResult> {
 
   const command = getCommand(root, targetFile);
+
+  let gradleVersionOutput = '[COULD NOT RUN gradle -v] ';
+  try {
+    gradleVersionOutput = await subProcess.execute(command, ['-v'], {cwd: root});
+  } catch (_) {
+    // intentionally empty
+  }
+
+  if (gradleVersionOutput.match(/Gradle 1/)) {
+    throw new Error('Gradle 1.x is not supported');
+  }
+
+  const { injectedScripPath, cleanupCallback } = await getInjectedScriptPath();
+  const args = buildArgs(root, targetFile, injectedScripPath, options);
+
   const fullCommandText = 'gradle command: ' + command + ' ' + args.join(' ');
   debugLog('Executing ' + fullCommandText);
   try {
     const stdoutText = await subProcess.execute(command, args, {cwd: root}, printIfEcho);
-    if (tmpInitGradle !== null) {
-      tmpInitGradle.removeCallback();
+    if (cleanupCallback) {
+      cleanupCallback();
     }
     return extractJsonFromScriptOutput(stdoutText);
   } catch (error0) {
     const error: Error = error0;
     const gradleErrorMarkers = /^\s*>\s.*$/;
     const gradleErrorEssence = error.message.split('\n').filter((l) => gradleErrorMarkers.test(l)).join('\n');
 
-    // It'd be nice to set it in the inner catch{} block below.
-    // However, it's not safe: the inner catch{} will be executed even it inner try{}
-    // succeeds. Seems like an async/await implementation problem.
-    let gradleVersionOutput = '[COULD NOT RUN gradle -v] ';
-    try {
-      gradleVersionOutput = await subProcess.execute(command, ['-v'], {cwd: root});
-    } catch (_) {
-      // intentionally empty
-    }
     const orange = chalk.rgb(255, 128, 0);
     const blackOnYellow = chalk.bgYellowBright.black;
     gradleVersionOutput = orange(gradleVersionOutput);

lib/init.gradle

@@ -2,6 +2,9 @@ import groovy.json.JsonOutput
 import java.util.regex.Pattern
 import java.util.regex.Matcher
 
+// Snyk dependency resolution script for Gradle.
+// Tested on Gradle versions from 2.14 to 5.4.1
+
 // This script does the following: for all the projects in the build file,
 // generate a merged configuration of all the available configurations,
 // and then list the dependencies as a tree.
@@ -16,7 +19,9 @@ import java.util.regex.Matcher
 
 // (-q to have clean output, -P supplies args as per https://stackoverflow.com/a/48370451)
 
-// confAttr is a comma-separated list of key:value pairs. The "key" is a case-insensitive substring
+// confAttr parameter (supported only in Gradle 3+) is used to perform attribute-based dependency variant matching
+// (important for Android: https://developer.android.com/studio/build/dependencies#variant_aware)
+// Its value is a comma-separated list of key:value pairs. The "key" is a case-insensitive substring
 // of the class name of the attribute (e.g. "buildtype" would match com.android.build.api.attributes.BuildTypeAttr),
 // the value should be a case-insensitive stringified value of the attribute
 
@@ -72,23 +77,38 @@ allprojects { everyProj ->
         depsToDict = { Iterable deps, Set currentChain ->
             def res = [:]
             deps.each { d ->
-                def depName = "$d.moduleGroup:$d.moduleName"
+                def depName = d.moduleGroup + ':' + d.moduleName
                 if (!currentChain.contains(depName)) {
-                    def row = ['name': depName, 'version': d.moduleVersion]
+                    def row = ['name': depName, 'version': d.moduleVersion, 'conf': d.configuration]
                     currentChain.add(depName)
                     def subDeps = depsToDict(d.children, currentChain)
                     currentChain.remove(depName)
                     if (subDeps.size() > 0) {
                         row['dependencies'] = subDeps
                     }
-                    res[row['name']] = row
+                    // In Gradle 2, there can be several instances of the same dependency present at each level,
+                    // each for a different configuration. In this case, we need to merge the dependencies.
+                    if (res.containsKey(depName)) {
+                        if (subDeps.size() > 0) {
+                            if (!res[depName].containsKey('dependencies')) {
+                                res[depName]['dependencies'] = [:]
+                            }
+                            res[depName]['dependencies'].putAll(subDeps)
+                        }
+                    } else {
+                        res[depName] = row
+                    }
                 }
             }
             return res
         }
 
         def matchesAttributeFilter
         matchesAttributeFilter = { conf ->
+            if (!conf.hasProperty('attributes')) {
+                // Gradle before version 3 does not support attributes
+                return true
+            }
             def matches = true
             def attrs = conf.attributes
             attrs.keySet().each({ attr ->
@@ -136,6 +156,10 @@ allprojects { everyProj ->
                 def attributesAsStrings = [:] // Map<String, Set<string>>
                 rootProject.allprojects.findAll(shouldScanProject).each { proj ->
                     proj.configurations.findAll({ it.name != 'snykMergedDepsConf' && it.name =~ confNameFilter && matchesAttributeFilter(it) }).each { conf ->
+                        if (!conf.hasProperty('attributes')) {
+                           // Gradle before version 3 does not support attributes
+                            return
+                        }
                         def attrs = conf.attributes
                         attrs.keySet().each({ attr ->
                             def value = attrs.getAttribute(attr)
@@ -184,11 +208,14 @@ allprojects { everyProj ->
                         mergeableConfs.each { snykConf.extendsFrom(it) }
 
                         // Copy all the unambiguous build attributes into the merged configuration
-                        allConfigurationAttributes.each({ attr, valueSet ->
-                            if (valueSet.size() == 1) {
-                                snykConf.attributes.attribute(attr, valueSet.head())
-                            }
-                        })
+                        // Gradle before version 3 does not support attributes
+                        if (snykConf.hasProperty('attributes')) {
+                            allConfigurationAttributes.each({ attr, valueSet ->
+                                if (valueSet.size() == 1) {
+                                    snykConf.attributes.attribute(attr, valueSet.head())
+                                }
+                            })
+                        }
                     }
                     if (snykConf != null) {
                         println('SNYKECHO resolving configuration ' + snykConf.name)

test/fixtures/api-configuration/build.gradle

@@ -1,4 +1,6 @@
-apply plugin: 'java-library'
+configurations {
+    api
+}
 
 repositories {
   mavenCentral()

test/fixtures/multi-config-attributes/build.gradle

@@ -11,19 +11,37 @@ repositories {
 }
 
 // See https://docs.gradle.org/current/userguide/dependency_management_attribute_based_matching.html
-def usageAttr = Attribute.of("org.gradle.usage", Usage)
 
-configurations {
-    apiConf {
-        attributes {
-            attribute(usageAttr, project.objects.named(Usage, "java-api"))
-        }
-    }
-    runtimeConf {
-        attributes {
-            attribute(usageAttr, project.objects.named(Usage, "java-runtime"))
-        }
+if (project.hasProperty('objects')) {
+  def usageAttr = Attribute.of("org.gradle.usage", Usage)
+  // Gradle 4+
+  configurations {
+      apiConf {
+          attributes {
+              attribute(usageAttr, project.objects.named(Usage, "java-api"))
+          }
+      }
+      runtimeConf {
+          attributes {
+              attribute(usageAttr, project.objects.named(Usage, "java-runtime"))
+          }
+      }
     }
+} else {
+ // Gradle 3
+  def usageAttr = Attribute.of('usage', String)
+  configurations {
+      apiConf {
+          attributes {
+              attribute(usageAttr, "java-api")
+          }
+      }
+      runtimeConf {
+          attributes {
+              attribute(usageAttr, "java-runtime")
+          }
+      }
+  }
 }
 
 dependencies {

test/fixtures/multi-config/build.gradle

@@ -11,11 +11,13 @@ repositories {
 }
 
 dependencies {
-  compile 'com.google.guava:guava:18.0'
-  compile 'batik:batik-dom:1.6'
-  compile 'commons-discovery:commons-discovery:0.2'
-  compile 'axis:axis:1.3'
-  compile 'com.android.tools.build:builder:2.3.0'
+  // Gradle 3+ will not pick up "compile" dependencies for "compileOnly"
+  // Gradle 2 will, so for configuration-matching tests we use "runtime"
+  runtime 'com.google.guava:guava:18.0'
+  runtime 'batik:batik-dom:1.6'
+  runtime 'commons-discovery:commons-discovery:0.2'
+  runtime 'axis:axis:1.3'
+  runtime 'com.android.tools.build:builder:2.3.0'
   compileOnly 'javax.servlet:servlet-api:2.5'
 }
 

test/fixtures/multi-project/subproj/build.gradle

@@ -11,11 +11,13 @@ repositories {
 }
 
 dependencies {
-  compile 'com.google.guava:guava:18.0'
-  compile 'batik:batik-dom:1.6'
-  compile 'commons-discovery:commons-discovery:0.2'
+  // Gradle 3+ will not pick up "compile" dependencies for "compileOnly"
+  // Gradle 2 will, so for configuration-matching tests we use "runtime"
+  runtime 'com.google.guava:guava:18.0'
+  runtime 'batik:batik-dom:1.6'
+  runtime 'commons-discovery:commons-discovery:0.2'
   compileOnly 'axis:axis:1.3'
-  compile 'com.android.tools.build:builder:2.3.0'
+  runtime 'com.android.tools.build:builder:2.3.0'
 }
 
 task sourcesJar(type: Jar, dependsOn: classes) {

test/system/failure-states.test.ts

@@ -21,7 +21,7 @@ test('failing inspect()', async (t) => {
   } catch (error) {
     t.match(error.message, 'Please ensure you are calling the `snyk` command with correct arguments',
       'proper error message');
-    t.match(error.message, /Gradle \d+\.\d+\.\d+/,
+    t.match(error.message, /Gradle \d+\.\d+(\.\d+)?/,
       'the error message has Gradle version');
   }
 });

test/system/plugin.test.ts

@@ -1,8 +1,8 @@
 import * as path from 'path';
 import {fixtureDir} from '../common';
 import {test} from 'tap';
-
 import {inspect} from '../../lib';
+import * as subProcess from '../../lib/sub-process';
 
 const rootNoWrapper = fixtureDir('no wrapper');
 
@@ -87,20 +87,25 @@ test('multi-confg: only deps for specified conf are picked up (using legacy CLI
   t.equal(Object.keys(result.package.dependencies!).length, 1, 'top level deps: 1');
 });
 
-if (!process.env.GRADLE_VERSION || !process.env.GRADLE_VERSION!.startsWith('3.')) {
-  test('multi-confg: only deps for specified conf are picked up (attribute match)', async (t) => {
-    const result = await inspect('.',
-      path.join(fixtureDir('multi-config-attributes'), 'build.gradle'),
-      {'configuration-attributes': 'usage:java-api'});
-    t.match(result.package.name, '.',
-      'returned project name is not sub-project');
-    t.notOk(result.package
-      .dependencies!['org.apache.commons:commons-lang3'],
-    'no runtime dep found');
-    t.equal(result.package
-      .dependencies!['commons-httpclient:commons-httpclient'].version,
-    '3.1',
-    'correct version of api dep found');
-    t.equal(Object.keys(result.package.dependencies!).length, 2, 'top level deps: 2'); // 1 with good attr, 1 with no attr
-  });
-}
+test('tests for Gradle 3+', async (t0) => {
+  const gradleVersionOutput = await subProcess.execute('gradle', ['-v'], {});
+  const isGradle3Plus = parseInt(gradleVersionOutput.match(/Gradle (\d+)\.\d+(\.\d+)?/)![1]) >= 3;
+  if (isGradle3Plus) {
+    t0.test('multi-confg: only deps for specified conf are picked up (attribute match)', async (t) => {
+
+      const result = await inspect('.',
+        path.join(fixtureDir('multi-config-attributes'), 'build.gradle'),
+        {'configuration-attributes': 'usage:java-api'});
+      t.match(result.package.name, '.',
+        'returned project name is not sub-project');
+      t.notOk(result.package
+        .dependencies!['org.apache.commons:commons-lang3'],
+      'no runtime dep found');
+      t.equal(result.package
+        .dependencies!['commons-httpclient:commons-httpclient'].version,
+      '3.1',
+      'correct version of api dep found');
+      t.equal(Object.keys(result.package.dependencies!).length, 2, 'top level deps: 2'); // 1 with good attr, 1 with no attr
+    });
+  }
+})