Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions src/messaging/messaging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
35 changes: 28 additions & 7 deletions test/unit/messaging/messaging.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)}`, () => {
Expand All @@ -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());
Expand Down Expand Up @@ -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()
Expand Down