Skip to content

Add port to redirect URL when server port is 80 #741

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
wants to merge 2 commits into from
Closed

Add port to redirect URL when server port is 80 #741

wants to merge 2 commits into from

Conversation

deansheather
Copy link
Member

Describe in detail the problem you had and how this PR fixes it

This fixes HTTP to HTTPS redirects when people are running code-server on port 80 (default HTTP port).

Essentially, if code-server's port is 80 and is accessed over HTTP, code-server would redirect to HTTPS without adding :80 to the end of the URL, resulting in the browser accessing over port 443.

I'm hesitant about this PR as it could cause problems with reverse proxies but unable to try multiple reverse proxy scenarios. I don't think this should be merged until reverse proxies are tested and everything works as expected.

Is there an open issue you can link to?

#737

I've made another PR for changing the documentation that lead the user in the above issue to using port 80: #740

This fixes HTTP to HTTPS redirects when people are running code-server
on port 80 (default HTTP port).
@code-asher
Copy link
Member

I think what we might want to do here is if the port is 80, also listen to 443.

@code-asher
Copy link
Member

Or allow the user to specify multiple ports and just have an expectation that the user will add all the necessary ports for their setup.

@deansheather
Copy link
Member Author

I think the second choice is a better solution. We could also add a warning during startup that says "you might want to add port 443 to your server ports" if port 80 is supplied but not 443 and --allow-http is false.

@code-asher
Copy link
Member

Yeah, that sounds great to me.

@deansheather
Copy link
Member Author

Closed in favor of #819 which allows for listening on multiple ports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants