From f54b9a42e710d37e84c8a8df32bf54a3f68af34a Mon Sep 17 00:00:00 2001 From: Waheed Ahmed Date: Wed, 11 Mar 2020 15:51:25 +0500 Subject: [PATCH] Disallow NaN value for speed key. The VideoBlock `handle_ajax` is allowing NaN values for speed key and causing videos to not load. Also added a data migration to fix the data for learners. PROD-1148 --- .../xmodule/video_module/video_handlers.py | 6 +++++ .../0014_fix_nan_value_for_global_speed.py | 25 +++++++++++++++++++ .../courseware/tests/test_video_handlers.py | 18 +++++++++++++ 3 files changed, 49 insertions(+) create mode 100644 lms/djangoapps/courseware/migrations/0014_fix_nan_value_for_global_speed.py diff --git a/common/lib/xmodule/xmodule/video_module/video_handlers.py b/common/lib/xmodule/xmodule/video_module/video_handlers.py index 0be32e3081..bde727a7ea 100644 --- a/common/lib/xmodule/xmodule/video_module/video_handlers.py +++ b/common/lib/xmodule/xmodule/video_module/video_handlers.py @@ -8,6 +8,7 @@ StudioViewHandlers are handlers for video descriptor instance. import json import logging +import math import six from django.core.files.base import ContentFile @@ -89,6 +90,11 @@ class VideoStudentViewHandlers(object): if key == 'bumper_last_view_date': value = now() + if key == 'speed' and math.isnan(value): + message = u"Invalid speed value {}, must be a float.".format(value) + log.warning(message) + return json.dumps({'success': False, 'error': message}) + setattr(self, key, value) if key == 'speed': diff --git a/lms/djangoapps/courseware/migrations/0014_fix_nan_value_for_global_speed.py b/lms/djangoapps/courseware/migrations/0014_fix_nan_value_for_global_speed.py new file mode 100644 index 0000000000..e509936553 --- /dev/null +++ b/lms/djangoapps/courseware/migrations/0014_fix_nan_value_for_global_speed.py @@ -0,0 +1,25 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.29 on 2020-03-11 10:07 +from __future__ import unicode_literals + +from django.db import migrations + + +def fix_nan_value_for_global_speed(apps, schema_editor): + XModuleStudentPrefsField = apps.get_model('courseware', 'XModuleStudentPrefsField') + XModuleStudentPrefsField.objects.filter( + field_name='global_speed', value='NaN' + ).update( + value=1.0 + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ('courseware', '0013_auto_20191001_1858'), + ] + + operations = [ + migrations.RunPython(fix_nan_value_for_global_speed, reverse_code=migrations.RunPython.noop) + ] diff --git a/lms/djangoapps/courseware/tests/test_video_handlers.py b/lms/djangoapps/courseware/tests/test_video_handlers.py index 5c66b4b081..736006cc34 100644 --- a/lms/djangoapps/courseware/tests/test_video_handlers.py +++ b/lms/djangoapps/courseware/tests/test_video_handlers.py @@ -165,6 +165,24 @@ class TestVideo(BaseTestVideoXBlock): status_codes = {response.status_code for response in responses.values()} self.assertEqual(status_codes.pop(), 404) + def test_handle_ajax_for_speed_with_nan(self): + self.item_descriptor.handle_ajax('save_user_state', {'speed': json.dumps(1.0)}) + self.assertEqual(self.item_descriptor.speed, 1.0) + self.assertEqual(self.item_descriptor.global_speed, 1.0) + + # try to set NaN value for speed. + response = self.item_descriptor.handle_ajax( + 'save_user_state', {'speed': json.dumps(float('NaN'))} + ) + + self.assertFalse(json.loads(response)['success']) + expected_error = u"Invalid speed value nan, must be a float." + self.assertEqual(json.loads(response)['error'], expected_error) + + # verify that the speed and global speed are still 1.0 + self.assertEqual(self.item_descriptor.speed, 1.0) + self.assertEqual(self.item_descriptor.global_speed, 1.0) + def test_handle_ajax(self): data = [