diff --git a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py index 3cdf01e543..1f2cdef395 100644 --- a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py +++ b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py @@ -9,6 +9,7 @@ from mock import patch, Mock from django.test.utils import override_settings from django.conf import settings from django.utils import translation +from django.utils.crypto import get_random_string from nose.plugins.skip import SkipTest @@ -230,6 +231,17 @@ class TestDownloadYoutubeSubs(SharedModuleStoreTestCase): self.assertEqual(html5_ids[2], 'baz.1.4') self.assertEqual(html5_ids[3], 'foo') + def test_html5_id_length(self): + """ + Test that html5_id is parsed with length less than 255, as html5 ids are + used as name for transcript objects and ultimately as filename while creating + file for transcript at the time of exporting a course. + Filename can't be longer than 255 characters. + 150 chars is agreed length. + """ + html5_ids = transcripts_utils.get_html5_ids([get_random_string(255)]) + self.assertEqual(len(html5_ids[0]), 150) + @patch('xmodule.video_module.transcripts_utils.requests.get') def test_fail_downloading_subs(self, mock_get): diff --git a/cms/static/js/spec/video/transcripts/utils_spec.js b/cms/static/js/spec/video/transcripts/utils_spec.js index f9ca4e6ae9..b5f448989d 100644 --- a/cms/static/js/spec/video/transcripts/utils_spec.js +++ b/cms/static/js/spec/video/transcripts/utils_spec.js @@ -215,6 +215,42 @@ function($, _, Utils, _str) { }); }); }); + + describe('Too long arguments ', function() { + var longFileName = (function() { + var text = ''; + var possibleChars = 'abcdefghijklmnopqrstuvwxyz'; + /* eslint vars-on-top: 0 */ + for (var i = 0; i < 255; i++) { + text += possibleChars.charAt(Math.floor(Math.random() * possibleChars.length)); + } + return text; + }()), + html5LongUrls = (function(videoName) { + var links = [ + 'http://somelink.com/%s?param=1¶m=2#hash', + 'http://somelink.com/%s#hash', + 'http://somelink.com/%s?param=1¶m=2', + 'http://somelink.com/%s', + 'ftp://somelink.com/%s', + 'https://somelink.com/%s', + 'https://somelink.com/sub/sub/%s', + 'http://cdn.somecdn.net/v/%s', + 'somelink.com/%s', + '%s' + ]; + return $.map(links, function(link) { + return _str.sprintf(link, videoName); + }); + }(longFileName)); + + $.each(html5LongUrls, function(index, link) { + it(link, function() { + var result = Utils.parseHTML5Link(link); + expect(result.video.length).toBe(150); + }); + }); + }); }); it('Method: getYoutubeLink', function() { diff --git a/cms/static/js/views/video/transcripts/utils.js b/cms/static/js/views/video/transcripts/utils.js index 4945d87801..4c2d74fc83 100644 --- a/cms/static/js/views/video/transcripts/utils.js +++ b/cms/static/js/views/video/transcripts/utils.js @@ -110,6 +110,7 @@ define(['jquery', 'underscore', 'jquery.ajaxQueue'], function($) { */ var _videoLinkParser = (function() { var cache = {}; + var maxVideoNameLength = 150; return function(url) { if (typeof url !== 'string') { @@ -129,7 +130,10 @@ define(['jquery', 'underscore', 'jquery.ajaxQueue'], function($) { match = link.pathname.match(/\/{1}([^\/]+)\.([^\/]+)$/); if (match) { cache[url] = { - video: match[1], + /* avoid too long video name, as it will be used as filename for video's transcript + and a filename can not be more that 255 chars, limiting here to 150. + */ + video: match[1].slice(0, maxVideoNameLength), type: match[2] }; } else { @@ -139,7 +143,7 @@ define(['jquery', 'underscore', 'jquery.ajaxQueue'], function($) { match = link.pathname.match(/\/{1}([^\/\.]+)$/); if (match) { cache[url] = { - video: match[1], + video: match[1].slice(0, maxVideoNameLength), type: 'other' }; } diff --git a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py index f464a9d1db..22d6786e3b 100644 --- a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py +++ b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py @@ -296,9 +296,11 @@ def copy_or_rename_transcript(new_name, old_name, item, delete_old=False, user=N def get_html5_ids(html5_sources): """ Helper method to parse out an HTML5 source into the ideas - NOTE: This assumes that '/' are not in the filename + NOTE: This assumes that '/' are not in the filename. + Slices each id by 150, restricting too long strings as video names. """ - html5_ids = [x.split('/')[-1].rsplit('.', 1)[0] for x in html5_sources] + html5_ids = [x.split('/')[-1].rsplit('.', 1)[0][:150] for x in html5_sources] + return html5_ids