xero-node icon indicating copy to clipboard operation
xero-node copied to clipboard

ObjectSerializer produces incorrect output when Array.prototype has been modified

Open marlonbernardes opened this issue 1 year ago • 0 comments

Hello - first of all thanks for taking the time to create this SDK!

I started using the SDK recently and I found a bug which can make it unusable in some cases (which is also the root cause for #508). The fix seems to be fairly straightforward (more info below) so I am happy to help to get this fixed.

SDK you're using (please complete the following information):

  • Version 4.22.0

Describe the bug

Whenever an SDK method is called (e.g xero.accountingApi.createInvoices or xero.accountingApi.createContacts) the class ObjectSerializer is called under the hood to serialize/deserialize the payloads sent/received to/from Xero's API.

The methods serialize and deserialize of this class produce incorrect output if new properties were added to Array.prototype.

Adding new properties to Array.prototype is certainly a bad practice (as it is a bad practice, in general, to modify any object that you don't own) - but regardless, it should not cause xero's SDK to break.

To Reproduce

const { ObjectSerializer } = require('xero-node');

// Example payload with a single invoice added to it
const payload = {
  invoices: [
    {
      type: 'ACCREC',
      contact: {  contactID: '123' },
      lineItems: []
    }
  ]
};

// Even though we wouldn't normally need to call ObjectSerializer directly, 
// this makes it easier to reproduce the bug that happens when we call other methods of the sdk.
console.log(ObjectSerializer.serialize(payload, 'Invoices'));
// correct! output has only one invoice
/* 
{
  Invoices: [
    {
      Type: 'ACCREC',
      Contact: [Object],
      LineItems: [],
      Date: undefined
      // ... 
    }
  ]
}
*/

// Adding two new properties to `Array.prototype`
Array.prototype.foo = () => {}
Array.prototype.bar = () => {}

// ERROR! output has three invoices instead of one
// (one extra invoice is added for each property added to Array.prototype)
console.log(ObjectSerializer.serialize(payload, 'Invoices'));

/*
/*
 {
  Invoices: [
    {
      Type: 'ACCREC',
      Contact: [Object],
      LineItems: [Array],
      Date: undefined,
      // ...
    },
    {
      Type: undefined,
      Contact: undefined,
      LineItems: undefined,
      Date: undefined,
      // ...
    },
    {
      Type: undefined,
      Contact: undefined,
      LineItems: undefined,
      Date: undefined,
      // ...
    }
  ],
  undefined: undefined
}
*/

How to fix

  • Iterate elements of array using a for..of loop (or plain for loop if compatibility is an issue), as the for..in expression should be used to enumerate properties and not to iterate over them.
  • Example: https://github.com/XeroAPI/xero-node/blob/master/src/gen/model/accounting/models.ts#L516-L519

Expected behavior

  • ObjectSerializer should not add new elements to arrays when serializing/deserializing values.

marlonbernardes avatar Jul 26 '22 14:07 marlonbernardes