Don't break when attempting to import a zip file that's not an Excel file (eg. .numbers)

[papandreou]

Hi!

We're running a service that uses exceljs to process uploaded Excel spreadsheets. The other day a user accidentally uploaded a wrongly named .numbers file, which caused an exceljs crash:

     TypeError: Cannot read property 'reduce' of undefined
      at module.exports.reconcile (dist/es5/xlsx/xform/book/workbook-xform.js:145:34)
      at module.exports.reconcile (dist/es5/xlsx/xlsx.js:85:19)
      at dist/es5/xlsx/xlsx.js:316:14

I had a quick look, and it seems like .numbers files also use a zip-based container, but since all the entries are differently named compared to .xlsx, some internal state ends up being unintialized, which makes reconcile break.

Turns out it was rather easy to just default that state so the file loads, but isn't populated with any worksheets. It would probably be better to reject the promise with a specific error, though. What do you think?

[papandreou]

Hi Michael, Thanks for taking a look! The issue is that `model.properties` doesn't appear to exist when the zip file isn't structured in a certain way, so `model.properties.date1904` (or even `model.properties || model.properties.date1904`) will dereference undefined and cause: ``` TypeError: Cannot read property 'date1904' of undefined ``` ... when running the enclosed regression test. Best regards, Andreas

…

On Mon, Nov 6, 2017 at 8:10 PM, Michael Lelyakin ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In lib/xlsx/xlsx.js <#423 (comment)>: > @@ -108,7 +108,7 @@ XLSX.prototype = { sharedStrings: model.sharedStrings, media: model.media, mediaIndex: model.mediaIndex, - date1904: model.properties.date1904, + date1904: model.properties && model.properties.date1904, I'm not sure that the meaning of this line of code, but I think that you want to use || operator instead of &&. Are you sure in this line? Can you provide me little clarification for it? :) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#423 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAWzKcaXrmRmtz7Zs_WWwEMVoLyiURsRks5sz1mVgaJpZM4P90sF> .

[guyonroche]

Hey Guys,
Finally I have some time to catch up on excel stuff. This looks good - non breaking benign kind of change so I'm happy to include it. Will try to publish some time this week