bug fix: when the arrayBuffer is to large, __toBuffer will cause error #937

2018-01-02T06:01:37Z

hacker1117 commented

2018-01-02 06:01:37 +00:00

(Migrated from github.com)

The __toBuufer function use "x.push.apply",but the argument has limits. when the length of array is larger than 65536, it will cause an error: Maximum call stack size exceeded.(refer to: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply)
so i apply chunks of the array at a time.

The __toBuufer function use "x.push.apply",but the argument has limits. when the length of array is larger than 65536, it will cause an error: Maximum call stack size exceeded.(refer to: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply) so i apply chunks of the array at a time.

👍 2

SheetJSDev commented

2018-01-02 06:55:05 +00:00

(Migrated from github.com)

On the read side, before adding the array input type, a helper function used to chunk in 10KB units to circumvent the same problem. Not sure why a similar change wasn't also applied here.
a8736580a5/index.html (L91-L96)

function fixdata(data) {
	var o = "", l = 0, w = 10240;
	for(; l<data.byteLength/w; ++l) o+=String.fromCharCode.apply(null,new Uint8Array(data.slice(l*w,l*w+w)));
	o+=String.fromCharCode.apply(null, new Uint8Array(data.slice(l*w)));
	return o;
}

Here's a test fiddle that fails in the current version https://jsfiddle.net/pg6oghLu/ and passes with the proposed changes https://jsfiddle.net/pg6oghLu/1/ :

// <pre id="results"></pre>
var verbose = false;
results.innerText = PRINTJ.sprintf("BOOKTYPE  TYPE   RESULT\n");
var wb = XLSX.utils.book_new();

/* Generate 500 rows of 20 columns each */
var data = [];
data[0] = ["Row Number"];
for(var j = 0; j < 19; ++j) data[0].push("Column " + j+1);

for(var i = 0; i < 499; ++i) {
  var o = ["Row " + i];
  for(j = 0; j < 19; ++j) o.push(i + j);
  data.push(o);
}

var ws = XLSX.utils.aoa_to_sheet(data);
XLSX.utils.book_append_sheet(wb, ws, "Data");

["xlsx", "xlsb", "biff8", "biff5", "biff2", "xlml", "ods", "fods", "csv", "txt", "sylk", "html", "dif", "dbf", "rtf", "prn", "eth"].forEach(function(btype) {
  ["binary", "array", "base64"].forEach(function(type) {
    var res = "Y";
    try { var out = XLSX.write(wb, {bookType:btype, type:type}); XLSX.read(out, {type:type}); } catch(e) { console.log(btype, type, e.message); res = "N"; }
    if(verbose || res != "Y") results.innerText += PRINTJ.sprintf("%9s %6s %s\n", btype, type, res);    	
  });
});

On the read side, before adding the array input type, a helper function used to chunk in 10KB units to circumvent the same problem. Not sure why a similar change wasn't also applied here. https://github.com/SheetJS/js-xlsx/blob/a8736580a58011d112032f656a0e21779f78aa2f/index.html#L91-L96 ```js function fixdata(data) { var o = "", l = 0, w = 10240; for(; l<data.byteLength/w; ++l) o+=String.fromCharCode.apply(null,new Uint8Array(data.slice(l*w,l*w+w))); o+=String.fromCharCode.apply(null, new Uint8Array(data.slice(l*w))); return o; } ``` Here's a test fiddle that fails in the current version https://jsfiddle.net/pg6oghLu/ and passes with the proposed changes https://jsfiddle.net/pg6oghLu/1/ : ```js // <pre id="results"></pre> var verbose = false; results.innerText = PRINTJ.sprintf("BOOKTYPE TYPE RESULT\n"); var wb = XLSX.utils.book_new(); /* Generate 500 rows of 20 columns each */ var data = []; data[0] = ["Row Number"]; for(var j = 0; j < 19; ++j) data[0].push("Column " + j+1); for(var i = 0; i < 499; ++i) { var o = ["Row " + i]; for(j = 0; j < 19; ++j) o.push(i + j); data.push(o); } var ws = XLSX.utils.aoa_to_sheet(data); XLSX.utils.book_append_sheet(wb, ws, "Data"); ["xlsx", "xlsb", "biff8", "biff5", "biff2", "xlml", "ods", "fods", "csv", "txt", "sylk", "html", "dif", "dbf", "rtf", "prn", "eth"].forEach(function(btype) { ["binary", "array", "base64"].forEach(function(type) { var res = "Y"; try { var out = XLSX.write(wb, {bookType:btype, type:type}); XLSX.read(out, {type:type}); } catch(e) { console.log(btype, type, e.message); res = "N"; } if(verbose || res != "Y") results.innerText += PRINTJ.sprintf("%9s %6s %s\n", btype, type, res); }); }); ```

hacker1117 commented

2018-01-02 08:27:52 +00:00

(Migrated from github.com)

@SheetJSDev So should I apply this change to the fixdata() function? but i can't find this function in 'index.html'.:(

SheetJSDev commented

2018-01-02 16:23:24 +00:00

(Migrated from github.com)

@hacker1117 sorry I wasn't clear. For the same reason that you encountered, there used to be a chunking algorithm in processing ArrayBuffers that used QUANTUM=10240 instead of the original 32768, assumably because of browser compatibility. We'll rework the commit and add a test :)

@hacker1117 sorry I wasn't clear. For the same reason that you encountered, there used to be a chunking algorithm in processing ArrayBuffers that used `QUANTUM=10240` instead of the original 32768, assumably because of browser compatibility. We'll rework the commit and add a test :)

SheetJSDev commented

2018-01-02 16:55:04 +00:00

(Migrated from github.com)

@hacker1117 The 10240 threshold works in IE8+ as evidenced in the browser test https://saucelabs.com/beta/builds/3de647aa4e09454fa30894170668a8d4 . For whatever reason, IE8 was failing on 32768, which is probably why we used the lower number in the first place.