Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@actions/cache mutates the paths argument. #1579

Closed
ramblingenzyme opened this issue Nov 9, 2023 · 1 comment
Closed

@actions/cache mutates the paths argument. #1579

ramblingenzyme opened this issue Nov 9, 2023 · 1 comment
Labels
bug Something isn't working

Comments

@ramblingenzyme
Copy link

ramblingenzyme commented Nov 9, 2023

Describe the bug
https://github.com/actions/toolkit/blob/main/packages/cache/src/internal/cacheHttpClient.ts#L83
This line means that the changes to the array change the array the user passes.
This breaks code such as

import * as cache from "@actions/cache";

const paths = ["some/paths"];
const key = "a-key";

// This calls `getCacheVersion` internally and mutates `paths`
const cacheKey = await cache.restoreCache(paths, key);
if (cacheKey) {
  core.info(`Cache restored from key: ${cacheKey}`);
  return;
}

// At this point the variable `paths = ["some/paths", "zstd-without-long", "1.0"]`


const savedCacheKey = await cache.saveCache(paths, key);
// This means when `saveCache` calls `getCacheVersion` it was actually calculated off of the final array value: `["some/paths", "zstd-without-long", "1.0", "zstd-without-long", "1.0"]`
}

This means that we run into a version mismatch when restoring the cache in another step later on.

Expected behavior
Inputs to functions don't get mutated and break intended functionality of the library.

Screenshots
image
This is following the hashing to create versions, with the inputs from our code, and you can see that components2 with the compression and version salt in twice matches the version of the created caches.

@ramblingenzyme ramblingenzyme added the bug Something isn't working label Nov 9, 2023
Smeb added a commit to Smeb/toolkit that referenced this issue Nov 27, 2023
@ramblingenzyme ramblingenzyme reopened this Mar 3, 2024
@bethanyj28
Copy link
Contributor

Fixed in #1378 (v3.2.3)

Smeb added a commit to Smeb/toolkit that referenced this issue Mar 7, 2024
bethanyj28 added a commit that referenced this issue Mar 7, 2024
fix #1579: add test to check getCacheVersion does not mutate arguments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants