From fa6acd9c40f997909bace380094b8ce5f7c15093 Mon Sep 17 00:00:00 2001 From: Or Sagie Date: Tue, 30 Jul 2019 17:30:23 +0300 Subject: [PATCH] fix: pr comments --- lib/nuget-parser/csproj-parser.ts | 69 +++++++------ lib/nuget-parser/dependency.ts | 2 +- lib/nuget-parser/dotnet-core-parser.ts | 78 +++++++-------- lib/nuget-parser/dotnet-framework-parser.ts | 105 ++++++++++---------- lib/nuget-parser/nuspec-parser.ts | 1 - lib/nuget-parser/packages-config-parser.ts | 6 +- lib/nuget-parser/project-json-parser.ts | 21 ++-- package.json | 2 +- test/csproj.test.ts | 2 +- tsconfig.json | 3 +- 10 files changed, 151 insertions(+), 138 deletions(-) diff --git a/lib/nuget-parser/csproj-parser.ts b/lib/nuget-parser/csproj-parser.ts index 9d6be8ff..f8a4d46e 100644 --- a/lib/nuget-parser/csproj-parser.ts +++ b/lib/nuget-parser/csproj-parser.ts @@ -7,41 +7,50 @@ import * as _ from 'lodash'; import * as debugModule from 'debug'; const debug = debugModule('snyk'); -export async function getTargetFrameworksFromProjFile(rootDir) { - debug('Looking for your .csproj file in ' + rootDir); - const csprojPath = findFile(rootDir, /.*\.csproj$/); - if (csprojPath) { - debug('Checking .net framework version in .csproj file ' + csprojPath); +interface TargetFramework { + framework: string; + original: string; + version: string; +} - const csprojContents = fs.readFileSync(csprojPath); +export async function getTargetFrameworksFromProjFile(rootDir: string): Promise { + return new Promise((resolve, reject) => { + debug('Looking for your .csproj file in ' + rootDir); + const csprojPath = findFile(rootDir, /.*\.csproj$/); + if (csprojPath) { + debug('Checking .net framework version in .csproj file ' + csprojPath); - let frameworks: any = []; - parseXML.parseString(csprojContents, (err, parsedCsprojContents) => { - if (err) { - throw new FileNotProcessableError(err); - } - const versionLoc = _.get(parsedCsprojContents, 'Project.PropertyGroup[0]'); - const versions = _.compact(_.concat([], - _.get(versionLoc, 'TargetFrameworkVersion[0]') || - _.get(versionLoc, 'TargetFramework[0]') || - _.get(versionLoc, 'TargetFrameworks[0]', '').split(';'))); + const csprojContents = fs.readFileSync(csprojPath); - if (versions.length < 1) { - debug('Could not find TargetFrameworkVersion/TargetFramework' + - '/TargetFrameworks defined in the Project.PropertyGroup field of ' + - 'your .csproj file'); - } - frameworks = _.compact(_.map(versions, toReadableFramework)); - if (versions.length > 1 && frameworks.length < 1) { - debug('Could not find valid/supported .NET version in csproj file located at' + csprojPath); - } - }); - return frameworks[0]; - } - debug('.csproj file not found in ' + rootDir + '.'); + let frameworks: TargetFramework[] = []; + parseXML.parseString(csprojContents, (err, parsedCsprojContents) => { + if (err) { + reject(new FileNotProcessableError(err)); + } + const versionLoc = _.get(parsedCsprojContents, 'Project.PropertyGroup[0]'); + const versions = _.compact(_.concat([], + _.get(versionLoc, 'TargetFrameworkVersion[0]') || + _.get(versionLoc, 'TargetFramework[0]') || + _.get(versionLoc, 'TargetFrameworks[0]', '').split(';'))); + + if (versions.length < 1) { + debug('Could not find TargetFrameworkVersion/TargetFramework' + + '/TargetFrameworks defined in the Project.PropertyGroup field of ' + + 'your .csproj file'); + } + frameworks = _.compact(_.map(versions, toReadableFramework)); + if (versions.length > 1 && frameworks.length < 1) { + debug('Could not find valid/supported .NET version in csproj file located at' + csprojPath); + } + resolve(frameworks[0]); + }); + } + debug('.csproj file not found in ' + rootDir + '.'); + resolve(); + }); } -function toReadableFramework(targetFramework) { +function toReadableFramework(targetFramework: string): TargetFramework | undefined { const typeMapping = { net: '.NETFramework', netcoreapp: '.NETCore', diff --git a/lib/nuget-parser/dependency.ts b/lib/nuget-parser/dependency.ts index d2e2ff75..6a2459ca 100644 --- a/lib/nuget-parser/dependency.ts +++ b/lib/nuget-parser/dependency.ts @@ -37,7 +37,7 @@ export function fromFolderName(folderName) { }; } -export function fromPackgesConfigEntry(manifest) { +export function fromPackagesConfigEntry(manifest) { debug('Extracting by packages.config entry:' + ' name = ' + manifest.$.id + ' version = ' + manifest.$.version); diff --git a/lib/nuget-parser/dotnet-core-parser.ts b/lib/nuget-parser/dotnet-core-parser.ts index 233708ff..f704248f 100644 --- a/lib/nuget-parser/dotnet-core-parser.ts +++ b/lib/nuget-parser/dotnet-core-parser.ts @@ -1,10 +1,19 @@ -import {BigTreeError, InvalidManifestError} from '../errors'; +import {BigTreeError, FileNotProcessableError, InvalidManifestError} from '../errors'; import * as _ from 'lodash'; import * as debugModule from 'debug'; const debug = debugModule('snyk'); // TODO: any convention for global vars? (gFreqDeps) -const freqDeps: any = {}; +interface FreqDepParent { + dependencies: any; + name: 'freqSystemDependencies'; + version: number; +} + +interface FreqDeps { + [dep: string]: boolean | FreqDepParent; +} +const freqDeps: FreqDeps = {}; function initFreqDepsDict() { freqDeps['Microsoft.NETCore.Platforms'] = false; @@ -137,48 +146,39 @@ function validateManifest(manifest) { } } -module.exports = { - parse: (tree, manifest) => { - return new Promise(function parseFileContents(resolve, reject) { - debug('Trying to parse dot-net-cli manifest'); +export async function parse (tree, manifest) { + debug('Trying to parse dot-net-cli manifest'); - try { - validateManifest(manifest); - } catch (err) { - debug('Invalid project.assets.json manifest file'); - reject(err); - } + validateManifest(manifest); - if (manifest.project.version) { - tree.version = manifest.project.version; - } + if (manifest.project.version) { + tree.version = manifest.project.version; + } - // If a targetFramework was not found in the proj file, we will extract it from the lock file - if (!tree.meta.targetFramework) { - tree.meta.targetFramework = getFrameworkToRun(manifest); - } - const selectedFrameworkObj = manifest.project.frameworks[tree.meta.targetFramework]; + // If a targetFramework was not found in the proj file, we will extract it from the lock file + if (!tree.meta.targetFramework) { + tree.meta.targetFramework = getFrameworkToRun(manifest); + } + const selectedFrameworkObj = manifest.project.frameworks[tree.meta.targetFramework]; - // We currently ignore the found targetFramework when looking for target dependencies - const selectedTargetObj = getTargetObjToRun(manifest); + // We currently ignore the found targetFramework when looking for target dependencies + const selectedTargetObj = getTargetObjToRun(manifest); - initFreqDepsDict(); + initFreqDepsDict(); - const directDependencies = collectFlatList(selectedFrameworkObj.dependencies); - debug(`directDependencies: '${directDependencies}'`); + const directDependencies = collectFlatList(selectedFrameworkObj.dependencies); + debug(`directDependencies: '${directDependencies}'`); - directDependencies.forEach((directDep) => { - debug(`First order dep: '${directDep}'`); - buildTreeRecursive(selectedTargetObj, directDep, tree, 0); - }); + directDependencies.forEach((directDep) => { + debug(`First order dep: '${directDep}'`); + buildTreeRecursive(selectedTargetObj, directDep, tree, 0); + }); - if (!_.isEmpty(freqDeps.dependencies.dependencies)) { - tree.dependencies.freqSystemDependencies = freqDeps.dependencies; - } - // to disconnect the object references inside the tree - // JSON parse/stringify is used - tree.dependencies = JSON.parse(JSON.stringify(tree.dependencies)); - resolve(tree); - }); - }, -}; + if (!_.isEmpty((freqDeps.dependencies as FreqDepParent).dependencies)) { + tree.dependencies.freqSystemDependencies = freqDeps.dependencies; + } + // to disconnect the object references inside the tree + // JSON parse/stringify is used + tree.dependencies = JSON.parse(JSON.stringify(tree.dependencies)); + return tree; +} diff --git a/lib/nuget-parser/dotnet-framework-parser.ts b/lib/nuget-parser/dotnet-framework-parser.ts index c747d47c..fc230370 100644 --- a/lib/nuget-parser/dotnet-framework-parser.ts +++ b/lib/nuget-parser/dotnet-framework-parser.ts @@ -1,6 +1,6 @@ import * as fs from 'fs'; import * as path from 'path'; -import * as dependency from './dependency'; +import {Dependency, cloneShallow, fromFolderName} from './dependency'; import {parseNuspec} from './nuspec-parser'; import * as debugModule from 'debug'; const debug = debugModule('snyk'); @@ -29,7 +29,7 @@ function scanInstalled(installedPackages, packagesFolder) { fs.readdirSync(packagesFolder) .map((folderName) => { try { - return dependency.fromFolderName(folderName); + return fromFolderName(folderName); } catch (err) { debug('Unable to parse dependency from folder'); debug(err); @@ -59,15 +59,15 @@ function scanInstalled(installedPackages, packagesFolder) { } async function fetchNugetInformationFromPackages(flattenedPackageList, targetFramework) { - const nuspecParserChain: any = []; + const nugetPackageInformation: any[] = []; // begin collecting information from .nuget files on installed packages debug('Trying to analyze .nuspec files'); for (const name of Object.keys(flattenedPackageList)) { const dep = flattenedPackageList[name]; debug('...' + name); - nuspecParserChain.push(await parseNuspec(dep, targetFramework)); + nugetPackageInformation.push(await parseNuspec(dep, targetFramework)); } - return Promise.all(nuspecParserChain); + return nugetPackageInformation; } function processNugetInformation(nuspecResolutionChain) { @@ -82,59 +82,58 @@ function processNugetInformation(nuspecResolutionChain) { return nuspecResolutions; } +function buildTree(node, requiredChildren, flattenedPackageList, nuspecResolutions) { + for (const requiredChild of requiredChildren) { + let transitiveDependency: Dependency; + if (flattenedPackageList[requiredChild.name]) { + // fetch from repo + transitiveDependency = cloneShallow(flattenedPackageList[requiredChild.name]); + } else { + // create as new (uninstalled) + transitiveDependency = { + dependencies: {}, + name: requiredChild.name, + version: requiredChild.version, + }; + } + const transitiveChildren = + (nuspecResolutions[transitiveDependency.name] && + nuspecResolutions[transitiveDependency.name].children) || []; + buildTree( + transitiveDependency, + transitiveChildren, + flattenedPackageList, + nuspecResolutions); + node.dependencies[transitiveDependency.name] = transitiveDependency; + } +} + + export async function parse(tree, manifest, targetFramework, packagesFolder) { if (!targetFramework) { throw new Error('No valid Dotnet target framework found'); } const flattenedPackageList = scanInstalled(manifest, packagesFolder); - return fetchNugetInformationFromPackages(flattenedPackageList, targetFramework) - .then(processNugetInformation) - .then(function buildDependencyTree(nuspecResolutions) { - // .nuget parsing is complete, returned as array of promise resolutions - // now the flat list should be rebuilt as a tree - debug('Building dependency tree'); - - function buildTree(node, requiredChildren, repository) { - requiredChildren.forEach((requiredChild) => { - let transitiveDependency: dependency.Dependency; - if (flattenedPackageList[requiredChild.name]) { - // fetch from repo - transitiveDependency = - dependency.cloneShallow(flattenedPackageList[requiredChild.name]); - } else { - // create as new (uninstalled) - transitiveDependency = { - dependencies: {}, - name: requiredChild.name, - version: requiredChild.version, - }; - } - const transitiveChildren = - (nuspecResolutions[transitiveDependency.name] && - nuspecResolutions[transitiveDependency.name].children) || []; - buildTree( - transitiveDependency, - transitiveChildren, - repository); - node.dependencies[transitiveDependency.name] = transitiveDependency; - }); - } + const nugetPackageInformation = await fetchNugetInformationFromPackages(flattenedPackageList, targetFramework); + const nuspecResolutions = processNugetInformation(nugetPackageInformation); + // .nuget parsing is complete, returned as array of promise resolutions + // now the flat list should be rebuilt as a tree + debug('Building dependency tree'); - const nugtKeys = Object.keys(nuspecResolutions); - Object.keys(flattenedPackageList).forEach((packageName) => { - tree.dependencies[packageName] = - dependency.cloneShallow(flattenedPackageList[packageName]); - }); - if (nugtKeys.length > 0) { - // local folders scanned, build list from .nuspec - for (const key of nugtKeys) { - const resolution = nuspecResolutions[key]; - const node = dependency.cloneShallow(flattenedPackageList[resolution.name]); - buildTree(node, resolution.children, flattenedPackageList); - tree.dependencies[node.name] = node; - } - } - return tree; - }); + const nugetKeys = Object.keys(nuspecResolutions); + Object.keys(flattenedPackageList).forEach((packageName) => { + tree.dependencies[packageName] = + cloneShallow(flattenedPackageList[packageName]); + }); + if (nugetKeys.length > 0) { + // local folders scanned, build list from .nuspec + for (const key of nugetKeys) { + const resolution = nuspecResolutions[key]; + const node = cloneShallow(flattenedPackageList[resolution.name]); + buildTree(node, resolution.children, flattenedPackageList, nuspecResolutions); + tree.dependencies[node.name] = node; + } + } + return tree; } diff --git a/lib/nuget-parser/nuspec-parser.ts b/lib/nuget-parser/nuspec-parser.ts index 1d3d9b0c..417d173b 100644 --- a/lib/nuget-parser/nuspec-parser.ts +++ b/lib/nuget-parser/nuspec-parser.ts @@ -34,7 +34,6 @@ export async function parseNuspec(dep, targetFramework) { // We are only going to check the first targetFramework we encounter // in the future we may want to support multiple, but only once // we have dependency version conflict resolution implemented - // _(targetFrameworks).forEach((targetFramework) => { _(result.package.metadata).forEach((metadata) => { _(metadata.dependencies).forEach((rawDependency) => { diff --git a/lib/nuget-parser/packages-config-parser.ts b/lib/nuget-parser/packages-config-parser.ts index 0f21ec2e..b953557e 100644 --- a/lib/nuget-parser/packages-config-parser.ts +++ b/lib/nuget-parser/packages-config-parser.ts @@ -1,10 +1,10 @@ import * as parseXML from 'xml2js'; import * as debugModule from 'debug'; const debug = debugModule('snyk'); -import * as dependency from './dependency'; +import {Dependency, fromPackagesConfigEntry} from './dependency'; export function parse(fileContent) { - const installedPackages: dependency.Dependency[] = []; + const installedPackages: Dependency[] = []; debug('Trying to parse packages.config manifest'); parseXML.parseString(fileContent, (err, result) => { if (err) { @@ -15,7 +15,7 @@ export function parse(fileContent) { packages.forEach( function scanPackagesConfigNode(node) { const installedDependency = - dependency.fromPackgesConfigEntry(node); + fromPackagesConfigEntry(node); installedPackages.push(installedDependency); }); } diff --git a/lib/nuget-parser/project-json-parser.ts b/lib/nuget-parser/project-json-parser.ts index 51c1aab0..9500814b 100644 --- a/lib/nuget-parser/project-json-parser.ts +++ b/lib/nuget-parser/project-json-parser.ts @@ -2,7 +2,7 @@ import * as dependency from './dependency'; import * as debugModule from 'debug'; const debug = debugModule('snyk'); -function scanForDependencies(obj, deps) { +function scanForDependencies(obj, deps): JsonManifestDependencies { deps = deps || {}; if (typeof obj !== 'object') { return deps; @@ -30,19 +30,26 @@ function scanForDependencies(obj, deps) { return deps; } -function parseJsonManifest(fileContent) { +interface JsonManifestDependencies{ + dependencies: any; + project?: { + version: string; + name: string; + }; +} + +function parseJsonManifest(fileContent): JsonManifestDependencies { const rawContent = JSON.parse(fileContent); - const result: any = {}; - result.dependencies = scanForDependencies(rawContent, {}); + const result: JsonManifestDependencies = { + dependencies: scanForDependencies(rawContent, {}), + }; if (typeof rawContent.project === 'object') { const pData = rawContent.project; const name = (pData.restore && pData.restore.projectName); result.project = { version: pData.version || '0.0.0', + name, }; - if (name) { - result.project.name = name; - } } return result; } diff --git a/package.json b/package.json index 28a276f4..eff92617 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "author": "snyk.io", "license": "Apache-2.0", "engines": { - "node": ">=4" + "node": ">=6" }, "files": [ "bin", diff --git a/test/csproj.test.ts b/test/csproj.test.ts index e9bf01db..76fa4fb4 100644 --- a/test/csproj.test.ts +++ b/test/csproj.test.ts @@ -11,7 +11,7 @@ const manifestFile = 'obj/project.assets.json'; test('parse dotnet with csproj containing multiple versions retrieves first one', async (t) => { const dotnetVersions = await getTargetFrameworksFromProjFile( multipleFrameworksPath); - t.equal('netcoreapp2.0', dotnetVersions.original); + t.equal('netcoreapp2.0', dotnetVersions!.original); }); test('parse dotnet with vbproj', async (t) => { diff --git a/tsconfig.json b/tsconfig.json index 75f88a55..5f939f90 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,8 +4,7 @@ "pretty": true, "target": "es2015", "lib": [ - "es2015", - "es2016.array.include", + "es2015" ], "module": "commonjs", "allowJs": true,