search

xtermjs / xterm.js

A terminal for the web
https://xtermjs.org/
MIT License
17.33k stars 1.61k forks source link

`;` inside the URL breaks the OSC hyperlink #4944

Closed schaffino closed 4 months ago

schaffino commented 7 months ago

Tried this in both VSCode and Jupyterlab. It seems the OSC hyperlink integration doesn't work properly when the linked URL contains a ';'

The following submitted to terminator terminal emulator results in a link directing me to https://www.example.com/query;arg=1;arg=2;arg=3

printf '\e]8;;https://www.example.com/query;arg=1;arg=2;arg=3\e\\This is a link\e]8;;\e\\n'

Running the same in jupyter or VSCode, I end up being redirected to https://www.example.com/query, with the arguments truncated

Details

Steps to reproduce

  1. Output a valid OSC hyperlink in tooling using xterm.js (See printf statement above)
  2. Observe the link is truncated
Tyriar commented 6 months ago

Before fixing this we should verify that ; is expected to be supported when not URL encoded as %3B

jerch commented 6 months ago

@Tyriar https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda#encodings states, that additional uri params may not contain : or ; due to its ambiguity with the sequence syntax. Thus I'd say this works as intended by the spec. And an error recovery strategy for excess parts separated by additional ; is not defined, which makes it undefined behavior.

Tyriar commented 6 months ago

Cool, I suspected that might be the case. Closing as designed then

schaffino commented 6 months ago

@Tyriar @jerch The parameters its referencing in the doc above is referring to the optional parameter argument of the OSC-8 hyperlink, not query parameters in the URI. It states that only id is specifically defined in the spec for this argument. My reading is that the URI can still be any string in the range 32–126. Given most other terminal emulators support this i believe this is still a bug.

https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda#the-escape-sequence

Without support for ; there are plenty of valid URI's that cannot be linked

Tyriar commented 6 months ago

@schaffino makes sense, thanks for the follow up. I read the "additional parameters" part wrong.

jerch commented 6 months ago

Eww, seems I also misread that.

josiahhudson commented 5 months ago

Attempted a fix here: https://github.com/xtermjs/xterm.js/pull/5003