Improved safegetzipfile() #567

2017-02-22T14:57:04Z

duzun commented

2017-02-22 14:57:04 +00:00

(Migrated from github.com)

I have an xlsx file that can't be read by this lib because there is a xl/SharedStrings.xml file (notice upper case "S").

I've improved safegetzipfile() function to overcome this issue.

I have an xlsx file that can't be read by this lib because there is a `xl/SharedStrings.xml` file (notice upper case "S"). I've improved `safegetzipfile()` function to overcome this issue.

SheetJSDev commented

2017-02-22 18:05:10 +00:00

(Migrated from github.com)

@duzun thanks for the heads up! It's a strange error because I can manually produce files with a capital S that work in excel and when fed back to js-xlsx.

If you don't mind doing a bit of sleuthing, can you look at the [Content_Types].xml file and find the part name corresponding to the content type application/vnd.openxmlformats-officedocument.spreadsheetml.sharedStrings+xml? We look at the content types file to find the name of the shared strings file. If that file was written correctly then you should see a line like:

<Override PartName="/xl/SharedStrings.xml" ContentType="application/vnd.openxmlformats-officedocument.spreadsheetml.sharedStrings+xml"/>

If the case is correct then there's some issue with the content types parsing. If you see the part name has a different case, like PartName="/xl/sharedStrings.xml", then we probably should make the getfile functions case insensitive.

@duzun thanks for the heads up! It's a strange error because I can manually produce files with a capital S that work in excel and when fed back to js-xlsx. If you don't mind doing a bit of sleuthing, can you look at the `[Content_Types].xml` file and find the part name corresponding to the content type `application/vnd.openxmlformats-officedocument.spreadsheetml.sharedStrings+xml`? We look at the content types file to find the name of the shared strings file. If that file was written correctly then you should see a line like: ``` <Override PartName="/xl/SharedStrings.xml" ContentType="application/vnd.openxmlformats-officedocument.spreadsheetml.sharedStrings+xml"/> ``` If the case is correct then there's some issue with the content types parsing. If you see the part name has a different case, like `PartName="/xl/sharedStrings.xml"`, then we probably should make the getfile functions case insensitive.

reviewher commented

2017-02-23 04:55:55 +00:00

(Migrated from github.com)

Related: https://github.com/SheetJS/js-xlsx/issues/439

duzun commented

2017-02-27 11:10:20 +00:00

(Migrated from github.com)

Hi @SheetJSDev

I've checked that file and found that "application/vnd.openxmlformats-officedocument.spreadsheetml.sharedStrings+xml" in [Content_Types].xml is "/xl/sharedStrings.xml", but the file is named "/xl/SharedStrings.xml" in the archive.

I suppose this particular file is generated by some software other then office.

BTW I've tested the new release 0.8.6 and the issue is gone!

Thanks!

Hi @SheetJSDev I've checked that file and found that `"application/vnd.openxmlformats-officedocument.spreadsheetml.sharedStrings+xml"` in `[Content_Types].xml` is `"/xl/sharedStrings.xml"`, but the file is named `"/xl/SharedStrings.xml"` in the archive. I suppose this particular file is generated by some software other then office. BTW I've tested the new release `0.8.6` and the issue is gone! Thanks!

SheetJSDev commented

2017-02-27 17:56:00 +00:00

(Migrated from github.com)

@duzun either way, as I explained in another comment, readers are supposed to do case-insensitive name comparisons.

And as for the PR, it's preferable to do a direct scan of the keys in this case. I think there may be a few more places where filter is used and they should ultimately be reworked with a non-filter variant

@duzun either way, as I explained [in another comment](https://github.com/SheetJS/js-xlsx/issues/439#issuecomment-281903752), readers are supposed to do case-insensitive name comparisons. And as for the PR, it's preferable to do a direct scan of the keys in this case. I think there may be a few more places where `filter` is used and they should ultimately be reworked with a non-filter variant