Skip to content

fix(webClientServer): use relative path in updateUrl #17

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

Merged
merged 1 commit into from
Nov 19, 2021

Conversation

jsjoeio
Copy link

@jsjoeio jsjoeio commented Nov 17, 2021

Description

This patches the webClientServer to ensure the updateUrl is relative to the root.
This ensures the correct URL in case code-server is being served behind a
reverse proxy.

An example of this would be using Caddy to server code-server on
localhost:8082/code/

This fix ensures request urls are created against localhost:8082/code/ instead
of localhost:8082/

Screenshot

Notice the Request URL in the image. Serving code-server regularly (i.e. on 8080), the update/check is made against the root.
Screen Shot 2021-11-17 at 2 50 47 PM

When serving code-server behind a reverse-proxy with Caddy, the update/check is made against the reverse-proxy root (i.e. /code/

Screen Shot 2021-11-17 at 2 50 43 PM

Testing Plan

I tested against the root and the reverse-proxy.

  1. cd vscode && git checkout jsjoeio-fix-csp-reverse-proxy
  2. yarn link
  3. cd code-server && yarn link code-oss-dev --modules-folder vendor/modules
  4. create Caddyfile somewhere:
http://localhost:8082/code/* {
  uri strip_prefix /code
  reverse_proxy 127.0.0.1:8080
}
  1. caddy run
  2. Navigate to http://localhost:8082/code/ in the browser and check Network tab. Filter for "check" and look at the Headers tab for the request.

I also did some basic testing around new URL in Codesandbox here.

This PR fixes coder/code-server#4476

(There is a chance this fixes other reverse proxy issues, but I will test after this is merged).

@jsjoeio jsjoeio self-assigned this Nov 17, 2021
@jsjoeio jsjoeio marked this pull request as ready for review November 17, 2021 22:06
@jsjoeio
Copy link
Author

jsjoeio commented Nov 18, 2021

NOTE: transitioning to draft so I can come up with a more reliable solution.

Look at http in code-server and add a new method that does this instead. We should be able to achieve this change with relative urls.

@jsjoeio jsjoeio marked this pull request as draft November 18, 2021 23:40
@jsjoeio jsjoeio force-pushed the jsjoeio-fix-csp-reverse-proxy branch from 120750f to 4b2ade3 Compare November 19, 2021 16:29
@jsjoeio jsjoeio marked this pull request as ready for review November 19, 2021 16:37
@jsjoeio jsjoeio requested a review from code-asher November 19, 2021 16:37
@jsjoeio
Copy link
Author

jsjoeio commented Nov 19, 2021

@TeffenEllis since Asher is out today and next week, do you mind reviewing? It's very minimal

@jsjoeio jsjoeio added bug Something isn't working enhancement New feature or request labels Nov 19, 2021
@GirlBossRush GirlBossRush merged commit e232be7 into code-server-v2 Nov 19, 2021
@GirlBossRush GirlBossRush deleted the jsjoeio-fix-csp-reverse-proxy branch November 19, 2021 19:04
@jsjoeio jsjoeio changed the title fix(webClientServer): include basePath in createRequestUrl fix(webClientServer): use relative path in updateUrl Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants