fix: remove server-side path redirect to prevent CDN-cached redirect loops#7
Conversation
…loops The server-side MapFrontendPath redirect was causing infinite redirect loops when CDN (Cloudflare) cached 302 responses for different themes: - User A (theme=default) visits /console → 302 → /dashboard (cached) - User B (theme=classic) visits /dashboard → 302 → /console (cached) - Any user hits /console → cached 302 → /dashboard → cached 302 → /console → loop Fix: Remove the server-side redirect entirely. Instead, always serve the correct theme's index.html for any SPA route. The frontend router will handle navigation to the correct page within its own theme.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe ChangesWeb Router Handler
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request removes the server-side path redirection logic in router/web-router.go. While this change is intended to fix CDN caching loops, it may break automatic path normalization for deep links and bookmarks across different themes. The reviewer suggests restoring the redirection logic but adding a Cache-Control: no-cache header to the response to prevent the CDN from caching the redirect, thereby maintaining functionality while resolving the caching issue.
I am having trouble creating individual review comments. Click here to see my feedback.
router/web-router.go (41-47)
Removing the server-side path redirect entirely fixes the CDN caching loop, but it also breaks the automatic path normalization between themes for deep links and bookmarks. For example, a user with the default theme visiting a /console URL (a classic path) will now stay on /console but be served the default theme's SPA. If the default theme's frontend router is not specifically configured to handle or redirect /console, the user will see a 404.
If the goal is to maintain the current URL behavior while fixing the CDN issue, a better approach would be to keep the redirect but explicitly set the Cache-Control: no-cache header on the 302 response. This prevents the CDN from caching the theme-dependent redirect.
If you prefer to proceed with the removal, please ensure that both the default and classic frontend applications are updated to handle cross-theme path mapping internally.
if redirectPath := common.MapFrontendPath(theme, c.Request.URL.Path); redirectPath != "" && redirectPath != c.Request.URL.Path {
if c.Request.URL.RawQuery != "" {
redirectPath = redirectPath + "?" + c.Request.URL.RawQuery
}
c.Header("Cache-Control", "no-cache")
c.Redirect(http.StatusFound, redirectPath)
return
}
Problem
Server-side \MapFrontendPath\ redirect was causing infinite redirect loops when CDN (Cloudflare) cached 302 responses for different themes:
\
/console → 302 → /dashboard → 302 → /console → ... → ERR_TOO_MANY_REDIRECTS
\\
Root Cause
The redirect response had no \Cache-Control\ header (it returned before the
o-cache\ header was set), so CDN cached it with the default TTL.
Fix
Remove the server-side path redirect entirely. SPA apps don't need server-side redirects — the server should always serve the correct theme's \index.html\ for any route, and let the frontend router handle navigation.
This eliminates:
Summary by CodeRabbit