Skip to content

fixed Bug: Admin Dashboard “Open” button does not URL‑encode slashes in pad names (#7865).#7876

Open
abhijeetgorhe26 wants to merge 5 commits into
ether:developfrom
abhijeetgorhe26:abhijeetgorhe-7865
Open

fixed Bug: Admin Dashboard “Open” button does not URL‑encode slashes in pad names (#7865).#7876
abhijeetgorhe26 wants to merge 5 commits into
ether:developfrom
abhijeetgorhe26:abhijeetgorhe-7865

Conversation

@abhijeetgorhe26
Copy link
Copy Markdown

@abhijeetgorhe26 abhijeetgorhe26 commented May 29, 2026

I solved that bug (#7865). Whenever someone types their name as Test/123 at padName, then only the URL will become ..p/Test%2F123, and the entry in recent Pads will remain the same as the typed name (i.e., Test/123).

### Changes verified

1) Working demo video
https://github.com/user-attachments/assets/b3c4f5f5-625f-40d6-84dd-7941daca499f

2) URL encoding working correctly
Screenshot 2026-05-30 at 12 29 06 AM

3) Pad name remains unchanged in Recent Pads list
Screenshot 2026-05-30 at 12 28 15 AM

fixes #7865

@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 with special characters

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• URL-encode pad names with slashes in admin dashboard "Open" button
• Decode URL-encoded pad names when retrieving from pathname
• Display decoded pad names in recent pads list and UI
• Apply consistent formatting to admin PadPage component
Diagram
flowchart LR
  A["Pad Name with Slash<br/>Test/123"] -->|encodeURIComponent| B["URL Encoded<br/>Test%2F123"]
  B -->|Navigate| C["Browser URL<br/>...p/Test%2F123"]
  C -->|decodeURIComponent| D["Display Name<br/>Test/123"]
  D -->|Store| E["Recent Pads List<br/>Test/123"]

Loading

Grey Divider

File Changes

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

Decode pad name from URL pathname

• Decode URL-encoded pad name from pathname using decodeURIComponent()
• Ensures pad name matches correctly when stored in recent pads list
• Fixes issue where URL-encoded names wouldn't match stored names

src/static/js/pad_userlist.ts


2. src/static/skins/colibris/index.js 🐞 Bug fix +3/-2

Encode/decode pad names in recent pads UI

• Decode pad name before displaying in recent pads list
• Re-encode pad name when constructing navigation URL
• Display human-readable pad name in UI while maintaining proper URL encoding

src/static/skins/colibris/index.js


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

Decode pad name from URL pathname

• Decode URL-encoded pad name from pathname
• Ensures correct pad name is stored in localStorage for recent pads

src/static/skins/colibris/pad.js


View more (2)
4. admin/src/pages/PadPage.tsx Bug fix, formatting +83/-83

Encode pad names in admin dashboard open button

• URL-encode pad name in "Open" button navigation URL using encodeURIComponent()
• Apply consistent spacing formatting to JSX attributes throughout file
• Standardize object literal and function parameter formatting

admin/src/pages/PadPage.tsx


5. var/update-state.json ⚙️ Configuration changes +26/-0

Add update state tracking configuration file

• Add new update state tracking file with rollback-failed status
• Track execution state, boot count, and last result information
• Initialize schema version and email notification state

var/update-state.json


Grey Divider

Qodo Logo

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

qodo-free-for-open-source-projects Bot commented May 29, 2026

Code Review by Qodo

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

Grey Divider


Action required

1. Recent pads double decode ✓ Resolved 🐞 Bug ≡ Correctness
Description
src/static/skins/colibris/index.js calls decodeURIComponent(pad.name) even though pad.name was
already decoded before being persisted to localStorage, which can throw URIError for names
containing '%' and can mutate pad IDs that contain percent-escape sequences (e.g. "abc%2Fdef"
becomes "abc/def"). This can break the recent pads UI and generate links to the wrong pad.
Code

src/static/skins/colibris/index.js[R73-79]

Evidence
The renderer decodes pad.name from localStorage, but the writer already stores a decoded pad name,
so this is a second decode pass with the failure/corruption modes described.

src/static/skins/colibris/index.js[66-80]
src/static/skins/colibris/pad.js[10-27]

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

## Issue description
The Colibris recent pads renderer decodes `pad.name` even though `pad.name` is already stored as a decoded string. This can:
- throw `URIError` when `pad.name` contains a literal `%` (e.g. pad named `100% legit`), breaking the page;
- corrupt pad IDs when the literal pad name contains percent-escape sequences (e.g. `abc%2Fdef`), causing the link to open a different pad.
### Issue Context
`src/static/skins/colibris/pad.js` stores `name: padName` where `padName` is derived via `decodeURIComponent()` from the URL segment, so localStorage already holds the decoded representation.
### Fix Focus Areas
- src/static/skins/colibris/index.js[73-80]
### What to change
- Do **not** call `decodeURIComponent()` on `pad.name` in index.js.
- Use the stored value directly for display (`link.innerText = pad.name`).
- Encode exactly once when building the URL: `.../p/${encodeURIComponent(pad.name)}`.
(If you want to support legacy localStorage entries that might be encoded, do not blindly decode: it can corrupt literal `%2F` pad names. Prefer leaving legacy entries as-is, or add a migration that only decodes when you can prove the value was produced by `encodeURIComponent()`.)

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


2. Committed updater state file ✓ Resolved 🐞 Bug ☼ Reliability
Description
The PR adds var/update-state.json containing a non-empty execution state (status:
"rollback-failed"); because the updater loads settings.root/var/update-state.json at runtime, this
will cause fresh installs/dev runs to start in a poisoned failure state and surface stale
operational data. The file is not ignored by var/.gitignore, so it is likely to be shipped/packaged
unintentionally.
Code

var/update-state.json[R1-26]

Evidence
The newly added file contains a rollback-failed state, and the updater explicitly loads that exact
path at runtime; var/.gitignore does not exclude it, making accidental shipping likely.

var/update-state.json[2-25]
src/node/updater/index.ts[35-41]
var/.gitignore[1-5]

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

## Issue description
A runtime-generated updater state file (`var/update-state.json`) was committed with a rollback-failed status. This file is used as the updater’s persisted state and should not be tracked in git.
### Issue Context
`stateFilePath()` points to `<settings.root>/var/update-state.json` and `getCurrentState()` loads it on first use, so any committed file directly affects runtime updater behavior.
### Fix Focus Areas
- var/update-state.json[1-26]
- var/.gitignore[1-5]
### What to change
1. Delete `var/update-state.json` from the repository.
2. Add `update-state.json` (and optionally `update-state.json.tmp`) to `var/.gitignore` so it cannot be re-committed.
3. Ensure any tests that require state use temp directories (they already do), not the repo’s `var/` directory.

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


Grey Divider

Qodo Logo

Comment thread src/static/skins/colibris/index.js Outdated
Comment thread var/update-state.json Outdated
@JohnMcLear
Copy link
Copy Markdown
Member

JohnMcLear commented May 29, 2026

@abhijeetgorhe26
Copy link
Copy Markdown
Author

@JohnMcLear Now see with least changes

@JohnMcLear
Copy link
Copy Markdown
Member

The core one-line fix in PadPage.tsx is correct and solves the issue. The recent-pads changes are reasonable
in intent but introduce a real edge-case regression (double-decode throwing on %-containing names) and bundle an unrelated .gitignore change.

a. Drop the var/.gitignore change.
b. In index.js, use pad.name directly for innerText and encodeURIComponent(pad.name) for the href (no extra decode or just guard).
c. Add a test asserting the encoded URL.

See point 7 - regression tests are important and your agents.md should have picked that up... Any reason it didn't?

@abhijeetgorhe26
Copy link
Copy Markdown
Author

abhijeetgorhe26 commented May 30, 2026

@JohnMcLear I make changes as you suggested to me.

a. I dropped the var/.gitignore change.
b. I used pad.name directly for innerText and encodeURIComponent(pad.name) for the href.
c. I added a test asserting the encoded URL.

I did regression test and it gave me result:

  1. 1512 passing
  2. 22 pending.

@JohnMcLear
Copy link
Copy Markdown
Member

@JohnMcLear
Copy link
Copy Markdown
Member

I think test coverage for the URL in the share dialog is missing.

@abhijeetgorhe26
Copy link
Copy Markdown
Author

@JohnMcLear According to your suggestions i done everything

@JohnMcLear
Copy link
Copy Markdown
Member

Test missing for the share dialog

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

2 participants