From 50041b23fc34ef76c7845a866323a8d0b8034348 Mon Sep 17 00:00:00 2001 From: Sven Sauleau Date: Fri, 26 Jan 2024 11:56:43 +0100 Subject: [PATCH] don't write SRIs when already existing Refs https://github.com/cdnjs/cdnjs/issues/14080 Bug is reproducible with moment-timezone 0.5.40, given; 1. a minified JS file is already present at `/builds/moment-timezone.min.js` and creates `/moment-timezone.min.js` 2. process-version will minify `/moment-timezone.js` and creates `/moment-timezone.min.js` process-version moves both files to the output and causes a conflict. The actual content (compressed with brotli and gzip) has first write wins semantics while the SRI has last write wins. This change makes SRI first write wins too. Note that since we process files concurrently, it's not possible to say what between 1. and 2. happens first. That means sometimes we will use moment-timezone's provided minfied file and other times our minified version. In both cases the compression + SRI steps are consistent. Demo of the fix: ``` $ bash ./scripts/test-process-version.sh moment-timezone 0.5.40 ... 2024/01/26 11:08:38 file /output/moment-timezone.min.js.sri already exists at the output ... $ cat /tmp/output/moment-timezone.min.js.sri sha512-Dn6u2lfaSQfF/hd5bqRFUHVtwJWfU4osGp3OGKuUyBg39ADAh67iDwK1HWoYE/vEf2UTtZH2ZmA1fsSjA4mgWg== $ cat /tmp/output/moment-timezone.min.js.gz | gunzip | sri-sha512 Dn6u2lfaSQfF/hd5bqRFUHVtwJWfU4osGp3OGKuUyBg39ADAh67iDwK1HWoYE/vEf2UTtZH2ZmA1fsSjA4mgWg== $ cat /tmp/output/moment-timezone.min.js.br | brotli -d | sri-sha512 Dn6u2lfaSQfF/hd5bqRFUHVtwJWfU4osGp3OGKuUyBg39ADAh67iDwK1HWoYE/vEf2UTtZH2ZmA1fsSjA4mgWg== ``` Without this path `moment-timezone.min.js.sri` has `sha512-NJfMpP34NDFAS8lJqH4FzsaD1fqoIJATgBpPjNUck9hC8kGvFhrcR8KIPnTtSinNyx8b1QPBE6NM4iux/0dHXQ==` which reproduces the issue found in https://github.com/cdnjs/cdnjs/issues/14080#issuecomment-1353195273. --- cmd/process-version/main.go | 8 ++++++-- packages/packages.go | 8 ++++---- scripts/test-process-version.sh | 6 ++++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/cmd/process-version/main.go b/cmd/process-version/main.go index 2fb0497..c9653a0 100644 --- a/cmd/process-version/main.go +++ b/cmd/process-version/main.go @@ -89,8 +89,12 @@ func (j optimizeJob) emitFromWorkspace(src string) { ext := path.Ext(src) if _, ok := calculateSRI[ext]; ok { outSRI := fmt.Sprintf("%s.sri", dest) - sri.CalculateFileSRI(src, outSRI) - log.Printf("sri %s -> %s\n", src, outSRI) + if _, err := os.Stat(outSRI); err == nil { + log.Printf("file %s already exists at the output\n", outSRI) + } else { + sri.CalculateFileSRI(src, outSRI) + log.Printf("sri %s -> %s\n", src, outSRI) + } } if _, ok := doNotCompress[ext]; !ok { diff --git a/packages/packages.go b/packages/packages.go index 97cf365..7db8474 100644 --- a/packages/packages.go +++ b/packages/packages.go @@ -176,15 +176,15 @@ func (p *Package) NpmFilesFrom(base string) []NpmFileMoveOp { util.Check(err) // should have already run before in checker so panic if glob invalid for _, f := range list { - fp := path.Join(basePath, f) + filename := path.Join(basePath, f) // check if file has been processed before - if _, ok := seen[fp]; ok { + if _, ok := seen[filename]; ok { continue } - seen[fp] = true + seen[filename] = true - info, staterr := os.Stat(fp) + info, staterr := os.Stat(filename) if staterr != nil { log.Printf("stat: %s\n", staterr.Error()) continue diff --git a/scripts/test-process-version.sh b/scripts/test-process-version.sh index 0e3de69..fe82a0f 100644 --- a/scripts/test-process-version.sh +++ b/scripts/test-process-version.sh @@ -6,19 +6,21 @@ set -e echo "processing $package $version" export DOCKER_BUILDKIT=1 +rm -rf /tmp/output /tmp/input mkdir -p /tmp/input /tmp/output -rm -rf /tmp/output/* /tmp/input/* echo "loading new version files" curl --fail https://storage.googleapis.com/cdnjs-incoming-prod/$package-$version.tgz > /tmp/input/new-version.tgz echo "loading package configuration" curl --fail https://raw.githubusercontent.com/cdnjs/packages/master/packages/${package::1}/$package.json > /tmp/input/config.json -cat /tmp/input/config.json +cat /tmp/input/config.json | jq . echo "----------------- input files -----------------" ls -lh /tmp/input +tar -tvf /tmp/input/new-version.tgz + docker build -f docker/process-version/Dockerfile -t sandbox . docker run -it -v /tmp/input:/input -v /tmp/output:/output sandbox