From daa316643a990043ec24a8fe6b9fa7f96da14fb4 Mon Sep 17 00:00:00 2001 From: Theodore Abshire Date: Fri, 6 Mar 2020 12:16:09 -0800 Subject: [PATCH] Fix compiled release version of supportsChunkSize. In the code for supportsChunkSize, we test to see if a given chunk size is supported by array to string conversion by trying to perform an operation on a Uint8Array of that size without throwing an error. However, the result of that operation was only ever referenced inside an assert. Because asserts are compiled out in release builds, the result of that operation was not being used... and thus, the entire call was being compiled out. This changes the return value of the function to use the result of the operation, thus preventing it from being compiled out. This also adds a unit test that will detect this problem in the future. Closes #2433 Change-Id: If3048531afc460beb16de0dda7f7fcbd5499fdaf --- lib/util/lazy.js | 5 + lib/util/string_utils.js | 11 +- test/player_integration.js | 26 + test/test/assets/large_file.mpd | 5111 +++++++++++++++++++++++++++++++ 4 files changed, 5152 insertions(+), 1 deletion(-) create mode 100644 test/test/assets/large_file.mpd diff --git a/lib/util/lazy.js b/lib/util/lazy.js index 5ac1910b1..5951bdc6b 100644 --- a/lib/util/lazy.js +++ b/lib/util/lazy.js @@ -39,4 +39,9 @@ shaka.util.Lazy = class { } return this.value_; } + + /** Resets the value of the lazy function, so it has to be remade. */ + reset() { + this.value_ = undefined; + } }; diff --git a/lib/util/string_utils.js b/lib/util/string_utils.js index 160ac5397..98f0bdb74 100644 --- a/lib/util/string_utils.js +++ b/lib/util/string_utils.js @@ -201,6 +201,15 @@ shaka.util.StringUtils = class { static fromCharCode(array) { return shaka.util.StringUtils.fromCharCodeImpl_.value()(array); } + + /** + * Resets the fromCharCode method's implementation. + * For debug use. + * @export + */ + static resetFromCharCode() { + shaka.util.StringUtils.fromCharCodeImpl_.reset(); + } }; @@ -219,7 +228,7 @@ shaka.util.StringUtils.fromCharCodeImpl_ = new shaka.util.Lazy(() => { // eslint-disable-next-line no-restricted-syntax const foo = String.fromCharCode.apply(null, buffer); goog.asserts.assert(foo, 'Should get value'); - return true; + return foo.length > 0; // Actually use "foo", so it's not compiled out. } catch (error) { return false; } diff --git a/test/player_integration.js b/test/player_integration.js index 0f24a2427..e184ff2f7 100644 --- a/test/player_integration.js +++ b/test/player_integration.js @@ -602,6 +602,32 @@ describe('Player', () => { }); }); // describe('tracks') + describe('loading', () => { + // A regression test for Issue #2433. + it('can load very large files', async () => { + // Reset the lazy function, so that it does not remember any chunk size + // that was detected beforehand. + compiledShaka.util.StringUtils.resetFromCharCode(); + const oldFromCharCode = String.fromCharCode; + try { + // Replace String.fromCharCode with a version that can only handle very + // small chunks. + // This has to be an old-style function, to use the "arguments" object. + // eslint-disable-next-line no-restricted-syntax + String.fromCharCode = function() { + if (arguments.length > 2000) { + throw new RangeError('Synthetic Range Error'); + } + // eslint-disable-next-line no-restricted-syntax + return oldFromCharCode.apply(null, arguments); + }; + await player.load('base/test/test/assets/large_file.mpd'); + } finally { + String.fromCharCode = oldFromCharCode; + } + }); + }); + describe('buffering', () => { const startBuffering = jasmine.objectContaining({buffering: true}); const endBuffering = jasmine.objectContaining({buffering: false}); diff --git a/test/test/assets/large_file.mpd b/test/test/assets/large_file.mpd new file mode 100644 index 000000000..f79fd60b1 --- /dev/null +++ b/test/test/assets/large_file.mpd @@ -0,0 +1,5111 @@ + + + + + + + + + text_el.mp4 + + + + + + + + + text_fr.mp4 + + + + + + + + + text_en.mp4 + + + + + + + + + text_pt-BR.mp4 + + + + + + + + + audio_es_2c_128k_aac.mp4 + + + + + + + + + audio_de_2c_128k_aac.mp4 + + + + + + + + video_240p_242k_h264.mp4 + + + + + + video_360p_400k_h264.mp4 + + + + + + video_144p_108k_h264.mp4 + + + + + + video_480p_1M_h264.mp4 + + + + + + video_576p_1.5M_h264.mp4 + + + + + + + + video_240p_151k_vp9.webm + + + + + + video_360p_277k_vp9.webm + + + + + + video_576p_768k_vp9.webm + + + + + + video_144p_96k_vp9.webm + + + + + + video_480p_512k_vp9.webm + + + + + + + + + audio_en_2c_128k_aac.mp4 + + + + + + + + + audio_fr_2c_64k_opus.webm + + + + + + + + + audio_es_2c_64k_opus.webm + + + + + + + + + audio_en_2c_64k_opus.webm + + + + + + + + + audio_fr_2c_128k_aac.mp4 + + + + + + + + + audio_de_2c_64k_opus.webm + + + + + + + + + audio_it_2c_64k_opus.webm + + + + + + + + + audio_it_2c_128k_aac.mp4 + + + + + + +