Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid error on malformed address #567

Merged
merged 2 commits into from
Jun 6, 2018

Conversation

papandreou
Copy link
Contributor

Hi, I've run into another weird Excel file that causes an error in exceljs:

TypeError: Cannot read property '0' of null
    at Object.decodeAddress (/Users/andreaslind/work/exceljs/lib/utils/col-cache.js:95:33)
    at Object.decodeEx (/Users/andreaslind/work/exceljs/lib/utils/col-cache.js:156:21)
    at /Users/andreaslind/work/exceljs/lib/xlsx/xform/book/workbook-xform.js:182:34
    at Array.forEach (<anonymous>)
    at Object.each (/Users/andreaslind/work/exceljs/lib/utils/under-dash.js:7:13)
    at module.exports.reconcile (/Users/andreaslind/work/exceljs/lib/xlsx/xform/book/workbook-xform.js:175:7)
    at module.exports.reconcile (/Users/andreaslind/work/exceljs/lib/xlsx/xlsx.js:91:19)
    at /Users/andreaslind/work/exceljs/lib/xlsx/xlsx.js:345:16
    at <anonymous>

The error is triggered by this xl/workbook.xml (slightly edited and pretty-printed):

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<workbook xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main" xmlns:r="http://schemas.openxmlformats.org/officeDocument/2006/relationships" xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" xmlns:x15="http://schemas.microsoft.com/office/spreadsheetml/2010/11/main" xmlns:xr="http://schemas.microsoft.com/office/spreadsheetml/2014/revision" xmlns:xr6="http://schemas.microsoft.com/office/spreadsheetml/2016/revision6" xmlns:xr10="http://schemas.microsoft.com/office/spreadsheetml/2016/revision10" xmlns:xr2="http://schemas.microsoft.com/office/spreadsheetml/2015/revision2" mc:Ignorable="x15 xr xr6 xr10 xr2">
  <fileVersion appName="xl" lastEdited="7" lowestEdited="6" rupBuild="10311"/>
  <workbookPr/>
  <xr:revisionPtr revIDLastSave="0" documentId="13_ncr:1_{C8FC9E1B-96BE-724A-A363-02D52AD78D60}" xr6:coauthVersionLast="31" xr6:coauthVersionMax="32" xr10:uidLastSave="{00000000-0000-0000-0000-000000000000}"/>
  <bookViews>
    <workbookView xWindow="260" yWindow="460" windowWidth="28280" windowHeight="15860" tabRatio="500" xr2:uid="{00000000-000D-0000-FFFF-FFFF00000000}"/>
  </bookViews>
  <sheets>
    <sheet name="Sheet1" sheetId="1" r:id="rId1"/>
  </sheets>
  <definedNames>
    <definedName name="_xlnm._FilterDatabase" localSheetId="0" hidden="1">Sheet1!$A$1:$V$121</definedName>
    <definedName name="_xlnm.Print_Area" localSheetId="0">Sheet1!$B:$F</definedName>
  </definedNames>
  <calcPr calcId="179017" concurrentCalc="0"/>
</workbook>

Specifically Sheet1!$B:$F. The fragments are passed to colCache.decodeAddress, which assumes that there's a row number in each of the references ($B and $F). I can't tell if this syntax is legal or not -- I don't really know much about the .xlsx file format.

I tried to create a spec that triggers the error, but it seems that basing it off
scenarios in https://github.com/guyonroche/exceljs/blob/master/spec/unit/xlsx/xform/book/workbook-xform.spec.js doesn't work as that does not exercise the reconciliation phase.

The enclosed change avoids the error by making decodeAddress do a "best effort" when handed this kind of weird reference. I honestly don't know if that is the right way to go about it.

Unfortunately, I'm unable to share the original .xlsx file that triggered the error.

@papandreou
Copy link
Contributor Author

The <definedName name="_xlnm.Print_Area" localSheetId="0">Sheet1!$B:$F</definedName> element survives a load->save roundtrip through Excel for Mac 16.12, so it doesn't seem like this one is due to a bad 3rd party integration.

@guyonroche
Copy link
Collaborator

@papandreou this is a good catch. I've just added an integration test using an Excel generated file based on your description - just create a Hello World spreadsheet with a print area set to $B:$F and boom!
Will merge now and publish :-)

@guyonroche guyonroche merged commit 17e4a5d into exceljs:master Jun 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants