Some filenames causes EXCEL to report error #2722

New Issue

2022-06-19T11:16:03Z

lemonbob commented

2022-06-19 11:16:03 +00:00

(Migrated from github.com)

Hi, attached is the file that has thrown an error by EXCEL. The fileName is: RiskThemes_ Bubble Chart.xlsx.

I tried exporting with a fileName of test.xlsx and it is fine, so this must be a filename issue. I don't see anything wrong with the filename RiskThemes_ Bubble Chart.xlsx - it seems perfectly valid to me? Any ideas?

Attached is the file.
RiskThemes_ Bubble Chart.xlsx

Hi, attached is the file that has thrown an error by EXCEL. The fileName is: `RiskThemes_ Bubble Chart.xlsx`. I tried exporting with a fileName of `test.xlsx` and it is fine, so this must be a filename issue. I don't see anything wrong with the filename `RiskThemes_ Bubble Chart.xlsx` - it seems perfectly valid to me? Any ideas? Attached is the file. [RiskThemes_ Bubble Chart.xlsx](https://github.com/SheetJS/sheetjs/files/8935251/RiskThemes_.Bubble.Chart.xlsx)

SheetJSDev commented

2022-06-19 17:06:16 +00:00

(Migrated from github.com)

: is missing from the bad characters list, feel free to submit a PR

--- a/bits/71_wbcommon.js
+++ b/bits/71_wbcommon.js
@@ -113,7 +113,7 @@ function safe1904(wb/*:Workbook*/)/*:string*/ {
        return parsexmlbool(wb.Workbook.WBProps.date1904) ? "true" : "false";
 }
 
-var badchars = /*#__PURE__*/"][*?\/\\".split("");
+var badchars = /*#__PURE__*/":][*?\/\\".split("");
 function check_ws_name(n/*:string*/, safe/*:?boolean*/)/*:boolean*/ {
        if(n.length > 31) { if(safe) return false; throw new Error("Sheet names cannot exceed 31 chars"); }
        var _good = true;

`:` is missing from the bad characters list, feel free to submit a PR ```diff --- a/bits/71_wbcommon.js +++ b/bits/71_wbcommon.js @@ -113,7 +113,7 @@ function safe1904(wb/*:Workbook*/)/*:string*/ { return parsexmlbool(wb.Workbook.WBProps.date1904) ? "true" : "false"; } -var badchars = /*#__PURE__*/"][*?\/\\".split(""); +var badchars = /*#__PURE__*/":][*?\/\\".split(""); function check_ws_name(n/*:string*/, safe/*:?boolean*/)/*:boolean*/ { if(n.length > 31) { if(safe) return false; throw new Error("Sheet names cannot exceed 31 chars"); } var _good = true; ```

lemonbob commented

2022-06-20 07:31:52 +00:00

(Migrated from github.com)

Hi,

I had a look at the xlsx.js file and I wondered what the purpose of check_ws_name safe param was, please? It is never used? Also the return value _good never seems to be used?

I also wondered, rather than throwing an Error when the filename contains "badchars" it would, perhaps, be better UX to simply replace them for the user?

This would have to be done in book_append_worksheet and the various write methods so is a bigger change - e.g.

name = name.replace(/[\\\/\?\*\[\]\:]/g, '_');

To maintain the logic exactly as is with the throw, the following appears to be all we need as the safe parameter is always undefined and the return value is never used

function check_ws_name(n) {
	let matches = n.match(/[\\\/\?\*\[\]\:]/g);
	if (matches != undefined) throw new Error("Sheet name cannot contain : \\ / ? * [ ] :");	
}

In any event, forEach and looping through badchars converted to an array seems a little clunky. Please advise which of the above solutions would be preferable?

Hi, I had a look at the `xlsx.js` file and I wondered what the purpose of `check_ws_name` `safe` param was, please? It is never used? Also the return value `_good` never seems to be used? I also wondered, rather than throwing an Error when the filename contains "badchars" it would, perhaps, be better UX to simply replace them for the user? This would have to be done in `book_append_worksheet` and the `various` write methods so is a bigger change - e.g. ```js name = name.replace(/[\\\/\?\*\[\]\:]/g, '_'); ``` To maintain the logic exactly as is with the throw, the following appears to be all we need as the safe parameter is always undefined and the return value is never used ```js function check_ws_name(n) { let matches = n.match(/[\\\/\?\*\[\]\:]/g); if (matches != undefined) throw new Error("Sheet name cannot contain : \\ / ? * [ ] :"); } ``` In any event, forEach and looping through badchars converted to an array seems a little clunky. Please advise which of the above solutions would be preferable?

SheetJSDev commented

2022-06-20 17:40:20 +00:00

(Migrated from github.com)

The latter solution is preferable (modifying check_ws_name).

It's preferable to throw the error message as that is consistent with Excel. If you try to rename a worksheet to contain a : character, Excel throws an error The name does not contain any of the following characters: : \ / ? * [ or ]

The main reason not to autocorrect the worksheet name involves features like defined names and formulae. Suppose you were unaware of the Excel limitation and wrote a formula in a cell that used a cross-sheet reference (='abc:def'!A1). This formula would be incorrect if the worksheet is renamed. It is possible to write the formula in XLSX, but Excel treats it like an external reference. Small example: https://jsfiddle.net/dtshugz7/

var ws = XLSX.utils.aoa_to_sheet([[{t:"n",f:"'abc:def'!A1"}]]);
var wb = XLSX.utils.book_new();
XLSX.utils.book_append_sheet(wb, ws, "Sheet1");
XLSX.writeFile(wb, "issue2722.xlsx");

The latter solution is preferable (modifying `check_ws_name`). It's preferable to throw the error message as that is consistent with Excel. If you try to rename a worksheet to contain a `:` character, Excel throws an error `The name does not contain any of the following characters: : \ / ? * [ or ]` The main reason not to autocorrect the worksheet name involves features like defined names and formulae. Suppose you were unaware of the Excel limitation and wrote a formula in a cell that used a cross-sheet reference (`='abc:def'!A1`). This formula would be incorrect if the worksheet is renamed. It is possible to write the formula in XLSX, but Excel treats it like an external reference. Small example: https://jsfiddle.net/dtshugz7/ ```js var ws = XLSX.utils.aoa_to_sheet([[{t:"n",f:"'abc:def'!A1"}]]); var wb = XLSX.utils.book_new(); XLSX.utils.book_append_sheet(wb, ws, "Sheet1"); XLSX.writeFile(wb, "issue2722.xlsx"); ```

Sign in to join this conversation.