microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
163.61k stars 29.03k forks source link

file://// URL not recognised #139702

Open yecril71pl opened 2 years ago

yecril71pl commented 2 years ago

Issue Type: Bug

  1. Control-Press file:////proc/sys/fs/inotify/max_user_instances in a text editor.

VS Code version: Code 1.63.2 (899d46d82c4c95423fb7e10e68eba52050e30ba3, 2021-12-15T09:39:46.686Z) OS version: Linux x64 5.15.8-1-default Restricted Mode: No

System Info |Item|Value| |---|---| |CPUs|Intel(R) Core(TM)2 Quad CPU Q6600 @ 2.40GHz (4 x 2379)| |GPU Status|2d_canvas: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: disabled_off
opengl: enabled_on
rasterization: disabled_software
skia_renderer: enabled_on
video_decode: disabled_software
vulkan: disabled_off
webgl: enabled
webgl2: enabled| |Load (avg)|1, 1, 1| |Memory (System)|5.66GB (0.14GB free)| |Process Argv|--unity-launch --crash-reporter-id 2a03cb52-9f4b-48bf-93d3-5c0537d534ee| |Screen Reader|no| |VM|0%| |DESKTOP_SESSION|/usr/share/xsessions/plasma5| |XDG_CURRENT_DESKTOP|KDE| |XDG_SESSION_DESKTOP|KDE| |XDG_SESSION_TYPE|x11|
Extensions (26) Extension|Author (truncated)|Version ---|---|--- doxdocgen|csc|1.3.2 vscode-resx-editor|Dom|0.0.5 vscode-pull-request-github|Git|0.34.2 Ionide-fsharp|Ion|5.10.2 better-cpp-syntax|jef|1.15.10 vscode-language-pack-pl|MS-|1.63.3 csharp|ms-|1.23.17 dotnet-interactive-vscode|ms-|1.0.2606010 vscode-dotnet-pack|ms-|1.0.4 python|ms-|2021.12.1559732655 vscode-pylance|ms-|2021.12.2 jupyter|ms-|2021.11.1001550889 jupyter-keymap|ms-|1.0.0 jupyter-renderers|ms-|1.0.4 remote-containers|ms-|0.209.6 remote-ssh|ms-|0.70.0 remote-ssh-edit|ms-|0.70.0 remote-wsl|ms-|0.63.13 cmake-tools|ms-|1.9.2 cpptools|ms-|1.7.1 cpptools-extension-pack|ms-|1.1.0 powershell|ms-|2021.12.0 vscode-markdown-notebook|ms-|0.0.26 vsliveshare|ms-|1.0.5200 vsliveshare-audio|ms-|0.1.91 cmake|twx|0.0.17 (1 theme extensions excluded)
A/B Experiments ``` vsliv368:30146709 vsreu685:30147344 python383:30185418 vspor879:30202332 vspor708:30202333 vspor363:30204092 pythontb:30283811 pythonvspyt551cf:30345471 pythonptprofiler:30281270 vshan820:30294714 vstes263:30335439 pythondataviewer:30285071 vscod805:30301674 pythonvspyt200:30340761 binariesv615:30325510 bridge0708:30335490 bridge0723:30353136 vsaa593cf:30376535 pythonvs932:30410667 vscop804cf:30404767 vscop940:30404999 vsrem710:30416614 ```
joeljosedev commented 2 years ago

I think there's an extra forward slash in your URL, does file:///proc/sys/fs/inotify/max_user_instances work?

jrieken commented 2 years ago

The link it invalid because it has four slashes. Assiging to @alexdima because we should consider not rendering this as link

yecril71pl commented 2 years ago

The link is valid, a path can contain consecutive slashes. https://github.com//microsoft/vscode/issues/139702

joeljosedev commented 2 years ago

There will be two forward slashes in file:// and then one more for /proc/sys/fs/inotify/max_user_instances right? So that's three slashes in total between file: and proc, but your URL has 4. Can you check if the 4 slashes in your link is a typo, please?

yecril71pl commented 2 years ago

There will be two forward slashes in file:// and then two more in //proc/sys/fs/inotify/max_user_instances. Yes, it was a typo. No, it is a valid path and should be supported.

jrieken commented 2 years ago

I see. Looks like today this fails uri validation

yecril71pl commented 2 years ago

Note that https://github.com//microsoft/vscode/issues/139702 apparently does not fail URI validation.

jrieken commented 2 years ago

The implementation is based on this comment: https://datatracker.ietf.org/doc/html/rfc3986#section-3.3. Maybe it misses some context that defines a prior normalisation step

Screen Shot 2022-01-03 at 17 07 59
yecril71pl commented 2 years ago

Unlikely, the specification indeed says this text does not represent a valid URI and normalisation applies to URI only. However, note that:

  1. the RFC is mostly for information interchange across domains, so it has less weight on local file URL,
  2. other clients are not that picky:
    kioclient5 cat 'file:////proc/sys/fs/inotify/max_user_instances'

It makes sense to disable news://// but leave file://// as a non-standard extension.

Of course, the rationale for that rule could invalidate my opinion—but I do not know what it is.