fix: truncate screenshot filenames using byteLength

cypress-io · Aug 4, 2020 · 0fdf4f6 · 0fdf4f6
1 parent 996fe97
commit 0fdf4f6
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 15 deletions.
diff --git a/packages/server/__snapshots__/5_screenshots_spec.js b/packages/server/__snapshots__/5_screenshots_spec.js
@@ -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                                                          
+     aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.png                           
   -  /XXX/XXX/XXX/cypress/screenshots/screenshots_spec.js/taking screenshots -- reall     (1000x660)
      y long test title aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa               
      aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa               
-     aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa (1).png                                                      
+     aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa (1).png                           
 
 
   (Video)

diff --git a/packages/server/lib/screenshots.js b/packages/server/lib/screenshots.js
@@ -289,12 +289,26 @@ const getDimensions = function (details) {
 }
 
 const ensureUniquePath = function (withoutExt, extension, num = 0) {
-  const fullPath = num ? `${withoutExt} (${num}).${extension}` : `${withoutExt}.${extension}`
+  const suffix = `${num ? ` (${num})` : ''}.${extension}`
+  // 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
+  // @see https://github.com/cypress-io/cypress/issues/2403
+  // @see https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits
+  const maxSafeBytes = 255 - suffix.length
+  const filenameBuf = Buffer.from(path.basename(withoutExt))
+
+  if (filenameBuf.byteLength > maxSafeBytes) {
+    const truncated = filenameBuf.slice(0, maxSafeBytes).toString()
+
+    withoutExt = path.join(path.dirname(withoutExt), truncated)
+  }
+
+  const fullPath = [withoutExt, suffix].join('')
 
   return fs.pathExists(fullPath)
   .then((found) => {
     if (found) {
-      return ensureUniquePath(withoutExt, extension, (num += 1))
+      return ensureUniquePath(withoutExt, extension, num + 1)
     }
 
     return fullPath
@@ -323,18 +337,8 @@ 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)`

diff --git a/packages/server/test/unit/screenshots_spec.js b/packages/server/test/unit/screenshots_spec.js
@@ -618,6 +618,24 @@ 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)
+    })
+
     _.each([Infinity, 0 / 0, [], {}, 1, false], (value) => {
       it(`doesn't err and stringifies non-string test title: ${value}`, () => {
         return screenshots.getPath({
@@ -632,7 +650,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',