function utils.book_append_sheet crashes if no sheet name provided #652

New Issue

2017-05-12T08:58:50Z

jomel commented

2017-05-12 08:58:50 +00:00

(Migrated from github.com)

TypeError: Cannot read property 'length' of undefined at check_ws_name (/***/node_modules/xlsx/xlsx.js:12142:6) at Object.utils.book_append_sheet (/***/node_modules/xlsx/xlsx.js:17594:2) ....
As far as I can tell it's because the sheet name is never SET.

Here's a fix:

/* add a worksheet to the end of a given workbook */
utils.book_append_sheet = function (wb/*:Workbook*/, ws/*:Worksheet*/, name/*:?string*/) {
   if (!name) {
      for (var i = 1; i <= 0xFFFF; ++i) {
         var name = "Sheet" + i;
         if (wb.SheetNames.indexOf(name) == -1)
            break;
      }
   }
   check_ws_name(name);
   if (wb.SheetNames.indexOf(name) >= 0) throw new Error("Worksheet with name |" + name + "| already exists!");

   wb.SheetNames.push(name);
   wb.Sheets[name] = ws;
};

`TypeError: Cannot read property 'length' of undefined at check_ws_name (/***/node_modules/xlsx/xlsx.js:12142:6) at Object.utils.book_append_sheet (/***/node_modules/xlsx/xlsx.js:17594:2) .... ` As far as I can tell it's because the sheet name is never SET. Here's a fix: ``` /* add a worksheet to the end of a given workbook */ utils.book_append_sheet = function (wb/*:Workbook*/, ws/*:Worksheet*/, name/*:?string*/) { if (!name) { for (var i = 1; i <= 0xFFFF; ++i) { var name = "Sheet" + i; if (wb.SheetNames.indexOf(name) == -1) break; } } check_ws_name(name); if (wb.SheetNames.indexOf(name) >= 0) throw new Error("Worksheet with name |" + name + "| already exists!"); wb.SheetNames.push(name); wb.Sheets[name] = ws; }; ```

👍 1

SheetJSDev commented

2017-05-12 14:20:59 +00:00

(Migrated from github.com)

@jomel good catch! you don't need a var there. The shortest fix is to assign name within the loop:

-       if(!name) for(var i = 1; i <= 0xFFFF; ++i) if(wb.SheetNames.indexOf("Sheet" + i) == -1) break;
+       if(!name) for(var i = 1; i <= 0xFFFF; ++i) if(wb.SheetNames.indexOf(name = "Sheet" + i) == -1) break;

@jomel good catch! you don't need a `var` there. The shortest fix is to assign `name` within the loop: ```diff - if(!name) for(var i = 1; i <= 0xFFFF; ++i) if(wb.SheetNames.indexOf("Sheet" + i) == -1) break; + if(!name) for(var i = 1; i <= 0xFFFF; ++i) if(wb.SheetNames.indexOf(name = "Sheet" + i) == -1) break; ```

Sign in to join this conversation.