Set remote authority on frontend #25
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Trying to determine the remote authority on the backend is brittle because it does not work behind reverse proxies unless they send the right headers containing information about the proxied source. We could require users add the relevant configuration or provide the remote authority via a flag but neither are user-friendly options. We can make it work out of the box by changing the frontend to make requests to its current address (which is what we try to set the remote authority to anyway). This actually already happens for the most part except in some UI and logs although recent issues suggest there might be other problems which should be entirely resolved by setting this on the frontend. In other words, the remote authority we set on the backend should never be used so we set it to something invalid to ensure we notice (the alternative is to rip it out but that is probably a bigger patch thus generating more conflicts). One scenario where we might want to set the remote authority from the backend is if the frontend is served from a different location than the backend but that is not supported behavior at the moment. Even if we did support this we still cannot determine the authority from the backend (even for non-proxy scenarios in this case) and would need to add a flag for it so this change would still be necessary. coder/code-server#4604 coder/code-server#4607 coder/code-server#4608
jsjoeio
approved these changes
Dec 14, 2021
|
They probably do serve it behind a reverse proxy but they will never need the port because the source is always 443 and they will never need the path because they always serve at the root, so for them it is safe to just use the host header as-is. And since they control everything they can ensure the host header is always set and accurate. |
Ah! They're in control of these values so they don't need it to be as flexible as we do. Makes sense! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This should resolve at least some of the port issues, if not all of them.
See the commit message for details.