Skip to content

[ZEPPELIN-6495] Use URLSearchParams in terminal getParams helper#5294

Open
kimyenac wants to merge 1 commit into
apache:masterfrom
kimyenac:ZEPPELIN-6495
Open

[ZEPPELIN-6495] Use URLSearchParams in terminal getParams helper#5294
kimyenac wants to merge 1 commit into
apache:masterfrom
kimyenac:ZEPPELIN-6495

Conversation

@kimyenac

Copy link
Copy Markdown
Contributor

What is this PR for?

The %sh.terminal frontend helper getParams(key) in shell/src/main/resources/html/js/index.js parsed the query string by hand: it built a RegExp over location.search, sliced it with substr(1), and decoded matches with the deprecated global unescape(...). This is harder to read than the platform API and can diverge from standard URL parsing for encoded/repeated/special characters.

This PR replaces that logic with the standard URLSearchParams API. URLSearchParams.get(key) returns null for missing keys, so the existing behavior for the noteId/paragraphId parameters consumed by the terminal page is preserved, while relying on standard URL decoding instead of the deprecated unescape.

function getParams(key) {
    var params = new URLSearchParams(location.search);
    return params.get(key);
}

The noteId/paragraphId/t values the server generates (see TerminalInterpreter#createTerminalDashboard) are plain alphanumeric IDs and a numeric timestamp, so decoded values are identical to the previous implementation for all real inputs.

What type of PR is it?

Refactoring

Todos

  • - Replace substr(...)/unescape(...) in getParams with URLSearchParams

What is the Jira issue?

How should this be tested?

  • The changed file is a static, vendored frontend resource (index.js); the shell module has no JavaScript test harness (no package.json, and the existing TerminalInterpreterTest covers the Java interpreter, not this script), so no automated JS test is added.
  • Behavior was verified equivalent to the previous implementation:
    • encoded value (e.g. %2F) → decoded (/)
    • missing key → null
    • empty value (key=) → ""
  • Optional manual check: open a %sh.terminal paragraph so the terminal dashboard loads ...?noteId=<id>&paragraphId=<id>&t=<ts>, and confirm the terminal connects and TERMINAL_READY carries the correct noteId/paragraphId.

Screenshots (if appropriate)

N/A

Questions:

  • Does the license files need to update? No — the file already carries the Apache License 2.0 header and no new file is added.
  • Is there breaking changes for older versions? No.
  • Does this needs documentation? No.

Replace the hand-rolled location.search.substr(1).match(...) parser and
the deprecated unescape(...) call in the %sh.terminal frontend helper with
the standard URLSearchParams API.

URLSearchParams.get(key) returns null for missing keys, preserving the
existing behavior for the noteId/paragraphId parameters consumed by the
terminal page, while relying on standard URL decoding instead of the
deprecated global unescape.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@ParkGyeongTae ParkGyeongTae left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. The replacement is correct and the behavior is preserved for all real inputs:

  • URLSearchParams.get() returns null for missing keys, matching the previous fallback
  • noteId/paragraphId values are server-generated alphanumeric IDs, so the unescape()URLSearchParams decoding difference has no practical effect

@ParkGyeongTae ParkGyeongTae self-assigned this Jul 13, 2026
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.

2 participants