coder / code-server

VS Code in the browser
https://coder.com
MIT License
68.6k stars 5.62k forks source link

[Feat]: Allow turning rewrites off or give better rewrites control over asExternalUri #6504

Open Sominemo opened 1 year ago

Sominemo commented 1 year ago

What is your suggestion?

Allow controlling how asExternalUri() rewrites work beyond current --proxy-domain capabilities or allow turning them off completely.

Why do you want this feature?

Extensions like Dart Code use asExternalUri to open a WebView in the editor with their DevTools. In Dart Code's case, it refuses to function unless it's served off http://localhost:port root, and also requires connection to its WebSocket server on another port.

I use manual SSH port forwarding for these ports and it used to work about a year ago, but now code-server rewrites these URLs with its own port forwarding, which doesn't work with the extension and I found no way to fix it.

Are there any workarounds to get this functionality today?

With CS_DISABLE_PROXY=1 environment variable, remote.forwardOnOpen and remote.autoForwardPorts set to false, when I set a breakpoint on mainThreadWindow.ts:65, I get the following structure:

{
    authority: "example.com",
    fragment: "",
    path: "/proxy/9100/",
    query: "",
    scheme: "https",
}

Some implementation details not very related to the problem, but useful to know: When debugging, I get the aforementioned structure twice, first for the DevTools URL, and the similar one for the debug service URL. While the debug service URL is actually a websocket connection, Dart Code uses http protocol to get the asExternalUri() and then replaces it with ws, since the method doesn't support ws port forwarding.

After the extension receives the mapped url, a webview opens at https://example.com/DEBUGGER_URL, dropping the /proxy/9100 part, which I saw mentioned in the docs, though I don't understand how it works. The webview contains only Not Found error.

When using subdomain port forwarding, devtools just fail to load because of some bug in them.

When I copy the URL to my browser and manually change https://example.com to http://localhost:<port> (in both URI itself and its URI parameter that contains the endpoint for the debug service), the DevTools load and successfully connect to the websocket port I SSH forwarded earlier.

When opening VS Code with port forwarding over http://localhost:8080, I get the following structures from asExternalUri:

{
    "external": "http://127.0.0.1:9100/DEBUGGER_URL",
    "path": "/network",
    "scheme": "http",
    "authority": "127.0.0.1:9100",
}

Which is exactly what I need. But the resulting webview still points to localhost:8080, so the URL gets patched on some stage, but I can't find any lines in Dart Code which would cause that. Unless I'm missing something, my suspicion is that code-server replaces the result somewhere on returning the result to the extension or displaying the webview, but I don't know how to debug the remote side of the extension and need help with that.

The extension also has "Launch DevTools in browser" option, which tries to open https://example.com/proxy/<port>/ when opening VS Code from example.com, and http://localhost:8080/proxy/<port>/ when opened from localhost. If I studied the extension's code correctly, it uses openExternal() method.

Thus my suggested solution is to either allow to specify any sort of origin template in --proxy-domain, like http://localhost:{{port}}, or at least allow rewrites to be turned off so the localhost URIs remain untouched.

Are you interested in submitting a PR for this?

I could try, but I'm not familiar with code-server's code base yet.

code-asher commented 1 year ago

tldr VSCODE_PROXY_URI= code-server might do what you want, or at least gets you back to the previous behavior since a blank proxy URI will skip our rewriting.

Also, sorry for the long delay in a response, I have been out the past few weeks.


I tried configuring .example.com port forwarding, but it doesn't work with the extension

Weird, that does seem like it should work. I would have to experiment more with the Dart Code extension. Based on this and some of the other information it sounds like it might be doing some odd things or has some odd expectations, at least when running in the web, but it could also be code-server improperly implementing asExternalUri. We might want to also compare how the extension works in Codespaces. I do not fully understand how/where/when the rewriting is actually used by VS Code so I am not able to say much more at the moment.

I can't set --proxy-domain http://localhost:{{port}} because it results in https://httplocalhost%3Aport URLs

I see, yeah if the protocol is already included we should avoid prepending, or strip it out first, or at the very least return an error saying to omit the protocol. We currently always prepend // to make it work with either http or https. https://github.com/coder/code-server/blob/984fb135dc0baa822f8031f1c639b1fd953a6291/src/node/cli.ts#L605

With --proxy-domain localhost:{{port}} VS Code fails to connect to workbench (The workbench failed to connect to the server (Error: Failed to construct 'URL': Invalid URL))

I can confirm the same. Definitely a bug. Seems to happen when we try resolving it against the current domain, in case it is relative.

https://github.com/coder/code-server/blob/984fb135dc0baa822f8031f1c639b1fd953a6291/patches/proxy-uri.diff#L60

I think maybe what we do here is avoid trying to resolve if it starts with // or a protocol like http/https.

Setting CS_DISABLE_PROXY=1 && code-server and deleting all the existing port forwards doesn't turn the rewrites off

Ahh yeah if the proxy is disabled we should definitely skip the rewrite.

This could mean passing around the disabled value in VS Code but I think what we do instead is in the patch we use VSCODE_PROXY_URI as-is and do not default it:

https://github.com/coder/code-server/blob/984fb135dc0baa822f8031f1c639b1fd953a6291/patches/proxy-uri.diff#L78

Then in the wrapping code-server code we set VSCODE_PROXY_URI's default instead but skip doing so if the proxy is disabled (better control this way if we avoid setting defaults in VS Code anyway).

Setting remote.forwardOnOpen and remote.autoForwardPorts to false doesn't turn the rewrites off.

Yeah I think this only disables showing the rewritten URLs automatically in the ports panel, but asExternalUri continues to work as it has.

Maybe I'm doing something wrong, but setting VSCODE_PROXY_URI=http://localhost:{{port}} environment variable doesn't produce any result

You should get the same Invaild URL error I think, or at least that is what happens to me. VSCODE_PROXY_URI is how we pass the domain to rewrite into VS Code, so setting it manually should have the same effect.

That said, I think making it blank (VSCODE_PROXY_URI= code-server) might actually do what you want, since we only default to the path-based proxy when the proxy URI is unset (see ?? in patch above), and when it is blank we do no rewriting:

https://github.com/coder/code-server/blob/984fb135dc0baa822f8031f1c639b1fd953a6291/patches/proxy-uri.diff#L115-L119


In summary/action items:

code-asher commented 1 year ago

Actually I am a bit confused why VSCODE_PROXY_URI= code-server appears to be working for me, because the else branch throws an error which I would expect to be shown to the user. Maybe when the resolver throws an error VS Code skips resolving and uses the URI as-is?

Ah yeah looks like they do in general: https://github.com/microsoft/vscode/blob/ed684fcd1464b22a8e99039b3dd8fb90bed8e96a/src/vs/editor/browser/services/openerService.ts#L202-L204 https://github.com/microsoft/vscode/blob/ed684fcd1464b22a8e99039b3dd8fb90bed8e96a/src/vs/workbench/contrib/remote/browser/tunnelFactory.ts#L101-L103

DanTup commented 4 months ago

Hi! I work on the Dart-Code extension and had some other reports of things not working correctly when using code-server (https://github.com/flutter/devtools/issues/8067). I was looking to see if some of these issues are easy fixes.

The issue with DevTools is that that backend server binds to a local port and thinks it is hosting at /. Because of how Flutter apps client side routing works, the <base href="" /> tag must be set correctly for the root of the app. However, the server that is serving DevTools does not know that code-server will be rewriting the URL to /1234/proxy and therefore serves up the page with <base href="/" />.

Does (or could?) the proxy pass through a header to indicate the original/client-side URL for the resource being requested? Eg. when it send a request for / to the DevTools server, if there was a header that said that the front-end path was actually /1234/proxy, perhaps that could be written into the base href to solve this issue?

Another possible option that may work based on https://github.com/coder/code-server/discussions/4563 is using --proxy-domain so that DevTools is still served from the root but on its own hostname (port.foo), although I wasn't able to test this well because Windows doesn't have wildcard localhost domains DevTools.. I tried localtest.me but unlike localhost that's not considered a secure context so DevTools also fails. I presume if this was running over HTTPS then things might work.

code-asher commented 4 months ago

Does Flutter support making it relative by any chance? <base href="." /> for example.

Would it work if code-server was not rewriting the path? We have a separate proxy endpoint /absproxy/<port> users can use which will not rewrite the request to /. Then you would also tell Flutter the base path is /absproxy/<port>, assuming it has a setting for that.

If none of that works then it seems very reasonable to add a header indicating the original path. I did some brief searching and did not find a standard but maybe we could call it X-Forwarded-Path.

--proxy-domain should definitely work in a regular setup, but I bet you are right about the secure context. For testing locally I have had luck with --proxy-domain localhost:8080 and then browsing to http://$port.localhost:8080.

code-asher commented 4 months ago

One problem with the non-rewriting /absproxy proxy, if it even works, is that a user has to manually enter and use it, which is not a great experience. Maybe the header is the only reasonable way to solve this, unless we can somehow automatically tell which proxy we should be using.

DanTup commented 4 months ago

Does Flutter support making it relative by any chance? <base href="." /> for example.

I tested that just after posting here and found that it actually does seem to. However there's another bug (in DevTools) we trigger in that mode (which coincidentally has recently been fixed). I will do some more testing with this - I think a relative base href (which we can adjust depending on the path of the incoming request) might be the best solution.

(I don't think using /absproxy will work because the server at the backend may be hosting multiple things, so it needs to know which requests to treat as DevTools requests and I don't think hard-coding knowledge of /absproxy would be ideal)

I'll post back once I've done some proper testing with the fix above and a relative base href. Thanks!

DanTup commented 2 months ago

(just to close off the discussion above, everything seemed to work fine with a relative base href now, so I'm going ahead with that fix and don't believe anything additional is required to fix the Dart DevTools issue)