Slashes in folder param become escaped

[Merith-TK]

Version: 1.41.1
Commit: f51e045cd5483561afc07694f39307fb673b6d1d
Date: 2020-01-17T22:58:55.612Z
Browser: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/79.0.3945.130 Safari/537.36
Code Server Version: 2.1698-vsc1.41.1

• OS Version: archlinux

Description

When changing directories, it becomes
https://code.example.com/?folder=vscode-remote%3A%2F%2Fcode.example.com%2Fhome%2Fdeveloper%2FWorkspace%2FDND
instead of `https://code.example.com/?folder=/home/developer/Workspace/

Steps to Reproduce

1. open folder as default (accessible at https://example.com/)
2. CTRL+k and CTRL+O and open a different folder
3. check url

[Merith-TK]

yes i know the name of the issue is horrible, my words have been failing me today

[nhooyr]

Fixed :)

Thanks for reporting, I confirmed this.

[Merith-TK]

Yeah, also whats with the ?folder=vscode-remote%3A%2F%2Fcode.example.com%2F instead of ?folder=/

I dont understand why it does this?

[nhooyr]

Not sure either. cc @code-asher.

[code-asher]

I'm not certain, that just seems to be what VS Code 1.41.1 is doing now.

[code-asher]

To clarify, if you click file > open that's what VS Code puts in the URL, so I updated our server code to match.

[Merith-TK]

i think it may be because of the implementation?
This suggests that code-server uses a varient of the vscode-remote extension and wasnt properly modifed

[code-asher]

Internally VS Code refers to remote files with the vscode-remote:// scheme. In our case the remote provider is code-server but that doesn't change the scheme that VS Code uses. We could change it (and we used to) but there didn't seem any benefit in the added patching it required.

[Merith-TK]

Ah, alright then,

[coadler]

Aren't slashes supposed to be URL encoded in query strings?

[nhooyr]

It's optional.

package main

import (
	"fmt"
	"net/url"
)

func main() {
	u, _ := url.Parse("localhost:8080/?meow=bar/foo&big=coadler/asher")
	fmt.Println(u.Query())
}

Prints:

map[big:[coadler/asher] meow:[bar/foo]]

[coadler]

That's true it parses, but there'd be no way to construct the same thing on the way out

package main

import (
	"fmt"
	"net/url"
)

func main() {
	u, err := url.ParseQuery("meow=bar/foo&big=coadler/asher")
	if err != nil {
		panic(err)
	}
	
	fmt.Println(u)
	fmt.Println(u.Encode())
}

map[big:[coadler/asher] meow:[bar/foo]]
big=coadler%2Fasher&meow=bar%2Ffoo

https://tools.ietf.org/html/rfc2396#section-3.4 seems to indicate slashes are reserved within query strings

The documentation for ParseQuery says that it expects a URL-encoded query string but seems to work fine if it doesn't
https://golang.org/src/net/url/url.go?s=25201:25246#L859

[nhooyr]

That's interesting. So then everything is working as expected.

@code-asher Why does the folder argument need the scheme when it's always vscode-remote? Let's just remove it and add it ourselves when appropriate?

[code-asher]

My assumption was that it would eventually be possible (if not already) to open different schemes on startup. For example maybe a git:// URL or an ssh:// URL or something. But that's just a guess and I haven't looked into the commits or issues to try determining VS Code's reasons for making this change.

[JtMotoX]

I navigate many times by manually editing the url to open the folder I want. It also helps me easily see the directory that I am currently in, since many times I will have 5 tabs open with different directories. The new vscode-remote:// URL has been pretty annoying. I wish there were a way to keep the folder= URL or at least an option or parameter we can pass when launching code-server.

Workaround to give you the old url functionality back:
My code-server instance sits behind a nginx reverse proxy with Let's Encrypt SSL and a secure username+pwd login form. If anyone has their instance behind an nginx reverse proxy, you can add the configuration lines below to your nginx configs which will strip out the vscode-remote:// and replace the characters with slashes which will give you the url format that we are all used to. Each additional line will support 1 subdirectory deep, so my configs below will support 12 subfolders deep. There may be a better way of doing this but I am new to nginx and only have about 10 hours of experience with it, 2 of which were spent trying to solve this.

This:
https://my.domain.com/?folder=vscode-remote%3A%2F%2Fmyhostname%2FUsers%2Fusername%2Ffolder1%2Ffolder2%2Ffolder3
Becomes This:
https://my.domain.com/?folder=/Users/username/folder1/folder2/folder3

    set $args_original $args;
    if ($args ~ ^(folder=)vscode-remote%3A%2F%2F.*?%2F(.*)$) { set $args $1/$2; }
    if ($args ~ ^(folder=.*)%2F(.*)$) { set $args $1/$2; }
    if ($args ~ ^(folder=.*)%2F(.*)$) { set $args $1/$2; }
    if ($args ~ ^(folder=.*)%2F(.*)$) { set $args $1/$2; }
    if ($args ~ ^(folder=.*)%2F(.*)$) { set $args $1/$2; }
    if ($args ~ ^(folder=.*)%2F(.*)$) { set $args $1/$2; }
    if ($args ~ ^(folder=.*)%2F(.*)$) { set $args $1/$2; }
    if ($args ~ ^(folder=.*)%2F(.*)$) { set $args $1/$2; }
    if ($args ~ ^(folder=.*)%2F(.*)$) { set $args $1/$2; }
    if ($args ~ ^(folder=.*)%2F(.*)$) { set $args $1/$2; }
    if ($args ~ ^(folder=.*)%2F(.*)$) { set $args $1/$2; }
    if ($args ~ ^(folder=.*)%2F(.*)$) { set $args $1/$2; }
    if ($args ~ ^(folder=.*)%2F(.*)$) { set $args $1/$2; }
    if ($args != $args_original) {
        return 301 "$scheme://${http_host}$uri?$args";
    }

[code-asher]

Yeah, I find the new format fairly annoying as well. I'm thinking we should patch VS Code to use the old URL format when the scheme is vscode-remote. That would restore the old behavior while still allowing other schemes to be used.

[Merith-TK]

how easy would that patch be?

[code-asher]

I imagine it would be a single line change (or two if we want to do the same for workspace paths). The URL is created in `src/vs/code/browser/workbench/workbench.ts` in `createTargetUrl` so we'd just need to make it use `.path` instead of `.toString()` and remove the encoding.

[code-asher]

Oh but we'd only do that if the scheme is vscode-remote, otherwise we keep the current behavior.

[JtMotoX]

That sounds great!

[JtMotoX]

I imagine it would be a single line change

30 lines later...

closed this in 26647c5

Thanks Asher! 👏

[code-asher]

I think we're going to need to revert this (at least re-introduce the encoding) because otherwise you can't open directories with #, &, etc to name a few.

[code-asher]

Hmm maybe we can decode just the slashes after encoding (.replace("%2F", "/")). Seems to work but I'm not sure if there's some case I'm not considering.

[t-d-d]

@code-asher and can't open paths with spaces which is fairly common. See #1488