From a62ccac9c5c8d6b80dc3d669d40ee69c83e91842 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Wed, 7 May 2025 14:03:28 -0600
Subject: [PATCH 1/3] Protect against nullish values stored for the suggestion
 list

In theory, this shouldn't happen because you can't select a reviewer
    (which is when this function is called - post-selection) without
    already having opened the reviewer dropdown, which would populate
    the storage location. However, if something strange happens (or
    someone is intentionally trying to cause an error, which would be
    harmless in this case), this will protect against that.
---
 .../merge_requests/utils/reviewer_positions.js           | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/app/assets/javascripts/merge_requests/utils/reviewer_positions.js b/app/assets/javascripts/merge_requests/utils/reviewer_positions.js
index ce07c3c6e81b91..e349c551bc2f25 100644
--- a/app/assets/javascripts/merge_requests/utils/reviewer_positions.js
+++ b/app/assets/javascripts/merge_requests/utils/reviewer_positions.js
@@ -11,8 +11,15 @@ export function setReviewersForList({ issuableId, listId, reviewers = [] } = {})
 export function getReviewersForList({ issuableId, listId } = {}) {
   const id = cacheId({ issuableId, listId });
   const list = window.sessionStorage.getItem(id);
+  let parsedList;
 
-  return list ? JSON.parse(list) : [];
+  try {
+    parsedList = JSON.parse(list);
+  } catch {
+    parsedList = [];
+  }
+
+  return parsedList;
 }
 
 export function suggestedPosition({ username, list = [] } = {}) {
-- 
GitLab


From 3d3506ca6608b8798481c8d93f892579f4fa2e03 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Wed, 7 May 2025 15:55:48 -0600
Subject: [PATCH 2/3] JSON.parse can "properly" parse non-array values

---
 .../javascripts/merge_requests/utils/reviewer_positions.js      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/assets/javascripts/merge_requests/utils/reviewer_positions.js b/app/assets/javascripts/merge_requests/utils/reviewer_positions.js
index e349c551bc2f25..ec9bfa66ee9d8a 100644
--- a/app/assets/javascripts/merge_requests/utils/reviewer_positions.js
+++ b/app/assets/javascripts/merge_requests/utils/reviewer_positions.js
@@ -14,7 +14,7 @@ export function getReviewersForList({ issuableId, listId } = {}) {
   let parsedList;
 
   try {
-    parsedList = JSON.parse(list);
+    parsedList = list ? JSON.parse(list) : [];
   } catch {
     parsedList = [];
   }
-- 
GitLab


From d2ae0e1c924f49161e4738ef7ee4669ffd79bc05 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Wed, 7 May 2025 17:01:36 -0600
Subject: [PATCH 3/3] Test when the data is not parseable as JSON

---
 .../merge_requests/utils/reviewer_positions_spec.js    | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/spec/frontend/merge_requests/utils/reviewer_positions_spec.js b/spec/frontend/merge_requests/utils/reviewer_positions_spec.js
index 0ab3c14c85f08f..ecb1c59fd86e81 100644
--- a/spec/frontend/merge_requests/utils/reviewer_positions_spec.js
+++ b/spec/frontend/merge_requests/utils/reviewer_positions_spec.js
@@ -17,6 +17,7 @@ describe('reviewer_positions utility', () => {
       getItem: jest.fn().mockImplementation((key) => {
         const vals = {
           'MergeRequest/123/test-list-id': mockReviewersString,
+          'MergeRequest/123/invalid-data': '#',
         };
 
         return vals[key];
@@ -64,6 +65,15 @@ describe('reviewer_positions utility', () => {
 
       expect(result).toEqual([]);
     });
+
+    it('returns an empty array when the data in storage is not a valid JSON string', () => {
+      const result = getReviewersForList({
+        issuableId: mockIssuableId,
+        listId: 'invalid-data',
+      });
+
+      expect(result).toEqual([]);
+    });
   });
 
   describe('suggestedPosition', () => {
-- 
GitLab