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

fix: truncate screenshot filenames using byteLength #8175

Merged
merged 5 commits into from Aug 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/server/__snapshots__/5_screenshots_spec.js
Expand Up @@ -147,11 +147,11 @@ Because this error occurred during a \`after each\` hook we are skipping the rem
- /XXX/XXX/XXX/cypress/screenshots/screenshots_spec.js/taking screenshots -- reall (1000x660)
y long test title aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.png
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.png
- /XXX/XXX/XXX/cypress/screenshots/screenshots_spec.js/taking screenshots -- reall (1000x660)
y long test title aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa (1).png
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa (1).png


(Video)
Expand Down
53 changes: 38 additions & 15 deletions packages/server/lib/screenshots.js
Expand Up @@ -18,6 +18,14 @@ const pathSeparatorRe = /[\\\/]/g
// internal id incrementor
let __ID__ = null

// many filesystems limit filename length to 255 bytes/characters, so truncate the filename to
// the smallest common denominator of safe filenames, which is 255 bytes. when ENAMETOOLONG
// errors are encountered, `maxSafeBytes` will be decremented to at most `MIN_PREFIX_BYTES`, at
// which point the latest ENAMETOOLONG error will be emitted.
// @see https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits
let maxSafeBytes = Number(process.env.CYPRESS_MAX_SAFE_FILENAME_BYTES) || 254
const MIN_PREFIX_BYTES = 64

// TODO: when we parallelize these builds we'll need
// a semaphore to access the file system when we write
// screenshots since its possible two screenshots with
Expand Down Expand Up @@ -288,16 +296,41 @@ const getDimensions = function (details) {
return pick(details.image.bitmap)
}

const ensureUniquePath = function (withoutExt, extension, num = 0) {
const fullPath = num ? `${withoutExt} (${num}).${extension}` : `${withoutExt}.${extension}`
const ensureSafePath = function (withoutExt, extension, num = 0) {
const suffix = `${num ? ` (${num})` : ''}.${extension}`
const maxSafePrefixBytes = maxSafeBytes - suffix.length
const filenameBuf = Buffer.from(path.basename(withoutExt))

if (filenameBuf.byteLength > maxSafePrefixBytes) {
const truncated = filenameBuf.slice(0, maxSafePrefixBytes).toString()

withoutExt = path.join(path.dirname(withoutExt), truncated)
}

const fullPath = [withoutExt, suffix].join('')

debug('ensureSafePath %o', { withoutExt, extension, num, maxSafeBytes, maxSafePrefixBytes })

return fs.pathExists(fullPath)
.then((found) => {
if (found) {
return ensureUniquePath(withoutExt, extension, (num += 1))
return ensureSafePath(withoutExt, extension, num + 1)
}

return fullPath
// path does not exist, attempt to create it to check for an ENAMETOOLONG error
return fs.outputFileAsync(fullPath, '')
.then(() => fullPath)
.catch((err) => {
debug('received error when testing path %o', { err, fullPath, maxSafePrefixBytes, maxSafeBytes })

if (err.code === 'ENAMETOOLONG' && maxSafePrefixBytes >= MIN_PREFIX_BYTES) {
maxSafeBytes -= 1

return ensureSafePath(withoutExt, extension, num)
}

throw err
})
})
}

Expand All @@ -323,26 +356,16 @@ const getPath = function (data, ext, screenshotsFolder) {
.value()
}

// truncate file names to be less than 220 characters
// to accomodate filename size limits
const maxFileNameLength = 220
const index = names.length - 1

if (names[index].length > maxFileNameLength) {
names[index] = _.truncate(names[index], {
length: maxFileNameLength,
omission: '',
})
}

// append (failed) to the last name
if (data.testFailure) {
names[index] = `${names[index]} (failed)`
}

const withoutExt = path.join(screenshotsFolder, ...specNames, ...names)

return ensureUniquePath(withoutExt, ext)
return ensureSafePath(withoutExt, ext)
}

const getPathToScreenshot = function (data, details, screenshotsFolder) {
Expand Down
2 changes: 2 additions & 0 deletions packages/server/test/support/helpers/e2e.ts
Expand Up @@ -697,6 +697,8 @@ const e2e = {
LINES: 24,
})
.defaults({
// match CircleCI's filesystem limits, so screenshot names in snapshots match
CYPRESS_MAX_SAFE_FILENAME_BYTES: 242,
FAKE_CWD_PATH: '/XXX/XXX/XXX',
DEBUG_COLORS: '1',
// prevent any Compression progress
Expand Down
52 changes: 51 additions & 1 deletion packages/server/test/unit/screenshots_spec.js
Expand Up @@ -577,6 +577,10 @@ describe('lib/screenshots', () => {
})

context('.getPath', () => {
beforeEach(() => {
sinon.stub(fs, 'outputFileAsync').resolves()
})

it('concats spec name, screenshotsFolder, and name', () => {
return screenshots.getPath({
specName: 'examples/user/list.js',
Expand Down Expand Up @@ -618,6 +622,52 @@ describe('lib/screenshots', () => {
})
})

// @see https://github.com/cypress-io/cypress/issues/2403
it('truncates long paths with unicode in them', async () => {
const fullPath = await screenshots.getPath({
titles: [
'WMED: [STORY] Тестовые сценарии для CI',
'Сценарии:',
'Сценарий 2: Создание обращения, создание медзаписи, привязкапривязка обращения к медзаписи',
'- Сценарий 2',
],
testFailure: true,
specName: 'WMED_UAT_Scenarios_For_CI_spec.js',
}, 'png', '/jenkins-slave/workspace/test-wmed/qa/cypress/wmed_ci/cypress/screenshots/')

const basename = path.basename(fullPath)

expect(Buffer.from(basename).byteLength).to.be.lessThan(255)
})

it('reacts to ENAMETOOLONG errors and tries to shorten the filename', async () => {
const err = new Error('enametoolong')

err.code = 'ENAMETOOLONG'

_.times(50, (i) => fs.outputFileAsync.onCall(i).rejects(err))

const fullPath = await screenshots.getPath({
specName: 'foo.js',
name: 'a'.repeat(256),
}, 'png', '/tmp')

expect(path.basename(fullPath)).to.have.length(204)
})

it('rejects with ENAMETOOLONG errors if name goes below MIN_PREFIX_LENGTH', async () => {
const err = new Error('enametoolong')

err.code = 'ENAMETOOLONG'

_.times(150, (i) => fs.outputFileAsync.onCall(i).rejects(err))

await expect(screenshots.getPath({
specName: 'foo.js',
name: 'a'.repeat(256),
}, 'png', '/tmp')).to.be.rejectedWith(err)
})

_.each([Infinity, 0 / 0, [], {}, 1, false], (value) => {
it(`doesn't err and stringifies non-string test title: ${value}`, () => {
return screenshots.getPath({
Expand All @@ -632,7 +682,7 @@ describe('lib/screenshots', () => {
})
})

return _.each([null, undefined], (value) => {
_.each([null, undefined], (value) => {
it(`doesn't err and removes null/undefined test title: ${value}`, () => {
return screenshots.getPath({
specName: 'examples$/user/list.js',
Expand Down