From 9d342e8951ea7848063e9f89d7816fa33ec5f9cc Mon Sep 17 00:00:00 2001 From: Mikunj Date: Wed, 7 Nov 2018 11:41:46 +1100 Subject: [PATCH 1/3] Show a message if user types in an invalid public key in search. --- js/models/conversations.js | 16 +++++++++++----- js/views/conversation_search_view.js | 24 ++++++++++-------------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/js/models/conversations.js b/js/models/conversations.js index c618baa5e..4135aa2b3 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -672,14 +672,19 @@ }, validateNumber() { + if (!this.id) return null; + if (this.isPrivate()) { - if (this.id.length === 33 * 2) { + // Check if it's hex + const isHex = this.id.replace(/[\s]*/g, '').match(/^[0-9a-fA-F]+$/); + if (!isHex) return 'Invalid Hex ID'; + + // Check if it has a valid length + if (this.id.length !== 33 * 2) { // 33 bytes in hex this.set({ id: this.id }); - return null; + return 'Invalid ID Length'; } - - return 'Invalid ID'; } return null; @@ -1477,9 +1482,10 @@ if (avatar && avatar.path) { return { url: getAbsoluteAttachmentPath(avatar.path), color }; } else if (this.isPrivate()) { + const symbol = this.isValid() ? '#' : '!'; return { color, - content: title ? title.trim()[0] : '#', + content: title ? title.trim()[0] : symbol, }; } return { url: 'images/group_default.png', color }; diff --git a/js/views/conversation_search_view.js b/js/views/conversation_search_view.js index 774500752..f1cdc981a 100644 --- a/js/views/conversation_search_view.js +++ b/js/views/conversation_search_view.js @@ -20,8 +20,10 @@ this.listenTo(this.model, 'change', this.render); // auto update }, render_attributes() { + // Show the appropriate message based on model validity + const message = this.model && this.model.isValid() ? i18n('startConversation') : i18n('invalidNumberError'); return { - number: i18n('startConversation'), + number: message, title: this.model.getNumber(), avatar: this.model.getAvatar(), }; @@ -68,14 +70,13 @@ filterContacts() { const query = this.$input.val().trim(); if (query.length) { - if (this.maybeNumber(query)) { - this.new_contact_view.model.set('id', query); - this.new_contact_view.render().$el.show(); - this.new_contact_view.validate(); - this.hideHints(); - } else { - this.new_contact_view.$el.hide(); - } + + // Update the contact model + this.new_contact_view.model.set('id', query); + this.new_contact_view.render().$el.show(); + this.new_contact_view.validate(); + this.hideHints(); + // NOTE: Temporarily allow `then` until we convert the entire file // to `async` / `await`: /* eslint-disable more/no-then */ @@ -108,7 +109,6 @@ async createConversation() { const isValidNumber = this.new_contact_view.model.isValid(); if (!isValidNumber) { - this.new_contact_view.$('.number').text(i18n('invalidNumberError')); this.$input.focus(); return; } @@ -155,9 +155,5 @@ this.hintView = null; } }, - - maybeNumber(number) { - return number.replace(/[\s]*/g, '').match(/^[0-9a-fA-F]+$/); // hex representation - }, }); })(); From 4e6df71999f0600de223e5d2c59fcc87cb952e96 Mon Sep 17 00:00:00 2001 From: Mikunj Date: Wed, 7 Nov 2018 12:15:22 +1100 Subject: [PATCH 2/3] Fix up tests. --- js/models/conversations.js | 18 +---- test/models/conversations_test.js | 83 ++++++++------------- test/views/conversation_search_view_test.js | 24 ------ 3 files changed, 32 insertions(+), 93 deletions(-) diff --git a/js/models/conversations.js b/js/models/conversations.js index 4135aa2b3..a8d65a387 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -1434,23 +1434,7 @@ if (!this.isPrivate()) { return ''; } - const number = this.id; - try { - const parsedNumber = libphonenumber.parse(number); - const regionCode = libphonenumber.getRegionCodeForNumber(parsedNumber); - if (regionCode === storage.get('regionCode')) { - return libphonenumber.format( - parsedNumber, - libphonenumber.PhoneNumberFormat.NATIONAL - ); - } - return libphonenumber.format( - parsedNumber, - libphonenumber.PhoneNumberFormat.INTERNATIONAL - ); - } catch (e) { - return number; - } + return this.id; }, isPrivate() { diff --git a/test/models/conversations_test.js b/test/models/conversations_test.js index 1b27ae032..63279e823 100644 --- a/test/models/conversations_test.js +++ b/test/models/conversations_test.js @@ -38,7 +38,7 @@ }); describe('Conversation', function() { - var attributes = { type: 'private', id: '+18085555555' }; + var attributes = { type: 'private', id: '771d11d01e56d9bfc3d74115c33225a632321b509ac17a13fdeac71165d09b94ab' }; before(async () => { var convo = new Whisper.ConversationCollection().add(attributes); await window.Signal.Data.saveConversation(convo.attributes, { @@ -59,7 +59,7 @@ after(clearDatabase); it('sorts its contacts in an intl-friendly way', function() { - var convo = new Whisper.Conversation({ id: '+18085555555' }); + var convo = new Whisper.Conversation({ id: '771d11d01e56d9bfc3d74115c33225a632321b509ac17a13fdeac71165d09b94ab' }); convo.contactCollection.add( new Whisper.Conversation({ name: 'C', @@ -83,7 +83,7 @@ it('contains its own messages', async function() { var convo = new Whisper.ConversationCollection().add({ - id: '+18085555555', + id: '771d11d01e56d9bfc3d74115c33225a632321b509ac17a13fdeac71165d09b94ab', }); await convo.fetchMessages(); assert.notEqual(convo.messageCollection.length, 0); @@ -91,7 +91,7 @@ it('contains only its own messages', async function() { var convo = new Whisper.ConversationCollection().add({ - id: '+18085556666', + id: '6eb56f06737d0966239e70d431d4dfd9e57c1e7dddacaf61907fcbc14295e424fd', }); await convo.fetchMessages(); assert.strictEqual(convo.messageCollection.length, 0); @@ -109,7 +109,7 @@ it('has a title', function() { var convos = new Whisper.ConversationCollection(); var convo = convos.add(attributes); - assert.equal(convo.getTitle(), '+1 808-555-5555'); + assert.equal(convo.getTitle(), '771d11d01e56d9bfc3d74115c33225a632321b509ac17a13fdeac71165d09b94ab'); convo = convos.add({ type: '' }); assert.equal(convo.getTitle(), 'Unknown group'); @@ -121,7 +121,7 @@ it('returns the number', function() { var convos = new Whisper.ConversationCollection(); var convo = convos.add(attributes); - assert.equal(convo.getNumber(), '+1 808-555-5555'); + assert.equal(convo.getNumber(), '771d11d01e56d9bfc3d74115c33225a632321b509ac17a13fdeac71165d09b94ab'); convo = convos.add({ type: '' }); assert.equal(convo.getNumber(), ''); @@ -134,41 +134,21 @@ assert.property(avatar, 'color'); }); - describe('phone number parsing', function() { - after(function() { - storage.remove('regionCode'); + describe('when set to private', function() { + it('correctly validates hex numbers', function() { + const regularId = new Whisper.Conversation({ type: 'private', id: '771d11d01e56d9bfc3d74115c33225a632321b509ac17a13fdeac71165d09b94ab' }); + const invalidId = new Whisper.Conversation({ type: 'private', id: 'j71d11d01e56d9bfc3d74115c33225a632321b509ac17a13fdeac71165d09b94ab' }); + assert.ok(regularId.isValid()); + assert.notOk(invalidId.isValid()); }); - function checkAttributes(number) { - var convo = new Whisper.ConversationCollection().add({ - type: 'private', - }); - convo.set('id', number); - convo.validate(convo.attributes); - assert.strictEqual(convo.get('id'), '+14155555555', number); - } - it('processes the phone number when validating', function() { - ['+14155555555'].forEach(checkAttributes); - }); - it('defaults to the local regionCode', function() { - storage.put('regionCode', 'US'); - ['14155555555', '4155555555'].forEach(checkAttributes); - }); - it('works with common phone number formats', function() { - storage.put('regionCode', 'US'); - [ - '415 555 5555', - '415-555-5555', - '(415) 555 5555', - '(415) 555-5555', - '1 415 555 5555', - '1 415-555-5555', - '1 (415) 555 5555', - '1 (415) 555-5555', - '+1 415 555 5555', - '+1 415-555-5555', - '+1 (415) 555 5555', - '+1 (415) 555-5555', - ].forEach(checkAttributes); + + it('correctly validates length', function() { + const regularId = new Whisper.Conversation({ type: 'private', id: '771d11d01e56d9bfc3d74115c33225a632321b509ac17a13fdeac71165d09b94ab' }); + const shortId = new Whisper.Conversation({ type: 'private', id: '771d11d' }); + const longId = new Whisper.Conversation({ type: 'private', id: '771d11d01e56d9bfc3d74115c33225a632321b509ac17a13fdeac71165d09b94abaa' }); + assert.ok(regularId.isValid()); + assert.notOk(shortId.isValid()); + assert.notOk(longId.isValid()); }); }); }); @@ -178,7 +158,7 @@ beforeEach(async function() { convo = new Whisper.ConversationCollection().add({ - id: '+14155555555', + id: '771d11d01e56d9bfc3d74115c33225a632321b509ac17a13fdeac71165d09b94ab', type: 'private', name: 'John Doe', }); @@ -202,21 +182,20 @@ }) ); } - it('matches by partial phone number', function() { + it('matches by partial keys', function() { return testSearch([ '1', - '4', - '+1', - '415', - '4155', - '4155555555', - '14155555555', - '+14155555555', + '771', + '1e', + '56d9bfc3d74115c3322', + '6d9bfc3d74115c33225a632321b509ac17a13fdeac71165d', + '771d11d01e56d9bfc3d74115c33225a632321b509ac17a13fdeac71165d09b94ab' ]); }); - it('matches by name', function() { - return testSearch(['John', 'Doe', 'john', 'doe', 'John Doe', 'john doe']); - }); + // TODO: Re-enable once we have nickanme functionality + // it('matches by name', function() { + // return testSearch(['John', 'Doe', 'john', 'doe', 'John Doe', 'john doe']); + // }); it('does not match +', async function() { var collection = new Whisper.ConversationCollection(); await collection.search('+'); diff --git a/test/views/conversation_search_view_test.js b/test/views/conversation_search_view_test.js index 5cd3f2c32..aa7e353e8 100644 --- a/test/views/conversation_search_view_test.js +++ b/test/views/conversation_search_view_test.js @@ -1,28 +1,4 @@ describe('ConversationSearchView', function() { - it('should match partial numbers', function() { - var $el = $('
'); - var view = new Whisper.ConversationSearchView({ - el: $el, - input: $(''), - }).render(); - var maybe_numbers = [ - '+1 415', - '+1415', - '+1415', - '415', - '(415)', - ' (415', - '(415) 123 4567', - '+1 (415) 123 4567', - ' +1 (415) 123 4567', - '1 (415) 123 4567', - '1 415-123-4567', - '415-123-4567', - ]; - maybe_numbers.forEach(function(n) { - assert.ok(view.maybeNumber(n), n); - }); - }); describe('Searching for left groups', function() { let convo; From 9ea44a5cd2a6fa0fb574e2415cc5395e2747f42c Mon Sep 17 00:00:00 2001 From: Mikunj Date: Wed, 7 Nov 2018 13:11:03 +1100 Subject: [PATCH 3/3] fix incorrect error return. --- js/models/conversations.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/models/conversations.js b/js/models/conversations.js index a8d65a387..d3abe1952 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -672,7 +672,7 @@ }, validateNumber() { - if (!this.id) return null; + if (!this.id) return 'Invalid ID'; if (this.isPrivate()) { // Check if it's hex