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

location compatibility with allow_other_host #237

Open
boardfish opened this issue Oct 28, 2022 · 2 comments
Open

location compatibility with allow_other_host #237

boardfish opened this issue Oct 28, 2022 · 2 comments

Comments

@boardfish
Copy link

boardfish commented Oct 28, 2022

When raise_on_open_redirects is enabled in Rails config, redirect_to now raises UnsafeRedirectError in case the host of the redirect location does not match that of the current request. It's ordinarily possible when using redirect_to to specify allow_other_host: true to prevent this error from being raised in situations where UnsafeRedirectError did not need to be raised.

However, if the location param is used, it isn't possible to do this - the redirect path is expected as a single variable.

This does not work:

respond_with @thing, location: redirect_path, allow_other_host: true

I also attempted this:

respond_with @thing do |format|
  format.html { redirect_to redirect_path, allow_other_host: true }
end

But this causes a regression in that I'm reliant on respond_with's handling of an invalid record, particularly the error flash.

I'm happy to look into a fix for this myself, but I could perhaps do with some guidance. I think all we might need is to pluck allow_other_host from options and pass this into this call, but we may have room to afford ourselves something more flexible than that.

@jotolo
Copy link

jotolo commented Sep 10, 2024

Hey @boardfish , did you manage to find any working workaround to this issue?

@boardfish
Copy link
Author

Afraid not, sorry – the instance where our code was falling foul of UnsafeRedirectError with respond_with was a one-off that we decided against fixing a couple of weeks after I raised this issue.

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

No branches or pull requests

2 participants