From 483d3bece173f06d913cafd00dc4e9af4ff74417 Mon Sep 17 00:00:00 2001 From: Italo Izaac Date: Wed, 20 Jan 2021 05:53:16 -0300 Subject: [PATCH 1/2] fix(messaging): Fix error throw when data property exists but is undefined value --- src/messaging/messaging.ts | 2 +- test/unit/messaging/messaging.spec.ts | 94 +++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/src/messaging/messaging.ts b/src/messaging/messaging.ts index 88e66cf565..9bcf56a166 100644 --- a/src/messaging/messaging.ts +++ b/src/messaging/messaging.ts @@ -857,7 +857,7 @@ export class Messaging implements MessagingInterface { } // Validate the data payload object does not contain blacklisted properties - if ('data' in payloadCopy) { + if ('data' in payloadCopy && payloadCopy.data !== undefined) { BLACKLISTED_DATA_PAYLOAD_KEYS.forEach((blacklistedKey) => { if (blacklistedKey in payloadCopy.data!) { throw new FirebaseMessagingError( diff --git a/test/unit/messaging/messaging.spec.ts b/test/unit/messaging/messaging.spec.ts index 82f0973f67..5485b5f7b3 100644 --- a/test/unit/messaging/messaging.spec.ts +++ b/test/unit/messaging/messaging.spec.ts @@ -566,6 +566,12 @@ describe('Messaging', () => { { token: 'mock-token' }, ).should.eventually.be.rejected.and.have.property('code', 'messaging/invalid-argument'); }); + + it('should not throws when the payload contains the data property with undefined value', () => { + mockedRequests.push(mockSendRequest()); + const message = { token: 'a', data: undefined }; + return messaging.send(message).should.eventually.equal('projects/projec_id/messages/message_id'); + }) }); describe('sendAll()', () => { @@ -823,6 +829,26 @@ describe('Messaging', () => { [validMessage], ).should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); }); + + it('should not throws when the payload contains the data property with undefined value', () => { + const messageIds = [ + 'projects/projec_id/messages/1', + 'projects/projec_id/messages/2', + 'projects/projec_id/messages/3', + ]; + mockedRequests.push(mockBatchRequest(messageIds)); + const validMessageWithUndefinedDataProp = { ...validMessage, data: undefined } + return messaging.sendAll([validMessageWithUndefinedDataProp, validMessageWithUndefinedDataProp, validMessageWithUndefinedDataProp]) + .then((response: BatchResponse) => { + expect(response.successCount).to.equal(3); + expect(response.failureCount).to.equal(0); + response.responses.forEach((resp, idx) => { + expect(resp.success).to.be.true; + expect(resp.messageId).to.equal(messageIds[idx]); + expect(resp.error).to.be.undefined; + }); + }); + }) }); describe('sendMulticast()', () => { @@ -1118,6 +1144,31 @@ describe('Messaging', () => { ).should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); }); + it('should not throws when the payload contains the data property with undefined value', () => { + const messageIds = [ + 'projects/projec_id/messages/1', + 'projects/projec_id/messages/2', + 'projects/projec_id/messages/3', + ]; + mockedRequests.push(mockBatchRequest(messageIds)); + return messaging.sendMulticast({ + tokens: ['a', 'b', 'c'], + android: { ttl: 100 }, + apns: { payload: { aps: { badge: 42 } } }, + data: undefined, + notification: { title: 'test title' }, + webpush: { data: { webKey: 'webValue' } }, + }).then((response: BatchResponse) => { + expect(response.successCount).to.equal(3); + expect(response.failureCount).to.equal(0); + response.responses.forEach((resp, idx) => { + expect(resp.success).to.be.true; + expect(resp.messageId).to.equal(messageIds[idx]); + expect(resp.error).to.be.undefined; + }); + }); + }) + function checkSendResponseSuccess(response: SendResponse, messageId: string): void { expect(response.success).to.be.true; expect(response.messageId).to.equal(messageId); @@ -1457,6 +1508,16 @@ describe('Messaging', () => { expect(mockOptionsClone).to.deep.equal(mocks.messaging.options); }); }); + + it('should not throws when the payload contains the data property with undefined value', () => { + mockedRequests.push(mockSendToDeviceStringRequest()); + + return messaging.sendToDevice( + mocks.messaging.registrationToken, + { ...mocks.messaging.payload, data: undefined }, + mocks.messaging.options, + ); + }) }); describe('sendToDeviceGroup()', () => { @@ -1708,6 +1769,15 @@ describe('Messaging', () => { expect(mockOptionsClone).to.deep.equal(mocks.messaging.options); }); }); + + it('should not throws when the payload contains the data property with undefined value', () => { + mockedRequests.push(mockSendToDeviceGroupRequest()); + + return messaging.sendToDeviceGroup( + mocks.messaging.notificationKey, + { ...mocks.messaging.payloadDataOnly, data: undefined }, + ); + }); }); describe('sendToTopic()', () => { @@ -1935,6 +2005,15 @@ describe('Messaging', () => { expect(mockOptionsClone).to.deep.equal(mocks.messaging.options); }); }); + + it('should not throws when the payload contains the data property with undefined value', () => { + mockedRequests.push(mockSendToTopicRequest()); + + return messaging.sendToTopic( + mocks.messaging.topic, + { ...mocks.messaging.payload, data: undefined }, + ); + }); }); describe('sendToCondition()', () => { @@ -2116,6 +2195,15 @@ describe('Messaging', () => { expect(mockOptionsClone).to.deep.equal(mocks.messaging.options); }); }); + + it('should not throws when the payload contains the data property with undefined value', () => { + mockedRequests.push(mockSendToConditionRequest()); + + return messaging.sendToCondition( + mocks.messaging.condition, + { ...mocks.messaging.payloadDataOnly, data: undefined }, + ); + }); }); describe('Payload validation', () => { @@ -2341,6 +2429,12 @@ describe('Messaging', () => { }); }); + it('should not throws when the payload contains the data property with undefined value', () => { + mockedRequests.push(mockSendToDeviceStringRequest()); + + return messaging.sendToDevice(mocks.messaging.registrationToken, { ...mocks.messaging.payloadDataOnly, data: undefined }); + }); + const invalidImages = ['', 'a', 'foo', 'image.jpg']; invalidImages.forEach((imageUrl) => { it(`should throw given an invalid imageUrl: ${imageUrl}`, () => { From 97e2f9d25bb069aa03790496e3da84789ba2b673 Mon Sep 17 00:00:00 2001 From: Italo Izaac Date: Wed, 20 Jan 2021 07:15:00 -0300 Subject: [PATCH 2/2] chore: fix the code standard --- test/unit/messaging/messaging.spec.ts | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/test/unit/messaging/messaging.spec.ts b/test/unit/messaging/messaging.spec.ts index 5485b5f7b3..e7978e669a 100644 --- a/test/unit/messaging/messaging.spec.ts +++ b/test/unit/messaging/messaging.spec.ts @@ -838,16 +838,19 @@ describe('Messaging', () => { ]; mockedRequests.push(mockBatchRequest(messageIds)); const validMessageWithUndefinedDataProp = { ...validMessage, data: undefined } - return messaging.sendAll([validMessageWithUndefinedDataProp, validMessageWithUndefinedDataProp, validMessageWithUndefinedDataProp]) - .then((response: BatchResponse) => { - expect(response.successCount).to.equal(3); - expect(response.failureCount).to.equal(0); - response.responses.forEach((resp, idx) => { - expect(resp.success).to.be.true; - expect(resp.messageId).to.equal(messageIds[idx]); - expect(resp.error).to.be.undefined; - }); + return messaging.sendAll([ + validMessageWithUndefinedDataProp, + validMessageWithUndefinedDataProp, + validMessageWithUndefinedDataProp + ]).then((response: BatchResponse) => { + expect(response.successCount).to.equal(3); + expect(response.failureCount).to.equal(0); + response.responses.forEach((resp, idx) => { + expect(resp.success).to.be.true; + expect(resp.messageId).to.equal(messageIds[idx]); + expect(resp.error).to.be.undefined; }); + }); }) }); @@ -2432,7 +2435,10 @@ describe('Messaging', () => { it('should not throws when the payload contains the data property with undefined value', () => { mockedRequests.push(mockSendToDeviceStringRequest()); - return messaging.sendToDevice(mocks.messaging.registrationToken, { ...mocks.messaging.payloadDataOnly, data: undefined }); + return messaging.sendToDevice(mocks.messaging.registrationToken, { + ...mocks.messaging.payloadDataOnly, + data: undefined, + }); }); const invalidImages = ['', 'a', 'foo', 'image.jpg'];