Skip to content

Conversation

@gunjambi
Copy link

@gunjambi gunjambi commented Dec 26, 2025

getLoc has an off-by-one where reverse proxy path gets parsed wrong if it's routed on the top-level path.

I have WLED in domain.com/wled/. On settings page we compute:

let path = l.pathname; // `/wled/settings`
let paths = path.slice(1,path.endsWith('/')?-1:undefined).split("/");   // [ "wled", "settings" ]
if (paths.length > 1) paths.pop(); // [ "wled" ]
if (paths.length > 0 && paths[paths.length-1]=="settings") paths.pop(); // No change

// paths.length == 1 so this isn't taken
if (paths.length > 0) {
	locproto = l.protocol;
	loc = true;
	locip = l.hostname + (l.port ? ":" + l.port : "") + "/" + paths.join('/');
}

This then breaks the computation for the "s.js" path.

If I have WLED in domain.com/w/led/ the above paths.length > 0 is taken as expected and settings work.

This PR fixes the off-by-one

Summary by CodeRabbit

  • Bug Fixes
    • Improved URL path handling for reverse-proxy configurations to enable localization settings to work with single-segment paths, in addition to the previously supported multi-segment paths.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 26, 2025

Walkthrough

The condition in getLoc() for populating localization data is adjusted from requiring more than one path segment to requiring at least one path segment, expanding when reverse-proxy localization logic triggers.

Changes

Cohort / File(s) Summary
Localization Path Logic
wled00/data/common.js
Modified getLoc() condition from paths.length > 1 to paths.length > 0, expanding when localization data (loc, locip, locproto) are populated for reverse-proxy configurations

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix reverse proxying in Settings' directly addresses the main change: fixing getLoc() logic for correct reverse-proxy path parsing in the Settings page.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8a2a705 and 23517ea.

📒 Files selected for processing (1)
  • wled00/data/common.js
🧰 Additional context used
📓 Path-based instructions (2)
wled00/data/**/*.{htm,html,css,js}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use tabs for indentation in web files (.htm/.html/.css/.js) under wled00/data

Files:

  • wled00/data/common.js
wled00/data/**

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

wled00/data/**: When modifying web UI files, run npm run build to regenerate embedded headers before any firmware build
For web UI changes, edit files only under wled00/data (not firmware or generated files)

Files:

  • wled00/data/common.js
🧠 Learnings (1)
📓 Common learnings
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR #5134, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.
🧬 Code graph analysis (1)
wled00/data/common.js (1)
wled00/data/index.js (1)
  • paths (231-231)
🔇 Additional comments (1)
wled00/data/common.js (1)

106-127: LGTM! The reverse proxy detection logic is sound.

The getLoc() function correctly handles reverse proxy paths at various nesting levels. For top-level reverse proxy paths like /wled/settings, after line 119-120 processes the path segments, the remaining segment (e.g., ["wled"]) properly triggers the reverse proxy localization via the paths.length > 0 check on line 121. The logic doesn't introduce regressions for non-reverse-proxy scenarios where paths becomes empty. Indentation uses tabs as required by coding guidelines.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant