Skip to content

Commit 98926e6

Browse files
mdjermanovicmdjermanovic
authored
fix: Ensure that extra data is not accidentally stored in the cache file (#17760)
Fixes #13507
1 parent a7a883b commit 98926e6

3 files changed

Lines changed: 216 additions & 6 deletions

File tree

‎lib/cli-engine/lint-result-cache.js‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -128,16 +128,28 @@ class LintResultCache {
128128
return null;
129129
}
130130

131+
const cachedResults = fileDescriptor.meta.results;
132+
133+
// Just in case, not sure if this can ever happen.
134+
if (!cachedResults) {
135+
return cachedResults;
136+
}
137+
138+
/*
139+
* Shallow clone the object to ensure that any properties added or modified afterwards
140+
* will not be accidentally stored in the cache file when `reconcile()` is called.
141+
* https://github.com/eslint/eslint/issues/13507
142+
* All intentional changes to the cache file must be done through `setCachedLintResults()`.
143+
*/
144+
const results = { ...cachedResults };
145+
131146
// If source is present but null, need to reread the file from the filesystem.
132-
if (
133-
fileDescriptor.meta.results &&
134-
fileDescriptor.meta.results.source === null
135-
) {
147+
if (results.source === null) {
136148
debug(`Rereading cached result source from filesystem: ${filePath}`);
137-
fileDescriptor.meta.results.source = fs.readFileSync(filePath, "utf-8");
149+
results.source = fs.readFileSync(filePath, "utf-8");
138150
}
139151

140-
return fileDescriptor.meta.results;
152+
return results;
141153
}
142154

143155
/**

‎tests/lib/eslint/eslint.js‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2993,6 +2993,105 @@ describe("ESLint", () => {
29932993
assert.deepStrictEqual(result, cachedResult, "result should be the same with or without cache");
29942994
});
29952995

2996+
// https://github.com/eslint/eslint/issues/13507
2997+
it("should not store `usedDeprecatedRules` in the cache file", async () => {
2998+
cacheFilePath = getFixturePath(".eslintcache");
2999+
doDelete(cacheFilePath);
3000+
assert(!shell.test("-f", cacheFilePath), "the cache file already exists and wasn't successfully deleted");
3001+
3002+
const deprecatedRuleId = "space-in-parens";
3003+
3004+
eslint = new ESLint({
3005+
cwd: path.join(fixtureDir, ".."),
3006+
useEslintrc: false,
3007+
3008+
// specifying cache true the cache will be created
3009+
cache: true,
3010+
cacheLocation: cacheFilePath,
3011+
overrideConfig: {
3012+
rules: {
3013+
[deprecatedRuleId]: 2
3014+
}
3015+
}
3016+
});
3017+
3018+
const filePath = fs.realpathSync(getFixturePath("cache/src", "test-file.js"));
3019+
3020+
/*
3021+
* Run linting on the same file 3 times to cover multiple cases:
3022+
* Run 1: Lint result wasn't already cached.
3023+
* Run 2: Lint result was already cached. The cached lint result is used but the cache is reconciled before the run ends.
3024+
* Run 3: Lint result was already cached. The cached lint result was being used throughout the previous run, so possible
3025+
* mutations in the previous run that occured after the cache was reconciled may have side effects for this run.
3026+
*/
3027+
for (let i = 0; i < 3; i++) {
3028+
const [result] = await eslint.lintFiles([filePath]);
3029+
3030+
assert(
3031+
result.usedDeprecatedRules && result.usedDeprecatedRules.some(rule => rule.ruleId === deprecatedRuleId),
3032+
"the deprecated rule should have been in result.usedDeprecatedRules"
3033+
);
3034+
3035+
assert(shell.test("-f", cacheFilePath), "the cache for eslint should have been created");
3036+
3037+
const fileCache = fCache.create(cacheFilePath);
3038+
const descriptor = fileCache.getFileDescriptor(filePath);
3039+
3040+
assert(typeof descriptor === "object", "an entry for the file should have been in the cache file");
3041+
assert(typeof descriptor.meta.results === "object", "lint result for the file should have been in its cache entry in the cache file");
3042+
assert(typeof descriptor.meta.results.usedDeprecatedRules === "undefined", "lint result in the cache file contains `usedDeprecatedRules`");
3043+
}
3044+
3045+
});
3046+
3047+
// https://github.com/eslint/eslint/issues/13507
3048+
it("should store `source` as `null` in the cache file if the lint result has `source` property", async () => {
3049+
cacheFilePath = getFixturePath(".eslintcache");
3050+
doDelete(cacheFilePath);
3051+
assert(!shell.test("-f", cacheFilePath), "the cache file already exists and wasn't successfully deleted");
3052+
3053+
eslint = new ESLint({
3054+
cwd: path.join(fixtureDir, ".."),
3055+
useEslintrc: false,
3056+
3057+
// specifying cache true the cache will be created
3058+
cache: true,
3059+
cacheLocation: cacheFilePath,
3060+
overrideConfig: {
3061+
rules: {
3062+
"no-unused-vars": 2
3063+
}
3064+
}
3065+
});
3066+
3067+
const filePath = fs.realpathSync(getFixturePath("cache/src", "fail-file.js"));
3068+
3069+
/*
3070+
* Run linting on the same file 3 times to cover multiple cases:
3071+
* Run 1: Lint result wasn't already cached.
3072+
* Run 2: Lint result was already cached. The cached lint result is used but the cache is reconciled before the run ends.
3073+
* Run 3: Lint result was already cached. The cached lint result was being used throughout the previous run, so possible
3074+
* mutations in the previous run that occured after the cache was reconciled may have side effects for this run.
3075+
*/
3076+
for (let i = 0; i < 3; i++) {
3077+
const [result] = await eslint.lintFiles([filePath]);
3078+
3079+
assert(typeof result.source === "string", "the result should have contained the `source` property");
3080+
3081+
assert(shell.test("-f", cacheFilePath), "the cache for eslint should have been created");
3082+
3083+
const fileCache = fCache.create(cacheFilePath);
3084+
const descriptor = fileCache.getFileDescriptor(filePath);
3085+
3086+
assert(typeof descriptor === "object", "an entry for the file should have been in the cache file");
3087+
assert(typeof descriptor.meta.results === "object", "lint result for the file should have been in its cache entry in the cache file");
3088+
3089+
// if the lint result contains `source`, it should be stored as `null` in the cache file
3090+
assert.strictEqual(descriptor.meta.results.source, null, "lint result in the cache file contains non-null `source`");
3091+
}
3092+
3093+
});
3094+
29963095
describe("cacheStrategy", () => {
29973096
it("should detect changes using a file's modification time when set to 'metadata'", async () => {
29983097
cacheFilePath = getFixturePath(".eslintcache");

‎tests/lib/eslint/flat-eslint.js‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2876,6 +2876,105 @@ describe("FlatESLint", () => {
28762876
assert.deepStrictEqual(result, cachedResult, "result should be the same with or without cache");
28772877
});
28782878

2879+
// https://github.com/eslint/eslint/issues/13507
2880+
it("should not store `usedDeprecatedRules` in the cache file", async () => {
2881+
cacheFilePath = getFixturePath(".eslintcache");
2882+
doDelete(cacheFilePath);
2883+
assert(!shell.test("-f", cacheFilePath), "the cache file already exists and wasn't successfully deleted");
2884+
2885+
const deprecatedRuleId = "space-in-parens";
2886+
2887+
eslint = new FlatESLint({
2888+
cwd: path.join(fixtureDir, ".."),
2889+
overrideConfigFile: true,
2890+
2891+
// specifying cache true the cache will be created
2892+
cache: true,
2893+
cacheLocation: cacheFilePath,
2894+
overrideConfig: {
2895+
rules: {
2896+
[deprecatedRuleId]: 2
2897+
}
2898+
}
2899+
});
2900+
2901+
const filePath = fs.realpathSync(getFixturePath("cache/src", "test-file.js"));
2902+
2903+
/*
2904+
* Run linting on the same file 3 times to cover multiple cases:
2905+
* Run 1: Lint result wasn't already cached.
2906+
* Run 2: Lint result was already cached. The cached lint result is used but the cache is reconciled before the run ends.
2907+
* Run 3: Lint result was already cached. The cached lint result was being used throughout the previous run, so possible
2908+
* mutations in the previous run that occured after the cache was reconciled may have side effects for this run.
2909+
*/
2910+
for (let i = 0; i < 3; i++) {
2911+
const [result] = await eslint.lintFiles([filePath]);
2912+
2913+
assert(
2914+
result.usedDeprecatedRules && result.usedDeprecatedRules.some(rule => rule.ruleId === deprecatedRuleId),
2915+
"the deprecated rule should have been in result.usedDeprecatedRules"
2916+
);
2917+
2918+
assert(shell.test("-f", cacheFilePath), "the cache for eslint should have been created");
2919+
2920+
const fileCache = fCache.create(cacheFilePath);
2921+
const descriptor = fileCache.getFileDescriptor(filePath);
2922+
2923+
assert(typeof descriptor === "object", "an entry for the file should have been in the cache file");
2924+
assert(typeof descriptor.meta.results === "object", "lint result for the file should have been in its cache entry in the cache file");
2925+
assert(typeof descriptor.meta.results.usedDeprecatedRules === "undefined", "lint result in the cache file contains `usedDeprecatedRules`");
2926+
}
2927+
2928+
});
2929+
2930+
// https://github.com/eslint/eslint/issues/13507
2931+
it("should store `source` as `null` in the cache file if the lint result has `source` property", async () => {
2932+
cacheFilePath = getFixturePath(".eslintcache");
2933+
doDelete(cacheFilePath);
2934+
assert(!shell.test("-f", cacheFilePath), "the cache file already exists and wasn't successfully deleted");
2935+
2936+
eslint = new FlatESLint({
2937+
cwd: path.join(fixtureDir, ".."),
2938+
overrideConfigFile: true,
2939+
2940+
// specifying cache true the cache will be created
2941+
cache: true,
2942+
cacheLocation: cacheFilePath,
2943+
overrideConfig: {
2944+
rules: {
2945+
"no-unused-vars": 2
2946+
}
2947+
}
2948+
});
2949+
2950+
const filePath = fs.realpathSync(getFixturePath("cache/src", "fail-file.js"));
2951+
2952+
/*
2953+
* Run linting on the same file 3 times to cover multiple cases:
2954+
* Run 1: Lint result wasn't already cached.
2955+
* Run 2: Lint result was already cached. The cached lint result is used but the cache is reconciled before the run ends.
2956+
* Run 3: Lint result was already cached. The cached lint result was being used throughout the previous run, so possible
2957+
* mutations in the previous run that occured after the cache was reconciled may have side effects for this run.
2958+
*/
2959+
for (let i = 0; i < 3; i++) {
2960+
const [result] = await eslint.lintFiles([filePath]);
2961+
2962+
assert(typeof result.source === "string", "the result should have contained the `source` property");
2963+
2964+
assert(shell.test("-f", cacheFilePath), "the cache for eslint should have been created");
2965+
2966+
const fileCache = fCache.create(cacheFilePath);
2967+
const descriptor = fileCache.getFileDescriptor(filePath);
2968+
2969+
assert(typeof descriptor === "object", "an entry for the file should have been in the cache file");
2970+
assert(typeof descriptor.meta.results === "object", "lint result for the file should have been in its cache entry in the cache file");
2971+
2972+
// if the lint result contains `source`, it should be stored as `null` in the cache file
2973+
assert.strictEqual(descriptor.meta.results.source, null, "lint result in the cache file contains non-null `source`");
2974+
}
2975+
2976+
});
2977+
28792978
describe("cacheStrategy", () => {
28802979
it("should detect changes using a file's modification time when set to 'metadata'", async () => {
28812980
cacheFilePath = getFixturePath(".eslintcache");

0 commit comments

Comments
 (0)