Skip to content

Commit

Permalink
Merge pull request #99 from snyk/fix/custom-file-names-fix
Browse files Browse the repository at this point in the history 
fix: use file contents to update deps not a file object (support for custom named manifests)
  • Loading branch information
@lili2311
lili2311 authored Jan 29, 2020
2 parents ea748d9 + 5355d17 commit 95e05cd
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 156 deletions.
2 changes: 1 addition & 1 deletion lib/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ export {
PythonInspectOptions,
} from './dependencies';

export { updateDependencies as applyRemediationToManifests } from './update-dependencies';
export { updateDependencies } from './update-dependencies';
2 changes: 1 addition & 1 deletion lib/requirements-file-parser.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
type VersionComparator = '<' | '<=' | '!=' | '==' | '>=' | '>' | '~=';

interface Requirement {
export interface Requirement {
originalText: string;
line: number;
name?: string;
Expand Down
57 changes: 20 additions & 37 deletions lib/update-dependencies/requirements/index.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import * as path from 'path';

import { ManifestFiles, DependencyUpdates } from '../../types';
import { parseRequirementsFile } from '../../requirements-file-parser';
import { DependencyUpdates } from '../../types';
import {
parseRequirementsFile,
Requirement,
} from '../../requirements-file-parser';

/**
* Given contents of manifest file(s) and a set of upgrades, apply the given
Expand All @@ -11,43 +12,15 @@ import { parseRequirementsFile } from '../../requirements-file-parser';
* `requirements.txt` must be in the manifests.
**/
export function updateDependencies(
manifests: ManifestFiles,
requirementsTxt: string,
upgrades: DependencyUpdates
) {
const manifestNames = Object.keys(manifests);
const targetFile = manifestNames.find(
(fn) => path.basename(fn) === 'requirements.txt'
);
if (
!targetFile ||
!manifestNames.every((fn) => path.basename(fn) === 'requirements.txt')
) {
throw new Error('Remediation only supported for requirements.txt file');
}

if (Object.keys(upgrades).length === 0) {
return manifests;
return requirementsTxt;
}

const requirementsFileName = Object.keys(manifests).find(
(fn) => path.basename(fn) === 'requirements.txt'
);

// If there is no usable manifest, return
if (typeof requirementsFileName === 'undefined') {
return manifests;
}

const requirementsFile = manifests[requirementsFileName];

const requirements = parseRequirementsFile(requirementsFile);

// This is a bit of a hack, but an easy one to follow. If a file ends with a
// new line, ensure we keep it this way. Don't hijack customers formatting.
let endsWithNewLine = false;
if (requirements[requirements.length - 1].originalText === '\n') {
endsWithNewLine = true;
}
const requirements = parseRequirementsFile(requirementsTxt);
const endsWithNewLine = fileEndsWithNewLine(requirements);

const topLevelDeps = requirements
.map(({ name }) => name && name.toLowerCase())
Expand Down Expand Up @@ -108,11 +81,21 @@ export function updateDependencies(
updatedManifest += '\n';
}

return { [requirementsFileName]: updatedManifest };
return updatedManifest;
}

// TS is not capable of determining when Array.filter has removed undefined
// values without a manual Type Guard, so thats what this does
function isDefined<T>(t: T | undefined): t is T {
return typeof t !== 'undefined';
}

function fileEndsWithNewLine(sanitisedFile: Requirement[]): boolean {
// This is a bit of a hack, but an easy one to follow. If a file ends with a
// new line, ensure we keep it this way. Don't hijack customers formatting.
let endsWithNewLine = false;
if (sanitisedFile[sanitisedFile.length - 1].originalText === '\n') {
endsWithNewLine = true;
}
return endsWithNewLine;
}
177 changes: 60 additions & 117 deletions test/remediation.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { readFileSync } from 'fs';
import * as path from 'path';
import { updateDependencies as applyRemediationToManifests } from '../lib/update-dependencies';
import { updateDependencies } from '../lib/update-dependencies';

describe('remediation', () => {
it('does not add extra new lines', () => {
Expand All @@ -9,91 +9,71 @@ describe('remediation', () => {
'transitive@1.0.0': { upgradeTo: 'transitive@1.1.1' },
};

const manifests = {
'requirements.txt': 'Django==1.6.1',
};
const manifestContents = 'Django==1.6.1';

const expectedManifests = {
'requirements.txt':
'Django==2.0.1\ntransitive>=1.1.1 # not directly required, pinned by Snyk to avoid a vulnerability',
};
const expectedManifest =
'Django==2.0.1\ntransitive>=1.1.1 # not directly required, pinned by Snyk to avoid a vulnerability';

const result = applyRemediationToManifests(manifests, upgrades);
const result = updateDependencies(manifestContents, upgrades);

// Note no extra newline was added to the expected manifest
expect(result).toEqual(expectedManifests);
expect(result).toEqual(expectedManifest);
});

it('ignores casing in upgrades (treats all as lowercase)', () => {
const upgrades = {
'Django@1.6.1': { upgradeTo: 'Django@2.0.1' },
};

const manifests = {
'requirements.txt': 'django==1.6.1\n',
};
const manifestContents = 'django==1.6.1\n';

const expectedManifests = {
'requirements.txt': 'django==2.0.1\n',
};
const expectedManifest = 'django==2.0.1\n';

const result = applyRemediationToManifests(manifests, upgrades);
const result = updateDependencies(manifestContents, upgrades);

expect(result).toEqual(expectedManifests);
expect(result).toEqual(expectedManifest);
});

it('maintains package name casing when upgrading', () => {
const upgrades = {
'django@1.6.1': { upgradeTo: 'django@2.0.1' },
};

const manifests = {
'requirements.txt': 'Django==1.6.1\n',
};
const manifestContents = 'Django==1.6.1\n';

const expectedManifests = {
'requirements.txt': 'Django==2.0.1\n',
};
const expectedManifest = 'Django==2.0.1\n';

const result = applyRemediationToManifests(manifests, upgrades);
const result = updateDependencies(manifestContents, upgrades);

expect(result).toEqual(expectedManifests);
expect(result).toEqual(expectedManifest);
});

it('matches a package with multiple digit versions i.e. 12.123.14', () => {
const upgrades = {
'foo@12.123.14': { upgradeTo: 'foo@55.66.7' },
};

const manifests = {
'requirements.txt': 'foo==12.123.14\n',
};
const manifestContents = 'foo==12.123.14\n';

const expectedManifests = {
'requirements.txt': 'foo==55.66.7\n',
};
const expectedManifest = 'foo==55.66.7\n';

const result = applyRemediationToManifests(manifests, upgrades);
const result = updateDependencies(manifestContents, upgrades);

expect(result).toEqual(expectedManifests);
expect(result).toEqual(expectedManifest);
});

it('maintains comments when upgrading', () => {
const upgrades = {
'django@1.6.1': { upgradeTo: 'django@2.0.1' },
};

const manifests = {
'requirements.txt': 'django==1.6.1 # this is a comment\n',
};
const manifestContents = 'django==1.6.1 # this is a comment\n';

const expectedManifests = {
'requirements.txt': 'django==2.0.1 # this is a comment\n',
};
const expectedManifest = 'django==2.0.1 # this is a comment\n';

const result = applyRemediationToManifests(manifests, upgrades);
const result = updateDependencies(manifestContents, upgrades);

expect(result).toEqual(expectedManifests);
expect(result).toEqual(expectedManifest);
});

it('maintains version comparator when upgrading', () => {
Expand All @@ -102,17 +82,13 @@ describe('remediation', () => {
'click@7.0': { upgradeTo: 'click@7.1' },
};

const manifests = {
'requirements.txt': 'django>=1.6.1\nclick>7.0',
};
const manifestContents = 'django>=1.6.1\nclick>7.0';

const expectedManifests = {
'requirements.txt': 'django>=2.0.1\nclick>7.1',
};
const expectedManifest = 'django>=2.0.1\nclick>7.1';

const result = applyRemediationToManifests(manifests, upgrades);
const result = updateDependencies(manifestContents, upgrades);

expect(result).toEqual(expectedManifests);
expect(result).toEqual(expectedManifest);
});

it('fixes a pip app', () => {
Expand All @@ -121,94 +97,61 @@ describe('remediation', () => {
'transitive@1.0.0': { upgradeTo: 'transitive@1.1.1' },
};

const manifests = {
'requirements.txt': readFileSync(
path.resolve('test', 'workspaces', 'pip-app', 'requirements.txt'),
'utf8'
),
};
const manifestContents = readFileSync(
path.resolve('test', 'workspaces', 'pip-app', 'requirements.txt'),
'utf8'
);

const expectedManifests = {
'requirements.txt': readFileSync(
path.resolve(
'test',
'fixtures',
'updated-manifest',
'requirements.txt'
),
'utf8'
),
};
const expectedManifest = readFileSync(
path.resolve('test', 'fixtures', 'updated-manifest', 'requirements.txt'),
'utf8'
);

const result = applyRemediationToManifests(manifests, upgrades);
const result = updateDependencies(manifestContents, upgrades);

expect(result).toEqual(expectedManifests);
expect(result).toEqual(expectedManifest);
});

it('retains python markers', () => {
const upgrades = {
'click@7.0': { upgradeTo: 'click@7.1' },
};

const manifests = {
'requirements.txt': readFileSync(
path.resolve(
'test',
'workspaces',
'pip-app-with-python-markers',
'requirements.txt'
),
'utf8'
const manifestContents = readFileSync(
path.resolve(
'test',
'workspaces',
'pip-app-with-python-markers',
'requirements.txt'
),
};
'utf8'
);

const expectedManifests = {
'requirements.txt': readFileSync(
path.resolve(
'test',
'fixtures',
'updated-manifests-with-python-markers',
'requirements.txt'
),
'utf8'
const expectedManifest = readFileSync(
path.resolve(
'test',
'fixtures',
'updated-manifests-with-python-markers',
'requirements.txt'
),
};
'utf8'
);

const result = applyRemediationToManifests(manifests, upgrades);
const result = updateDependencies(manifestContents, upgrades);

expect(result).toEqual(expectedManifests);
expect(result).toEqual(expectedManifest);
});

it('handles no-op upgrades', () => {
const upgrades = {};

const manifests = {
'requirements.txt': readFileSync(
path.resolve('test', 'workspaces', 'pip-app', 'requirements.txt'),
'utf8'
),
};

const result = applyRemediationToManifests(manifests, upgrades);

expect(result).toEqual(manifests);
});

it('cannot fix a Pipfile app', () => {
const upgrades = {
'Django@1.6.1': { upgradeTo: 'Django@2.0.1' },
'transitive@1.0.0': { upgradeTo: 'transitive@1.1.1' },
};
const manifestContents = readFileSync(
path.resolve('test', 'workspaces', 'pip-app', 'requirements.txt'),
'utf8'
);

const manifests = {
Pipfile: readFileSync(
path.resolve('test', 'workspaces', 'pipfile-pipapp', 'Pipfile'),
'utf8'
),
};
const result = updateDependencies(manifestContents, upgrades);

expect(() => applyRemediationToManifests(manifests, upgrades)).toThrow(
'Remediation only supported for requirements.txt file'
);
expect(result).toEqual(manifestContents);
});
});

0 comments on commit 95e05cd

Please sign in to comment.