Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set environment variable VSCODE_PROXY_URI #1510

Closed
benz0li opened this issue Apr 10, 2020 · 35 comments · Fixed by #4681 or coder/vscode#42
Closed

Set environment variable VSCODE_PROXY_URI #1510

benz0li opened this issue Apr 10, 2020 · 35 comments · Fixed by #4681 or coder/vscode#42
Assignees
Labels
enhancement Some improvement that isn't a feature
Milestone

Comments

@benz0li
Copy link
Contributor

benz0li commented Apr 10, 2020

With the implementation of the HTTP proxy it would be great to have an environment variable VSCODE_HTTP_REFERER available to build custon URLs for proxied webview requests.

This feature helps solving REditorSupport/vscode-R#275. The existence of such an environment variable also allows to differentiate a local (VS Code) installation from a server (code-server) installation.

Other server-based IDEs set an environment variable based on the HTTP referer when the client session is initialized. See https://github.com/rstudio/rstudio/blob/aa266ded96123f654e5f3163cde39697744eee89/src/cpp/session/SessionClientInit.cpp#L170 for example.

Could you implement something similar in code-server? Or is the information on the HTTP referer already available somewhere else?

@kylecarbs
Copy link
Member

Neat. I don't think we have this ATM.

Thoughts @code-asher ?

@code-asher
Copy link
Member

code-asher commented Apr 15, 2020

I experimented with replacing 127.0.0.1:<port> with /proxy/<port> before writing out the webview HTML which worked for the PlatformIO extension but then it tried to make a websocket connection against the root (/wsrpc) instead of being relative (/path/<port>/wsrpc) which obviously won't work.

So the environment variable sounds like a reasonable way to solve this. Either that or require that extensions be aware that they might be behind a proxied base path and make connections accordingly. There might be other cases I haven't considered though so the environment variable seems like the most flexible.

@nhooyr nhooyr added the enhancement Some improvement that isn't a feature label Apr 17, 2020
@code-asher
Copy link
Member

code-asher commented Aug 3, 2020

We should also make this environment variable available to the terminal (in addition to extensions), see #1939.

@benz0li
Copy link
Contributor Author

benz0li commented Jun 13, 2021

@code-asher Any timeframe on this issue? Does the name tend towards VSCODE_HTTP_REFERER or CODE_SERVER_HTTP_REFERER?

The project collaborators at Ikuyadeu/vscode-R might give another go at REditorSupport/vscode-R#275 while working on REditorSupport/vscode-R#358.

@code-asher
Copy link
Member

code-asher commented Jun 14, 2021 via email

@code-asher
Copy link
Member

code-asher commented Jun 14, 2021 via email

@benz0li
Copy link
Contributor Author

benz0li commented Jun 14, 2021

Oh right the name. I think CODE_SERVER_HTTP_REFERER makes sense.

@code-asher Keeping in mind other derivatives of VS Code (e.g. Eclipse Theia) it would make sense to tend towards VSCODE_HTTP_REFERER.
👉 The name of their ancestor; a common name to implement the functionality for Ikuyadeu/vscode-R in all of VS Code derivatives - if their collaborators decide to do so.

Agreed?

@benz0li
Copy link
Contributor Author

benz0li commented Jun 14, 2021

Alternaively HTTP_REFERER; neutral, not mentioning a product name.

@code-asher
Copy link
Member

That makes perfect sense to me! My vote would be for HTTP_REFERER I think but I'm good with either one.

@benz0li
Copy link
Contributor Author

benz0li commented Jun 15, 2021

Please use VSCODE_HTTP_REFERER then, as there are already other environment variables in code-server prefixed with VSCODE_ referencing to its ancestor; e.g. VSCODE_GIT_IPC_HANDLE, VSCODE_GIT_ASKPASS_NODE, etc.

Thank you!

@code-asher code-asher modified the milestones: Backlog, 3.11.0 Jun 15, 2021
@code-asher
Copy link
Member

Should we be using the actual referer header here (or host actually I suppose)? Or do we want the proxy path? If it needs to be general for other editors I'm thinking they might not all have /proxy or might do it differently so maybe we need something like VSCODE_PROXY_URI or something like that, perhaps with {port} in it.

Then if the user has sub-domain proxies enabled we could set it to {port}.mydomain.com and if they don't we could set it to mydomain.com/proxy/{port}. And if the environment variable already exists we wouldn't override it so users could provide their own proxy through Caddy, NGINX, etc.

Otherwise we might need to use CODE_SERVER_HTTP_REFERER so extensions can tell the difference between editors and select the URL appropriately.

@benz0li
Copy link
Contributor Author

benz0li commented Jun 15, 2021

@code-asher The actual referer is sufficient.

/proxy/{port} works by default, right? According to #1453 (comment)

No command-line flags are necessary to enable this.

@code-asher
Copy link
Member

It does, it's just less stable because not all applications handle the base path properly so I'm biased against it haha 😛 In this case it's probably fine though.

Does Theia support /proxy as well?

@benz0li
Copy link
Contributor Author

benz0li commented Jun 15, 2021

I don't think Theia supports /proxy: eclipse-theia/theia#8821.

@code-asher
Copy link
Member

Gotcha, do we need to consider that they (or some other editor) might choose a different path than code-server if they do implement it? (In which case I imagine extensions need a way to distinguish between editors.)

@benz0li
Copy link
Contributor Author

benz0li commented Jun 15, 2021

For the time being, you don't have to take this into account.

But keeping an open mind: Why not setting VSCODE_PROXY_PATH to /proxy in case of code-server?

@benz0li
Copy link
Contributor Author

benz0li commented Jun 15, 2021

And if the user has sub-domain proxies enabled: Not setting VSCODE_PROXY_PATH at all.

From the viewpoint of the extension:

  1. If VSCODE_PROXY_PATH is set, use that proxy path.
  2. If VSCODE_PROXY_PATH is unset, use the sub-domain proxy.

@benz0li
Copy link
Contributor Author

benz0li commented Jun 15, 2021

@code-asher Having said the above, I realise that VSCODE_PROXY_URI is the better solution.

[...] if the user has sub-domain proxies enabled we could set it to {port}.mydomain.com and if they don't we could set it to mydomain.com/proxy/{port}. And if the environment variable already exists we wouldn't override it so users could provide their own proxy through Caddy, NGINX, etc.

Go ahead with this solution.

@code-asher code-asher self-assigned this Jun 15, 2021
@code-asher
Copy link
Member

Sounds good! I'll knock this out tomorrow so let me know if anything changes in the meantime.

@benz0li
Copy link
Contributor Author

benz0li commented Jun 16, 2021

@code-asher I'm running code-server via JupyterHub as part of a JupyterLab image. There is a deployment at https://vscode-r.jupyter.b-data.ch for which your GitHub account has been whitelisted.

code-asher added a commit to code-asher/code-server that referenced this issue Jun 21, 2021
code-asher added a commit to code-asher/code-server that referenced this issue Jun 21, 2021
@benz0li
Copy link
Contributor Author

benz0li commented Jun 23, 2021

Wow! I'm really impressed with this platform/setup. Might be interesting to have you chat about it at Coffee and Coder with @bpmct

I've whitelisted @bpmct for https://vscode-r.jupyter.b-data.ch, too.

@benz0li
Copy link
Contributor Author

benz0li commented Jun 23, 2021

The setup is based on https://gitlab.b-data.ch/docker/deployments/jupyter and currently running

  • JupyterHub v1.4.1
  • JupyterLab v3.0.16 including
    • code-server v3.10.2 (aka VS Code v1.56.1)
    • Extensions: R v1.6.8 and R LSP Client v0.1.14, ...
    • ...

👉 Full specification of the JupyterLab image at https://gitlab.b-data.ch/jupyterlab/r/r-ver/-/blob/master/r-ver/latest.Dockerfile; plus libxt-dev installed in addition.

I’m happy to further customize the Docker image, i.e. adding any Linux package you need for testing/development/etc*.
ℹ️ JupyterLab (code-server) runs in a docker container; docker volume for $HOME; no sudo rights

@benz0li
Copy link
Contributor Author

benz0li commented Jun 23, 2021

Installed packages according to https://github.com/cdr/code-server/blob/main/docs/CONTRIBUTING.md#requirements

@benz0li
Copy link
Contributor Author

benz0li commented Jun 23, 2021

@jsjoeio Should one be able to develop and test extensions with code-server?

I followed https://github.com/Ikuyadeu/vscode-R/wiki/Contributing and /opt/code-server/extensions/ikuyadeu.r-1.6.8/out/extension gets activated instead of the modified version of vscode-R (in this case ~/projects/ikuyadeu/vscode-R/out/extension).

@jsjoeio
Copy link
Contributor

jsjoeio commented Jun 23, 2021

Hmm...it looks like there was an issue in the past but I haven't tried ever so I can't say.

I'm also not familiar with vscode-R so I can't help much but that does sound odd.

If you follow this Hello World guide and it doesn't work with code-server, then there is probably a bug (and you can open a separate bug report for us to look into).

@benz0li
Copy link
Contributor Author

benz0li commented Jun 24, 2021

Opened separate bug report: #3661

@jsjoeio
Copy link
Contributor

jsjoeio commented Jun 24, 2021

Awesome. You rock! Thank you!

@jsjoeio jsjoeio modified the milestones: 3.11.0, 3.12.0 Jul 19, 2021
code-asher added a commit to code-asher/code-server that referenced this issue Aug 25, 2021
@jsjoeio jsjoeio modified the milestones: 3.12.0, 3.12.1 Sep 9, 2021
@benz0li
Copy link
Contributor Author

benz0li commented Sep 16, 2021

FYI https://vscode-r.jupyter.b-data.ch has been updated:

  • JupyterHub v1.4.2
  • JupyterLab v3.1.14 including
    • code-server v3.12.0 (aka VS Code v1.60.0)
    • Extensions: R v2.3.0
    • ...

@jsjoeio
Copy link
Contributor

jsjoeio commented Sep 16, 2021

Just a heads-up - we've had a bunch of other priorities come up and haven't been able to get to this yet (but we will soon)

@benz0li
Copy link
Contributor Author

benz0li commented Jan 5, 2022

@code-asher coder/vscode#37 sets mydomain.com/proxy/{{port}} instead of mydomain.com/proxy/{port}.
ℹ️ Tested with the latest release artefacts.

Why the double brackets?

@benz0li benz0li mentioned this issue Jan 5, 2022
11 tasks
@benz0li
Copy link
Contributor Author

benz0li commented Jan 5, 2022

Since REditorSupport/vscode-R#803 already in place, this will lead to errors with the current version of the R extension.

I suggest the following:

  1. @code-asher modifies Add proxy URI enviroment variable vscode#37 before releasing v4.0.1, i.e.
    diff --git a/src/vs/server/webClientServer.ts b/src/vs/server/webClientServer.ts
    index 9c0f0e1cf8f..d9e41798aa2 100644
    --- a/src/vs/server/webClientServer.ts
    +++ b/src/vs/server/webClientServer.ts
    @@ -320,7 +320,7 @@ export class WebClientServer {
                                            // Endpoints
                                            base,
                                            logoutEndpointUrl: base + '/logout',
    -                                       proxyEndpointUrlTemplate: base + '/proxy/{{port}}',
    +                                       proxyEndpointUrlTemplate: base + '/proxy/{port}',
                                            webEndpointUrl: vscodeBase + '/static',
                                            webEndpointUrlTemplate: vscodeBase + '/static',
                                            webviewContentExternalBaseUrlTemplate: vscodeBase + '/webview/{{uuid}}/',
  2. @code-asher is going to implement /proxy/{{port}} with the next release, assumingly v4.0.2.

In the meantime and before releasing code-server v4.0.2:

  1. I am going to propose the following update in Support both single and double brackets in code-server's URI template REditorSupport/vscode-R#934:
    diff --git a/R/session/vsc.R b/R/session/vsc.R
    index c2f0695..8f0505a 100644
    --- a/R/session/vsc.R
    +++ b/R/session/vsc.R
    @@ -516,7 +516,7 @@ show_browser <- function(url, title = url, ...,
       if (nzchar(proxy_uri)) {
         is_base_path <- grepl("\\:\\d+$", url)
         url <- sub("^https?\\://(127\\.0\\.0\\.1|localhost)(\\:)?",
    -      sub("{port}", "", proxy_uri, fixed = TRUE), url)
    +      sub("\\{\\{?port\\}\\}?", "", proxy_uri), url)
         if (is_base_path) {
           url <- paste0(url, "/")
         }
    @@ -573,7 +573,7 @@ show_webview <- function(url, title, ..., viewer) {
       if (nzchar(proxy_uri)) {
         is_base_path <- grepl("\\:\\d+$", url)
         url <- sub("^https?\\://(127\\.0\\.0\\.1|localhost)(\\:)?",
    -      sub("{port}", "", proxy_uri, fixed = TRUE), url)
    +      sub("\\{\\{?port\\}\\}?", "", proxy_uri), url)
         if (is_base_path) {
           url <- paste0(url, "/")
         }

ℹ️ This makes sure, that both versions (single brackets, double brackets) are handled correctly.

@code-asher
Copy link
Member

Ahh I did not realize this was already implemented in vscode-R!

I used double braces to match VS Code's own templating scheme they use with webview URIs. I will go ahead and move back to one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Some improvement that isn't a feature
Projects
None yet
5 participants