Accept prefixed topic names (#203)

hiranya911 · web-flow · commit e9ea3ab88db0 · 2018-02-07T16:15:31.000-08:00
* Accept prefixed topic names

* Further validating topic names
diff --git a/src/messaging/messaging.ts b/src/messaging/messaging.ts
@@ -429,17 +429,24 @@ function validateMessage(message: Message) {
   }
 
   const anyMessage = message as any;
+  if (anyMessage.topic) {
+    // If the topic name is prefixed, remove it.
+    if (anyMessage.topic.startsWith('/topics/')) {
+      anyMessage.topic = anyMessage.topic.replace(/^\/topics\//, '');
+    }
+    // Checks for illegal characters and empty string.
+    if (!/^[a-zA-Z0-9-_.~%]+$/.test(anyMessage.topic)) {
+      throw new FirebaseMessagingError(
+        MessagingClientErrorCode.INVALID_PAYLOAD, 'Malformed topic name');
+    }
+  }
+
   const targets = [anyMessage.token, anyMessage.topic, anyMessage.condition];
   if (targets.filter((v) => validator.isNonEmptyString(v)).length !== 1) {
     throw new FirebaseMessagingError(
       MessagingClientErrorCode.INVALID_PAYLOAD,
       'Exactly one of topic, token or condition is required');
   }
-  if (anyMessage.topic && anyMessage.topic.startsWith('/topics/')) {
-    throw new FirebaseMessagingError(
-      MessagingClientErrorCode.INVALID_PAYLOAD,
-      'Topic name must be specified without the "/topics/" prefix');
-  }
 
   validateStringMap(message.data, 'data');
   validateAndroidConfig(message.android);
diff --git a/test/unit/messaging/messaging.spec.ts b/test/unit/messaging/messaging.spec.ts
@@ -365,12 +365,6 @@ describe('Messaging', () => {
       });
     });
 
-    it('should throw given message with invalid topic name', () => {
-      expect(() => {
-        messaging.send({topic: '/topics/foo'});
-      }).to.throw('Topic name must be specified without the "/topics/" prefix');
-    });
-
     const invalidDryRun = [null, NaN, 0, 1, '', 'a', [], [1, 'a'], {}, { a: 1 }, _.noop];
     invalidDryRun.forEach((dryRun) => {
       it(`should throw given invalid dryRun parameter: ${JSON.stringify(dryRun)}`, () => {
@@ -380,7 +374,19 @@ describe('Messaging', () => {
       });
     });
 
-    const targetMessages = [{token: 'mock-token'}, {topic: 'mock-topic'}, {condition: '"foo" in topics'}];
+    const invalidTopics = ['/topics/', '/foo/bar', 'foo bar'];
+    invalidTopics.forEach((topic) => {
+      it(`should throw given invalid topic name: ${JSON.stringify(topic)}`, () => {
+        expect(() => {
+          messaging.send({topic});
+        }).to.throw('Malformed topic name');
+      });
+    });
+
+    const targetMessages = [
+      {token: 'mock-token'}, {topic: 'mock-topic'},
+      {topic: '/topics/mock-topic'}, {condition: '"foo" in topics'},
+    ];
     targetMessages.forEach((message) => {
       it(`should be fulfilled with a message ID given a valid message: ${JSON.stringify(message)}`, () => {
         mockedRequests.push(mockSendRequest());
@@ -2308,6 +2314,21 @@ describe('Messaging', () => {
       });
     });
 
+    it('should not throw when the message is addressed to the prefixed topic name', () => {
+      return mockApp.INTERNAL.getToken()
+        .then(() => {
+          httpsRequestStub = sinon.stub(https, 'request');
+          httpsRequestStub.callsArgWith(1, mockResponse).returns(mockRequestStream);
+          return messaging.send({topic: '/topics/mock-topic'});
+        })
+        .then(() => {
+          expect(requestWriteSpy).to.have.been.calledOnce;
+          const requestData = JSON.parse(requestWriteSpy.args[0][0]);
+          const expectedReq = {topic: 'mock-topic'};
+          expect(requestData.message).to.deep.equal(expectedReq);
+        });
+    });
+
     it('should convert whitelisted camelCased properties to underscore_cased properties', () => {
       // Wait for the initial getToken() call to complete before stubbing https.request.
       return mockApp.INTERNAL.getToken()
