From e7320b4ec9e736ffbafdbad335c9041aed97212e Mon Sep 17 00:00:00 2001
From: "Lucas.Xu" <lucas.xu@appflowy.io>
Date: Tue, 3 Mar 2026 14:06:51 +0800
Subject: [PATCH] fix: address Sourcery review comments

- Add safeDecodeRedirectParam helper (returns null on malformed input)
  and use it in afterAuth and LoginPage instead of bare decodeURIComponent
- Restore query-param/hash preservation for root-path redirects in afterAuth
- Use immutable URLSearchParams construction in LoginPage setSearch updater
---
 .../session/__tests__/sign_in.test.ts         | 30 +++++++++++++++++--
 src/application/session/sign_in.ts            | 20 +++++++++++--
 src/pages/LoginPage.tsx                       | 16 +++++-----
 3 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/src/application/session/__tests__/sign_in.test.ts b/src/application/session/__tests__/sign_in.test.ts
index c2820400..853e3c20 100644
--- a/src/application/session/__tests__/sign_in.test.ts
+++ b/src/application/session/__tests__/sign_in.test.ts
@@ -1,4 +1,4 @@
-import { afterAuth, isSafeRedirectUrl } from '../sign_in';
+import { afterAuth, isSafeRedirectUrl, safeDecodeRedirectParam } from '../sign_in';
 
 // Mock localStorage
 const localStorageMock = (() => {
@@ -122,8 +122,14 @@ describe('afterAuth', () => {
     expect(window.location.href).toBe('/app');
   });
 
-  it('redirects to /app for root path', () => {
-    localStorage.setItem('redirectTo', encodeURIComponent('http://localhost/'));
+  it('redirects to /app path while preserving query params for root path', () => {
+    localStorage.setItem('redirectTo', encodeURIComponent('http://localhost/?foo=bar'));
+    afterAuth();
+    expect(window.location.href).toBe('http://localhost/app?foo=bar');
+  });
+
+  it('redirects to /app when stored redirectTo has malformed encoding', () => {
+    localStorage.setItem('redirectTo', '%zz');
     afterAuth();
     expect(window.location.href).toBe('/app');
   });
@@ -152,3 +158,21 @@ describe('afterAuth', () => {
     expect(localStorage.getItem('redirectTo')).toBeNull();
   });
 });
+
+describe('safeDecodeRedirectParam', () => {
+  it('decodes a valid percent-encoded string', () => {
+    expect(safeDecodeRedirectParam('https%3A%2F%2Fevil.com')).toBe('https://evil.com');
+  });
+
+  it('returns the string unchanged when nothing is encoded', () => {
+    expect(safeDecodeRedirectParam('/settings')).toBe('/settings');
+  });
+
+  it('returns null for a malformed percent-encoded sequence', () => {
+    expect(safeDecodeRedirectParam('%zz')).toBeNull();
+  });
+
+  it('returns null for a lone percent sign', () => {
+    expect(safeDecodeRedirectParam('%')).toBeNull();
+  });
+});
diff --git a/src/application/session/sign_in.ts b/src/application/session/sign_in.ts
index fd3c62d3..f4bd54be 100644
--- a/src/application/session/sign_in.ts
+++ b/src/application/session/sign_in.ts
@@ -40,6 +40,18 @@ export function withSignIn() {
   };
 }
 
+/**
+ * Decodes a percent-encoded redirect parameter, returning null on malformed input
+ * so that bad values are always treated as unsafe rather than crashing.
+ */
+export function safeDecodeRedirectParam(value: string): string | null {
+  try {
+    return decodeURIComponent(value);
+  } catch {
+    return null;
+  }
+}
+
 /**
  * Returns true only if the URL is safe to redirect to after authentication.
  * Safe means: a relative path (starts with "/" but NOT "//") OR
@@ -69,9 +81,9 @@ export function afterAuth() {
   clearRedirectTo();
 
   if (redirectTo) {
-    const decoded = decodeURIComponent(redirectTo);
+    const decoded = safeDecodeRedirectParam(redirectTo);
 
-    if (!isSafeRedirectUrl(decoded)) {
+    if (!decoded || !isSafeRedirectUrl(decoded)) {
       window.location.href = '/app';
       return;
     }
@@ -87,7 +99,9 @@ export function afterAuth() {
       // Don't redirect to user-specific pages from previous sessions
       window.location.href = '/app';
     } else if (pathname === '/' || !pathname) {
-      window.location.href = '/app';
+      // Preserve query params and hash but redirect to /app path
+      url.pathname = '/app';
+      window.location.href = url.toString();
     } else {
       window.location.href = decoded;
     }
diff --git a/src/pages/LoginPage.tsx b/src/pages/LoginPage.tsx
index a00218af..7fbc9004 100644
--- a/src/pages/LoginPage.tsx
+++ b/src/pages/LoginPage.tsx
@@ -9,7 +9,7 @@ import { LOGIN_ACTION } from '@/components/login/const';
 import { EnterPassword } from '@/components/login/EnterPassword';
 import { ForgotPassword } from '@/components/login/ForgotPassword';
 import { SignUpPassword } from '@/components/login/SignUpPassword';
-import { isSafeRedirectUrl } from '@/application/session/sign_in';
+import { isSafeRedirectUrl, safeDecodeRedirectParam } from '@/application/session/sign_in';
 import { AFConfigContext } from '@/components/main/app.hooks';
 
 function LoginPage() {
@@ -26,12 +26,14 @@ function LoginPage() {
   useEffect(() => {
     if (!redirectTo) return;
 
-    const decodedRedirect = decodeURIComponent(redirectTo);
+    const decodedRedirect = safeDecodeRedirectParam(redirectTo);
 
-    if (!isSafeRedirectUrl(decodedRedirect)) {
+    if (!decodedRedirect || !isSafeRedirectUrl(decodedRedirect)) {
       setSearch((prev) => {
-        prev.delete('redirectTo');
-        return prev;
+        const next = new URLSearchParams(prev);
+
+        next.delete('redirectTo');
+        return next;
       });
     }
   }, [redirectTo, setSearch]);
@@ -42,9 +44,9 @@ function LoginPage() {
     }
 
     if (isAuthenticated && redirectTo) {
-      const decodedRedirect = decodeURIComponent(redirectTo);
+      const decodedRedirect = safeDecodeRedirectParam(redirectTo);
 
-      if (decodedRedirect !== window.location.href) {
+      if (decodedRedirect && decodedRedirect !== window.location.href) {
         window.location.href = decodedRedirect;
       }
     }