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

Deprecate assigning single trusted_proxies value #40789

Merged
merged 1 commit into from Mar 24, 2021

Conversation

csutter
Copy link
Contributor

@csutter csutter commented Dec 10, 2020

Summary

Fixes #40772

  • Deprecate ability to assign a single value to trusted_proxies in RemoteIp middleware
  • Add an explicit test for the setting overriding the default list of trusted proxies

The RemoteIp middleware currently behaves inconsistently depending on whether config.action_dispatch.trusted_proxies is configured with a single value or an enumerable. This is surprising behaviour and can lead to frustration when e.g. moving from having one single trusted proxy specified to needing multiple (and vice versa).

For example, the following two settings behave differently:

# Adds 4.2.42.0/24 to default list of trusted proxies
app.config.action_dispatch.trusted_proxies = IPAddr.new("4.2.42.0/24")

# Replaces default list of trusted proxies with 4.2.42.0/24
app.config.action_dispatch.trusted_proxies = [IPAddr.new("4.2.42.0/24")]

@csutter
Copy link
Contributor Author

csutter commented Dec 10, 2020

Got the original issue number wrong in the commit message and changelog, whoops! 😅 Re-pushed.

@csutter csutter force-pushed the deprecate-remote-ip-single-value branch 2 times, most recently from b853b02 to a1b1009 Compare December 10, 2020 19:08
Fixes rails#40772

The `RemoteIp` middleware currently behaves inconsistently depending on
whether `config.action_dispatch.trusted_proxies` is configured with a
single value or an enumerable.

- Deprecate ability to assign a single value to `trusted_proxies` in
  `RemoteIp` middleware
- Add an explicit test for the setting overriding the default list of
  trusted proxies
@csutter csutter force-pushed the deprecate-remote-ip-single-value branch from a1b1009 to bdd49dc Compare December 10, 2020 19:47
Base automatically changed from master to main January 14, 2021 17:02
@rafaelfranca rafaelfranca merged commit 7702ce8 into rails:main Mar 24, 2021
csutter added a commit to csutter/rails that referenced this pull request Mar 26, 2021
The way this test initializes `ActionDispatch::RemoteIp` has been
deprecated in rails#40789. This makes the test append to the existing
trusted proxy list instead of assigning a single value.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent behaviour of config.action_dispatch.trusted_proxies depending on value
2 participants