Skip to content

fix: URL-encode pad names in admin 'Open' button and recent pads (#7865)#7895

Open
JohnMcLear wants to merge 1 commit into
developfrom
fix/admin-open-pad-url-encode
Open

fix: URL-encode pad names in admin 'Open' button and recent pads (#7865)#7895
JohnMcLear wants to merge 1 commit into
developfrom
fix/admin-open-pad-url-encode

Conversation

@JohnMcLear
Copy link
Copy Markdown
Member

Description

Fixes #7865 — Admin Dashboard "Open" button does not URL-encode slashes in pad names, causing broken navigation for pads like Test/123.

Changes

admin/src/pages/PadPage.tsx — Added encodeURIComponent() around pad.padName in the "Open" button URL. No formatting changes.

src/static/js/pad_userlist.ts — Added decodeURIComponent() when reading pad name from URL pathname for recent pads member-count matching.

src/static/skins/colibris/pad.js — Added decodeURIComponent() when reading pad name from URL pathname before storing in localStorage.

src/static/skins/colibris/index.js — Added encodeURIComponent() in the recent pads href. Display text (link.innerText) uses pad.name directly — no double-decode (avoids URIError on names containing literal %).

Tests

src/tests/frontend-new/specs/recent_pads.spec.ts (new) — Verifies recent pads links have properly encoded URLs while showing the decoded name.

src/tests/frontend-new/specs/embed_value.spec.ts — Added share dialog test asserting special characters (e.g. %2F) are preserved in the share link.

Notes

No formatting/whitespace changes anywhere. No unrelated files touched.

- encodeURIComponent in admin PadPage 'Open' button href
- decodeURIComponent when reading pad name from URL pathname
  in pad_userlist.ts and colibris/pad.js (recent pads storage)
- encodeURIComponent in colibris/index.js recent pads href;
  display text uses stored name directly (no double-decode)
- add recent_pads spec asserting encoded URLs
- add share dialog spec asserting URL encoding of special chars
@qodo-code-review
Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Fix URL encoding of pad names in admin and recent pads

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• URL-encode pad names in admin dashboard "Open" button
• Decode pad names from URL pathname for recent pads
• Encode pad names in recent pads links and localStorage
• Add tests for URL encoding of special characters
Diagram
flowchart LR
  A["Pad Name with Special Chars"] -->|encodeURIComponent| B["Admin Open Button"]
  A -->|encodeURIComponent| C["Recent Pads Link"]
  D["URL Pathname"] -->|decodeURIComponent| E["Pad Name for Matching"]
  E --> F["localStorage Storage"]
  G["Share Dialog"] -->|encodeURIComponent| H["Share Link"]

Loading

Grey Divider

File Changes

1. admin/src/pages/PadPage.tsx 🐞 Bug fix +1/-1

Encode pad name in admin Open button

• Wrapped pad.padName with encodeURIComponent() in the "Open" button href
• Ensures special characters like slashes are properly encoded in navigation URLs

admin/src/pages/PadPage.tsx


2. src/static/js/pad_userlist.ts 🐞 Bug fix +1/-1

Decode pad name from URL pathname

• Added decodeURIComponent() when extracting pad name from URL pathname
• Ensures proper matching of recent pads with encoded names in localStorage

src/static/js/pad_userlist.ts


3. src/static/skins/colibris/pad.js 🐞 Bug fix +1/-1

Decode pad name before localStorage storage

• Added decodeURIComponent() when reading pad name from URL pathname
• Ensures correct pad name is stored in localStorage for recent pads

src/static/skins/colibris/pad.js


View more (3)
4. src/static/skins/colibris/index.js 🐞 Bug fix +1/-1

Encode pad name in recent pads link

• Wrapped pad.name with encodeURIComponent() in recent pads href
• Display text uses decoded name directly to avoid double-decoding errors

src/static/skins/colibris/index.js


5. src/tests/frontend-new/specs/recent_pads.spec.ts 🧪 Tests +30/-0

Add recent pads URL encoding test

• New test file verifying recent pads links have properly encoded URLs
• Tests that decoded pad names are displayed while URLs contain encoded names
• Covers special characters including spaces, ampersands, and slashes

src/tests/frontend-new/specs/recent_pads.spec.ts


6. src/tests/frontend-new/specs/embed_value.spec.ts 🧪 Tests +13/-0

Add share link URL encoding test

• Added new test for URL encoding of special characters in share links
• Verifies that special characters like %2F are preserved in share dialog

src/tests/frontend-new/specs/embed_value.spec.ts


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented Jun 4, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

Context used

Grey Divider


Action required

1. Recent pads double-encoding 🐞 Bug ≡ Correctness
Description
colibris recent pad links now apply encodeURIComponent(pad.name) to values loaded from
recentPads, so pre-existing entries that already contain percent-escapes (e.g. Test%2F123) will
become double-encoded (%252F) and open the wrong pad. This is triggered by the storage format
changing to decoded names in colibris/pad.js without migrating older stored values.
Code

src/static/skins/colibris/index.js[73]

+      const padPath = `${window.location.href}p/${encodeURIComponent(pad.name)}`;
Evidence
index.js encodes pad.name directly from localStorage when building padPath, while pad.js now
stores a decoded padName into localStorage; without normalizing older stored values,
already-encoded names will be encoded again and produce the wrong URL.

src/static/skins/colibris/index.js[33-79]
src/static/skins/colibris/pad.js[10-34]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`recentPads` entries from older versions can already be URL-encoded. The new code encodes `pad.name` again when building the href, producing double-encoded URLs and broken navigation.

## Issue Context
- `colibris/pad.js` now stores a decoded pad name.
- `colibris/index.js` encodes whatever is stored in localStorage when generating recent pad URLs.
- Existing localStorage data is not migrated/normalized.

## Fix
When reading `recentPads` from localStorage in `colibris/index.js`, normalize each entry’s name to a decoded form using a safe decode (try/catch). Then:
- Use the normalized (decoded) name for `link.innerText`.
- Use `encodeURIComponent(normalizedName)` for the href.
- Optionally rewrite localStorage once with normalized names so the fix is persistent and deduping works consistently.

## Fix Focus Areas
- src/static/skins/colibris/index.js[34-79]
- src/static/skins/colibris/pad.js[10-34]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Missing noopener on _blank 🐞 Bug ⛨ Security
Description
The Admin Pads “Open” button uses window.open(..., '_blank') without noopener,noreferrer,
allowing the opened page to access window.opener and potentially redirect the admin tab (reverse
tabnabbing). The codebase already uses noopener,noreferrer for safe _blank opens elsewhere.
Code

admin/src/pages/PadPage.tsx[421]

+                            onClick={() => window.open(`../../p/${encodeURIComponent(pad.padName)}`, '_blank')}
Evidence
PadPage.tsx uses _blank without noopener, while ace2_inner.ts demonstrates the established
safe pattern within this repo.

admin/src/pages/PadPage.tsx[419-423]
src/static/js/ace2_inner.ts[2284-2296]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`window.open(url, '_blank')` without `noopener` allows the new page to control `window.opener`, enabling reverse tabnabbing if the target is compromised or redirected.

## Issue Context
The repo already mitigates this risk in other UI code by passing `'noopener,noreferrer'` as the third argument to `window.open`.

## Fix
Update the Admin Pads “Open” handler to:
- `window.open(url, '_blank', 'noopener,noreferrer')`
Optionally, for extra defense-in-depth, set `newWindow.opener = null` if a window handle is returned.

## Fix Focus Areas
- admin/src/pages/PadPage.tsx[419-423]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo


li.className = 'recent-pad';
const padPath = `${window.location.href}p/${pad.name}`;
const padPath = `${window.location.href}p/${encodeURIComponent(pad.name)}`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Recent pads double-encoding 🐞 Bug ≡ Correctness

colibris recent pad links now apply encodeURIComponent(pad.name) to values loaded from
recentPads, so pre-existing entries that already contain percent-escapes (e.g. Test%2F123) will
become double-encoded (%252F) and open the wrong pad. This is triggered by the storage format
changing to decoded names in colibris/pad.js without migrating older stored values.
Agent Prompt
## Issue description
`recentPads` entries from older versions can already be URL-encoded. The new code encodes `pad.name` again when building the href, producing double-encoded URLs and broken navigation.

## Issue Context
- `colibris/pad.js` now stores a decoded pad name.
- `colibris/index.js` encodes whatever is stored in localStorage when generating recent pad URLs.
- Existing localStorage data is not migrated/normalized.

## Fix
When reading `recentPads` from localStorage in `colibris/index.js`, normalize each entry’s name to a decoded form using a safe decode (try/catch). Then:
- Use the normalized (decoded) name for `link.innerText`.
- Use `encodeURIComponent(normalizedName)` for the href.
- Optionally rewrite localStorage once with normalized names so the fix is persistent and deduping works consistently.

## Fix Focus Areas
- src/static/skins/colibris/index.js[34-79]
- src/static/skins/colibris/pad.js[10-34]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@JohnMcLear JohnMcLear requested a review from SamTV12345 June 4, 2026 15:04
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.

Bug: Admin Dashboard “Open” button does not URL‑encode slashes in pad names

1 participant