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
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# Unreleased

-
- [fixed] Gracefully handling array-like objects in `messaging.sendAll()` and
`messaging.sendMulticast()` APIs.
- [fixed] Updated the metadata server URL (used by the application default credentials)
to the `v1` endpoint.

# v8.1.0

Expand Down
7 changes: 7 additions & 0 deletions src/messaging/messaging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,13 @@ export class Messaging implements FirebaseServiceInterface {
* of the send operation.
*/
public sendAll(messages: Message[], dryRun?: boolean): Promise<BatchResponse> {
if (validator.isArray(messages) && messages.constructor !== Array) {
// In more recent JS specs, an array-like object might have a constructor that is not of
// Array type. Our deepCopy() method doesn't handle them properly. Convert such objects to
// a regular array here before calling deepCopy(). See issue #566 for details.
messages = Array.from(messages);
}

const copy: Message[] = deepCopy(messages);
if (!validator.isNonEmptyArray(copy)) {
throw new FirebaseMessagingError(
Expand Down
36 changes: 36 additions & 0 deletions test/unit/messaging/messaging.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,8 @@ function disableRetries(messaging: Messaging) {
(messaging as any).messagingRequestHandler.httpClient.retry = null;
}

class CustomArray extends Array { }

describe('Messaging', () => {
let mockApp: FirebaseApp;
let messaging: Messaging;
Expand Down Expand Up @@ -608,6 +610,40 @@ describe('Messaging', () => {
});
});

it('should be fulfilled with a BatchResponse given array-like (issue #566)', () => {
const messageIds = [
'projects/projec_id/messages/1',
'projects/projec_id/messages/2',
'projects/projec_id/messages/3',
];
mockedRequests.push(mockBatchRequest(messageIds));
const message = {
token: 'a',
android: {
ttl: 3600,
},
};
const arrayLike = new CustomArray();
arrayLike.push(message);
arrayLike.push(message);
arrayLike.push(message);
// Explicitly patch the constructor so that down compiling to ES5 doesn't affect the test.
// See https://github.com/firebase/firebase-admin-node/issues/566#issuecomment-501974238
// for more context.
arrayLike.constructor = CustomArray;

return messaging.sendAll(arrayLike)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you validate the request arrayLike was property processed and converted to array?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no way to validate it. But at least on non-ES5 targets, this test case fails if the SDK doesn't handle array-like objects correctly (it would throw the same error reported in #566).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Thanks for clarifying.

.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;
});
});
});

it('should be fulfilled with a BatchResponse given valid messages in dryRun mode', () => {
const messageIds = [
'projects/projec_id/messages/1',
Expand Down