Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fix basePath #1272

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

Closed
akhilputhiry wants to merge 1 commit into coder:master from akhilputhiry:fix-basepath
Closed

Fix basePath #1272

akhilputhiry wants to merge 1 commit into coder:master from akhilputhiry:fix-basepath

Conversation

Copy link

@akhilputhiry akhilputhiry commented Jan 13, 2020
edited by sr229
Loading

This PR fixes #1237

@akhilputhiry akhilputhiry changed the title (削除) Addresses https://github.com/cdr/code-server/issues/1237 (削除ここまで) (追記) Fix basePath (追記ここまで) Jan 13, 2020
Copy link
Contributor

sr229 commented Jan 13, 2020

Edited the message just to reference GH-1237 and allow a merge to auto-close this.

@sr229 sr229 added the enhancement Some improvement that isn't a feature label Jan 13, 2020
Copy link
Member

code-asher commented Jan 13, 2020 via email

The base path actually doesn't have anything to do routing incoming requests. That's the job of a reverse proxy. We use the base path to redirect from the login page and to set some headers to the root path. If we made this change then this could break reverse proxies that are properly configured. For example if someone configured their path to be `/vs` then all of our `/vs` paths will stop working because the `vs` gets stripped out. When receiving a request to `/code/some/path` the reverse proxy should route that and code-server should receive it as `/some/path`, meaning the base path is already stripped. So the linked issue most likely has to do with a misconfigured reverse proxy (or isn't using one at all, in which case there is no need for the base path in the first place). Hopefully that makes sense. Maybe we should write a message to stdout when the `base-path` flag is in use explaining some of this?

Copy link
Author

My understanding about --base-path is different.
I can pass any random path to the flag --base-path and code-server should run in that path.
eg: --base-path=/akhil/server1 then code server should be up and running in the path /akhil/server1. This is a common functionality available in almost all web apps like Prometheus, grafana, jupyter, kibana etc. etc.

Implementation could be different, but this is the ask of all those issues raised saying base path is not working.

Any thoughts??

Copy link
Member

code-asher commented Jan 15, 2020 via email

I think all of those applications provide a base path exactly for the purpose of putting them behind a reverse proxy. Is that not the case? I'm not clear on what the benefit would be of using a base path without a reverse proxy. Suppose you host code-server at example.com and then provide a base path of /vs so it's accessible at example.com/vs. Why not let it be accessed at example.com and skip the base path entirely? Nothing can be hosted there anyway since code-server is already there. Also after this change a reverse proxy working today at example.com/code would require you to access code-server at example.com/code/code. I can't think of any implementation that wouldn't have this problem. Let me know if I'm missing something!

Copy link
Author

@code-asher Let me try to break it down for you.

The requirement is, we need to run multiple code-servers behind the proxy for multiple users.

Now this can be achieved this by rewriting the URL inside the proxy server. But not all proxy servers support this feature (eg: configurable-http-proxy).

if we can run the app in the context or path provided by the --base-path all proxies will be able to handle the code-server.

mausch reacted with thumbs up emoji

Copy link
Member

code-asher commented Jan 17, 2020 via email

Ahhh, so this is for proxies that cannot rewrite the URL before passing it along to code-server. That makes sense. I'm still not sure how best we can support this scenario without breaking it for proxies that do support that like I previously mentioned. We would need to know beforehand whether the proxy was rewriting the URL or not so we can act appropriately. I'm open to ideas on this. Maybe a new flag like `--proxy-path`?

Copy link
Member

code-asher commented Jan 17, 2020 via email

Or maybe `--rewrite-url`.

Copy link
Author

@code-asher We dont need to do anything extra for proxy's with rewrite capability. It will work out of the box.

Copy link
Member

code-asher commented Jan 21, 2020 via email

If the base path just so happens not to be any path that code-server itself used then everything would be fine. That does seem likely, but it's not something I'd feel comfortable relying on. For example suppose you hosted code-server at `example.com/vs` with `--base-path=/vs`. Then suppose code-server itself has an endpoint at `/vs` and a request is made to example.com/vs/vs. Without a rewriting proxy the result is /vs. All is well and the /vs endpoint is served. With rewriting the result is / and the homepage is served instead. This could be solved by not passing the base path if you're using a rewriting proxy, but unfortunately the base path would still be necessary for redirections and for setting certain headers (like the base path for service workers). I'm not sure we'll be able to have consistent rewriting behavior without first knowing whether the URL has already been rewritten, and the only way to know that is to be told by the user. Maybe we could add a boolean flag `--rewrite` which turns on URL rewriting? When `--base-path` is set we could also output some text about `--rewrite` to raise awareness. I'm definitely open to other solutions if there are any.

Copy link
Contributor

nhooyr commented Jan 27, 2020

See #1237

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@code-asher code-asher Awaiting requested review from code-asher

@kylecarbs kylecarbs Awaiting requested review from kylecarbs

1 more reviewer

@sr229 sr229 sr229 approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
enhancement Some improvement that isn't a feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

base-path option is not use.

AltStyle によって変換されたページ (->オリジナル) /