Merge pull request #909 from endlessm/T35159-validate-saved-state

collectionviews: Ignore session state if it uses the wrong format

Author: dylanmccall

kolibri_explore_plugin/collectionviews.py

@@ -6,6 +6,7 @@
 import time
 from enum import auto
 from enum import IntEnum
+from threading import RLock
 
 from django.utils.translation import gettext_lazy as _
 from kolibri.core.content.errors import InsufficientStorageSpaceError
@@ -72,6 +73,8 @@
     "completed": 1,
 }
 
+ENSURE_INITIATED_RLOCK = RLock()
+
 
 class ChannelNotImported(Exception):
     pass
@@ -639,6 +642,20 @@ def to_state(self):
 
         return state
 
+    def validate_state(self, state):
+        """Check a serialized representation of this download manager's state.
+
+        Returns True if the state is valid.
+        """
+
+        required_keys = (
+            "collection_name",
+            "collection_sequence",
+            "stage",
+        )
+
+        return all(key in state for key in required_keys)
+
     def _next_task_or_stage(self, user):
         if not self._tasks_pending:
             # No more tasks pending in this stage, move to the next one
@@ -749,8 +766,9 @@ def ensure_initiated(api_function):
     """Decorator to initiate only once in the first API call."""
 
     def wrapper(*args, **kwargs):
-        if _collection_download_manager is None:
-            _read_content_manifests()
+        with ENSURE_INITIATED_RLOCK:
+            if _collection_download_manager is None:
+                _read_content_manifests()
         return api_function(*args, **kwargs)
 
     return wrapper
@@ -766,6 +784,15 @@ def _save_state_in_request_session(request):
     request.session["COLLECTIONS_STATE"] = new_state
 
 
+def _read_state_from_request_session(request):
+    saved_state = request.session.get("COLLECTIONS_STATE", {})
+
+    if _collection_download_manager.validate_state(saved_state):
+        return saved_state
+
+    return None
+
+
 def _get_collections_info_by_name_sequence(name, sequence):
     if name not in _collections_by_name_sequence:
         return None
@@ -843,7 +870,7 @@ def get_all_collections_info(request):
 @api_view(["GET"])
 def get_should_resume(request):
     """Return if there is a saved state that should be resumed."""
-    saved_state = request.session.get("COLLECTIONS_STATE")
+    saved_state = _read_state_from_request_session(request)
     name = None
     sequence = None
     if saved_state is not None:
@@ -881,7 +908,7 @@ def start_download(request):
     collection = _collections_by_name_sequence[name][sequence]
 
     # Fail if a previous download can be resumed
-    saved_state = request.session.get("COLLECTIONS_STATE")
+    saved_state = _read_state_from_request_session(request)
     if saved_state is not None:
         raise APIException("A previous download state was found. Resume it.")
 
@@ -910,15 +937,15 @@ def resume_download(request):
     Returns download status.
     """
 
-    saved_state = request.session.get("COLLECTIONS_STATE")
+    saved_state = _read_state_from_request_session(request)
     if saved_state is None:
         raise APIException("No download state was found. Can't resume.")
 
     # Init the download manager and start downloading
     try:
         _collection_download_manager.from_state(saved_state)
-        name = saved_state["name"]
-        sequence = saved_state["sequence"]
+        name = saved_state["collection_name"]
+        sequence = saved_state["collection_sequence"]
         logger.info(f"Download resumed for name={name} sequence={sequence}")
         logger.info(f"Resumed download state: {saved_state}")
     except DownloadError as err:
@@ -979,7 +1006,7 @@ def cancel_download(request):
 def get_download_status(request):
     """Return the download status."""
 
-    saved_state = request.session.get("COLLECTIONS_STATE")
+    saved_state = _read_state_from_request_session(request)
     if (
         _collection_download_manager.is_state_unset()
         and saved_state is not None