Skip to content

Commit

Permalink
Fixed stack overflow with StringUtils.
Browse files Browse the repository at this point in the history
Because we were using Function.apply to create strings, large arrays
were causing problems with browser argument count limits.  This
now splits the arrays up before calling.

Closes #335

Change-Id: Ic94a950997e2f17563ecba8fb628f62c0ed18fc2
  • Loading branch information
TheModMaker committed Apr 19, 2016
1 parent 1749bb6 commit 7c1e7ea
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 7 deletions.
25 changes: 22 additions & 3 deletions lib/util/string_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ goog.require('shaka.util.Error');
shaka.util.StringUtils.fromUTF8 = function(data) {
if (!data) return '';
// http://stackoverflow.com/a/13691499
var utf8 = String.fromCharCode.apply(null, new Uint8Array(data));
var utf8 = shaka.util.StringUtils.fromCharCode_(new Uint8Array(data));
// This converts each character in the string to an escape sequence. If the
// character is in the ASCII range, it is not converted; otherwise it is
// converted to a URI escape sequence.
Expand Down Expand Up @@ -85,13 +85,13 @@ shaka.util.StringUtils.fromUTF16 = function(data, littleEndian) {
}

// Use a DataView to ensure correct endianness.
var arr = [];
var length = data.byteLength / 2;
var arr = new Uint16Array(length);
var dataView = new DataView(buffer);
for (var i = 0; i < length; i++) {
arr[i] = dataView.getUint16(i * 2, littleEndian);
}
return String.fromCharCode.apply(null, arr);
return shaka.util.StringUtils.fromCharCode_(arr);
};


Expand Down Expand Up @@ -159,3 +159,22 @@ shaka.util.StringUtils.toUTF8 = function(str) {
}
return result.buffer;
};


/**
* Creates a new string from the given array of char codes.
*
* @param {!ITypedArray} args
* @return {string}
* @private
*/
shaka.util.StringUtils.fromCharCode_ = function(args) {
var max = 16000;
var ret = '';
for (var i = 0; i < args.length; i += max) {
var subArray = args.subarray(i, i + max);
ret += String.fromCharCode.apply(null, subArray);
}

return ret;
};
15 changes: 11 additions & 4 deletions test/string_utils_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,17 @@ describe('StringUtils', function() {
it('parses fromUTF16 big-endian', function() {
// This is big-endian pairs of 16-bit numbers. This translates into 3
// Unicode characters where the last is split into a surrogate pair.
var arr = [0x00, 0x46, 0x38, 0x01, 0xdc, 0x37, 0x5d, 0x00];
var arr = [0x00, 0x46, 0x38, 0x01, 0xd8, 0x01, 0xdc, 0x37];
var buffer = new Uint8Array(arr).buffer;
expect(StringUtils.fromUTF16(buffer, false)).toBe('F\u3801\udc37\u5d00');
expect(StringUtils.fromUTF16(buffer, false)).toBe('F\u3801\ud801\udc37');
});

it('parses fromUTF16 little-endian', function() {
// This is little-endian pairs of 16-bit numbers. This translates into 3
// Unicode characters where the last is split into a surrogate pair.
var arr = [0x46, 0x00, 0x01, 0x38, 0x37, 0xdc, 0x00, 0x5d];
var arr = [0x46, 0x00, 0x01, 0x38, 0x01, 0xd8, 0x37, 0xdc];
var buffer = new Uint8Array(arr).buffer;
expect(StringUtils.fromUTF16(buffer, true)).toBe('F\u3801\udc37\u5d00');
expect(StringUtils.fromUTF16(buffer, true)).toBe('F\u3801\ud801\udc37');
});

describe('fromBytesAutoDetect', function() {
Expand Down Expand Up @@ -102,4 +102,11 @@ describe('StringUtils', function() {
var buffer = StringUtils.toUTF8(str);
expect(new Uint8Array(buffer)).toEqual(new Uint8Array(arr));
});

it('does not cause stack overflow, #335', function() {
var buffer = new Uint8Array(8e5).buffer; // Well above arg count limit.
expect(StringUtils.fromUTF8(buffer).length).toBe(buffer.byteLength);
expect(StringUtils.fromUTF16(buffer, true).length)
.toBe(buffer.byteLength / 2);
});
});

0 comments on commit 7c1e7ea

Please sign in to comment.