Skip to content
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

Change streaming API host to not be overridden to localhost in development mode #28557

Merged
merged 2 commits into from Jan 3, 2024

Conversation

ClearlyClaire
Copy link
Contributor

Fixes #28539

I'm not sure there are situations in which we don't want the code path where REMOTE_DEV=true.

cc @ykzts as you added this code path in #5942

ykzts
ykzts previously approved these changes Jan 2, 2024
@sgtatham
Copy link

sgtatham commented Jan 2, 2024

My 2p (as the raiser of #28539, not a developer): I agree that this is sensible.

Rationale: a web server should basically never send a 301 redirect with the target hostname of localhost, because that means "go and look at the web server running on your client machine, wherever that might be". Quite likely the user's client machine isn't running a web server at all – and if it is, the redirecting server has no idea what might be on it, or what URLs even exist, or what they mean if they do.

One exception might be if the request comes from one of the server's own IP addresses; in that situation it would probably be safe to assume that the client understands the same thing by localhost that you do. But as a response to a remote client, I can't think of any situation in which it would be correct to redirect to localhost.

If the Mastodon test VM needs to have a configuration in which the separate streaming HTTP port isn't accessible from the host machine, then that should be signalled by an HTTP 4xx fatal error, or perhaps by redirecting to a service that returns Connection Refused, not by redirecting to a URL that makes no sense at all!

@ClearlyClaire ClearlyClaire added this pull request to the merge queue Jan 3, 2024
Merged via the queue into main with commit bd415af Jan 3, 2024
47 checks passed
@ClearlyClaire ClearlyClaire deleted the fixes/development-streaming-host branch January 3, 2024 10:29
vmstan pushed a commit to vmstan/mastodon that referenced this pull request Jan 5, 2024
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.

Streaming endpoint on Vagrant sends 301 redirect to localhost
3 participants