Non-mini xlsx.js missing "Root Entry" removal for proper ZIP parsing #1736

New Issue

2020-02-16T19:24:32Z

shliama commented

2020-02-16 19:24:32 +00:00

(Migrated from github.com)

There is a difference in safegetzipfile method between xlsx.js & xlsx.mini.js.

One is missing replace(/^Root Entry[\/]/,"") so I get this error "Cannot find file [Content_Types].xml in zip" every time trying to read a XLSX file 🤷‍♂️

There is a difference in `safegetzipfile` method between [xlsx.js](https://github.com/SheetJS/sheetjs/blob/a81bb78f18960053a03bb3350f31ea54d28245be/xlsx.js#L2810) & [xlsx.mini.js](https://github.com/SheetJS/sheetjs/blob/a81bb78f18960053a03bb3350f31ea54d28245be/xlsx.mini.js#L2789). One is missing `replace(/^Root Entry[\/]/,"")` so I get this error "Cannot find file [Content_Types].xml in zip" every time trying to read a XLSX file 🤷‍♂️

👍 2

SheetJSDev commented

2020-02-17 16:34:36 +00:00

(Migrated from github.com)

Thanks for reporting (and for the amusing commentary elsewhere :)

The xlsx.js script was not intended to be used on its own -- there are browser builds in the dist/ folder, and nodejs does something different that avoids the problem.

In particular, the mini build uses cfb for zip processing while the "full" build uses a modified jszip and assumes it exists. The CFB library presents a zip file like an OLE blob, with an implied root folder "Root Entry" even though it doesn't exist in the zip archive.

Fortunately there's an easy patch. The fix should be applied to https://github.com/SheetJS/sheetjs/blob/master/bits/21_ziputils.js#L32 :

-		var n = k[i].toLowerCase();
+		var n = (zip.FullPaths ? k[i].replace(/^Root Entry[\/]/,"") : k[i]).toLowerCase();

Thanks for reporting (and for the amusing commentary elsewhere :) The xlsx.js script was not intended to be used on its own -- there are browser builds in the dist/ folder, and nodejs does something different that avoids the problem. In particular, the mini build uses [cfb](https://github.com/SheetJS/js-cfb) for zip processing while the "full" build uses a modified jszip and assumes it exists. The CFB library presents a zip file like an OLE blob, with an implied root folder "Root Entry" even though it doesn't exist in the zip archive. Fortunately there's an easy patch. The fix should be applied to https://github.com/SheetJS/sheetjs/blob/master/bits/21_ziputils.js#L32 : ```diff - var n = k[i].toLowerCase(); + var n = (zip.FullPaths ? k[i].replace(/^Root Entry[\/]/,"") : k[i]).toLowerCase(); ```

👍 1

tommy7420 commented

2020-04-18 04:28:10 +00:00

(Migrated from github.com)

erinn tjernagel

Alebat commented

2020-04-20 13:49:48 +00:00

(Migrated from github.com)

When will it be published?

👍 1

szilardd commented

2020-05-23 06:23:07 +00:00

(Migrated from github.com)

I'm trying to develop a new functionality (excel taskpane add-in) and was also trying to use xlsx.js in a browser (because this is generated when building the project with make) and got a corruption error when opening the xlsx file.

If you include the jszip.js script before xlsx.js it works.

<script src="jszip.js"></script>
<script src="xlsx.js"></script>

Not sure if there is a different way to test changes in a browser? Haven't found a way to create a new browser build in the dist version.

I'm trying to develop a new functionality (excel taskpane add-in) and was also trying to use `xlsx.js` in a browser (because this is generated when building the project with `make`) and got a corruption error when opening the xlsx file. If you include the `jszip.js` script before `xlsx.js` it works. ``` <script src="jszip.js"></script> <script src="xlsx.js"></script> ``` Not sure if there is a different way to test changes in a browser? Haven't found a way to create a new browser build in the `dist` version.

SheetJSDev commented

2021-09-16 21:36:48 +00:00

(Migrated from github.com)

@szilardd make dist generates all of the standalone browser files. After doing so, dist/xlsx.full.min.js is the desired script.

@szilardd `make dist` generates all of the standalone browser files. After doing so, `dist/xlsx.full.min.js` is the desired script.

reviewher commented

2021-09-21 01:33:45 +00:00

(Migrated from github.com)

ef4aec8796

ef4aec8796c72dc891b619edcd3ef24dde4829d6

Sign in to join this conversation.