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

Fix streaming API redirection ignoring the port of streaming_api_base_url #28558

Merged
merged 1 commit into from Jan 2, 2024

Conversation

ClearlyClaire
Copy link
Contributor

@ClearlyClaire ClearlyClaire added the bug Something isn't working label Jan 2, 2024
@ClearlyClaire ClearlyClaire requested a review from a team January 2, 2024 11:15
Copy link

codecov bot commented Jan 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cd09be0) 84.59% compared to head (adf76a6) 84.59%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #28558   +/-   ##
=======================================
  Coverage   84.59%   84.59%           
=======================================
  Files        1039     1039           
  Lines       28238    28240    +2     
  Branches     4550     4550           
=======================================
+ Hits        23889    23891    +2     
  Misses       3197     3197           
  Partials     1152     1152           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ClearlyClaire ClearlyClaire changed the title Fix streaming API redirection ignoring the port path of streaming_api_base_url Fix streaming API redirection ignoring the port of streaming_api_base_url Jan 2, 2024
@ClearlyClaire ClearlyClaire added this pull request to the merge queue Jan 2, 2024
Merged via the queue into main with commit 1184887 Jan 2, 2024
50 checks passed
@ClearlyClaire ClearlyClaire deleted the fixes/streaming-redirection-ignoring-port branch January 2, 2024 12:33
vmstan pushed a commit to vmstan/mastodon that referenced this pull request Jan 5, 2024
@ThisIsMissEm
Copy link
Contributor

I think the check of if Rails.configuration.x.streaming_api_base_url == request.host may actually be wrong too, since that configuration option is a full URL potentially.

@ClearlyClaire
Copy link
Contributor Author

You are right, this comparison will always evaluate to false, as Rails.configuration.x.streaming_api_base_url is an URL (with scheme and potentially port number) while request.host is the host without port nor port number. I will address that in another PR.

This was referenced Jan 23, 2024
noellabo pushed a commit to fedibird/mastodon that referenced this pull request Jan 24, 2024
ttrace pushed a commit to ttrace/mastodon that referenced this pull request Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants