From cc6dcf67b72b7ffb8892310906fbb13825f5690f Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Wed, 6 Sep 2017 18:20:42 -0700 Subject: [PATCH] Export: Limit attachment filename length, + convo date, + tests (#1439) * Export: limit attachment names to 30 chars, tests for helper fns Also, reintroduce last contact date in conversation dir name FREEBIE * MessageView tests: Fix failures during blanket coverage run FREEBIE --- js/backup.js | 42 ++++++++-- test/backup_test.js | 134 ++++++++++++++++++++++++++++++++ test/index.html | 2 + test/views/message_view_test.js | 20 ++--- 4 files changed, 184 insertions(+), 14 deletions(-) create mode 100644 test/backup_test.js diff --git a/js/backup.js b/js/backup.js index 320894806..b45bae218 100644 --- a/js/backup.js +++ b/js/backup.js @@ -267,8 +267,35 @@ }); } + function trimFileName(filename) { + var components = filename.split('.'); + if (components.length <= 1) { + return filename.slice(0, 30); + } + + var extension = components[components.length - 1]; + var name = components.slice(0, components.length - 1); + if (extension.length > 5) { + return filename.slice(0, 30); + } + + return name.join('.').slice(0, 24) + '.' + extension; + } + + function getAttachmentFileName(attachment) { - return attachment.fileName || (attachment.id + '.' + attachment.contentType.split('/')[1]); + if (attachment.fileName) { + return trimFileName(attachment.fileName); + } + + var name = attachment.id; + + if (attachment.contentType) { + var components = attachment.contentType.split('/'); + name += '.' + (components.length > 1 ? components[1] : attachment.contentType); + } + + return name; } function readAttachment(parent, message, attachment) { @@ -407,11 +434,10 @@ function getConversationDirName(conversation) { var name = conversation.active_at || 'never'; if (conversation.name) { - return ' (' + conversation.name.slice(0, 30) + ' ' + conversation.id + ')'; + return name + ' (' + conversation.name.slice(0, 30) + ' ' + conversation.id + ')'; } else { - return ' (' + conversation.id + ')'; + return name + ' (' + conversation.id + ')'; } - return name; } // Goals for logging names: @@ -673,7 +699,13 @@ ); return Promise.reject(error); }); - } + }, + // for testing + sanitizeFileName: sanitizeFileName, + trimFileName: trimFileName, + getAttachmentFileName: getAttachmentFileName, + getConversationDirName: getConversationDirName, + getConversationLoggingName: getConversationLoggingName }; }()); diff --git a/test/backup_test.js b/test/backup_test.js new file mode 100644 index 000000000..fadae90bd --- /dev/null +++ b/test/backup_test.js @@ -0,0 +1,134 @@ +'use strict'; + +describe('Backup', function() { + describe('sanitizeFileName', function() { + it('leaves a basic string alone', function() { + var initial = 'Hello, how are you #5 (\'fine\' + great).jpg'; + var expected = initial; + assert.strictEqual(Whisper.Backup.sanitizeFileName(initial), expected); + }); + + it('replaces all unknown characters', function() { + var initial = '!@$%^&*='; + var expected = '________'; + assert.strictEqual(Whisper.Backup.sanitizeFileName(initial), expected); + }); + }); + + describe('trimFileName', function() { + it('handles a file with no extension', function() { + var initial = '0123456789012345678901234567890123456789'; + var expected = '012345678901234567890123456789'; + assert.strictEqual(Whisper.Backup.trimFileName(initial), expected); + }); + + it('handles a file with a long extension', function() { + var initial = '0123456789012345678901234567890123456789.01234567890123456789'; + var expected = '012345678901234567890123456789'; + assert.strictEqual(Whisper.Backup.trimFileName(initial), expected); + }); + + it('handles a file with a normal extension', function() { + var initial = '01234567890123456789012345678901234567890123456789.jpg'; + var expected = '012345678901234567890123.jpg'; + assert.strictEqual(Whisper.Backup.trimFileName(initial), expected); + }); + }); + + describe('getAttachmentFileName', function() { + it('uses original filename if attachment has one', function() { + var attachment = { + fileName: 'blah.jpg' + }; + var expected = 'blah.jpg'; + assert.strictEqual(Whisper.Backup.getAttachmentFileName(attachment), expected); + }); + + it('uses attachment id if no filename', function() { + var attachment = { + id: '123' + }; + var expected = '123'; + assert.strictEqual(Whisper.Backup.getAttachmentFileName(attachment), expected); + }); + + it('uses filename and contentType if available', function() { + var attachment = { + id: '123', + contentType: 'image/jpeg' + }; + var expected = '123.jpeg'; + assert.strictEqual(Whisper.Backup.getAttachmentFileName(attachment), expected); + }); + + it('handles strange contentType', function() { + var attachment = { + id: '123', + contentType: 'something' + }; + var expected = '123.something'; + assert.strictEqual(Whisper.Backup.getAttachmentFileName(attachment), expected); + }); + }); + + describe('getConversationDirName', function() { + it('uses name if available', function() { + var conversation = { + active_at: 123, + name: '0123456789012345678901234567890123456789', + id: 'id' + }; + var expected = '123 (012345678901234567890123456789 id)'; + assert.strictEqual(Whisper.Backup.getConversationDirName(conversation), expected); + }); + + it('uses just id if name is not available', function() { + var conversation = { + active_at: 123, + id: 'id' + }; + var expected = '123 (id)'; + assert.strictEqual(Whisper.Backup.getConversationDirName(conversation), expected); + }); + + it('uses never for missing active_at', function() { + var conversation = { + name: 'name', + id: 'id' + }; + var expected = 'never (name id)'; + assert.strictEqual(Whisper.Backup.getConversationDirName(conversation), expected); + }); + }); + + describe('getConversationLoggingName', function() { + it('uses plain id if conversation is private', function() { + var conversation = { + active_at: 123, + id: 'id', + type: 'private' + }; + var expected = '123 (id)'; + assert.strictEqual(Whisper.Backup.getConversationLoggingName(conversation), expected); + }); + + it('uses just id if name is not available', function() { + var conversation = { + active_at: 123, + id: 'groupId', + type: 'group' + }; + var expected = '123 ([REDACTED_GROUP]pId)'; + assert.strictEqual(Whisper.Backup.getConversationLoggingName(conversation), expected); + }); + + it('uses never for missing active_at', function() { + var conversation = { + id: 'id', + type: 'private' + }; + var expected = 'never (id)'; + assert.strictEqual(Whisper.Backup.getConversationLoggingName(conversation), expected); + }); + }); +}); diff --git a/test/index.html b/test/index.html index a5804a950..b4478345b 100644 --- a/test/index.html +++ b/test/index.html @@ -580,6 +580,7 @@ + @@ -648,6 +649,7 @@ + diff --git a/test/views/message_view_test.js b/test/views/message_view_test.js index a9c680603..514ccde00 100644 --- a/test/views/message_view_test.js +++ b/test/views/message_view_test.js @@ -1,15 +1,17 @@ describe('MessageView', function() { + var convo, message; + before(function() { - return storage.put('number_id', '+18088888888.1'); - }); + convo = ConversationController.createTemporary({id: 'foo'}); + message = convo.messageCollection.add({ + conversationId: convo.id, + body: 'hello world', + type: 'outgoing', + source: '+14158675309', + received_at: Date.now() + }); - var convo = ConversationController.createTemporary({id: 'foo'}); - var message = convo.messageCollection.add({ - conversationId: convo.id, - body: 'hello world', - type: 'outgoing', - source: '+14158675309', - received_at: Date.now() + return storage.put('number_id', '+18088888888.1'); }); it('should display the message text', function() {