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

Make the ALLOWED_HOSTS threadsafe #1054

Open
nhorton opened this issue Apr 10, 2024 · 0 comments
Open

Make the ALLOWED_HOSTS threadsafe #1054

nhorton opened this issue Apr 10, 2024 · 0 comments

Comments

@nhorton
Copy link

nhorton commented Apr 10, 2024

Right now, the ALLOW list is global and meant to be set in initialization. We have some very good reasons we want to dynamically allow and disallow some hosts, namely with only a few tests needing to hit that endpoint, and not wanting others to do it accidentally.

I would be game to add the APIs myself for this, but I wanted to confirm that you are good with my approach first before I put in the time.

Proposal

  1. Leave the existing WebMock.disable_net_connect!(allow:) declaration unaltered fully
  2. Add a new `WebMock.with_allowed_connections(*allowed)
  3. That block method would set a thread-local var of the local allowances, and remove it in an ensure
  4. Replace the addr_accessor for allow on config with a version that joins the static variable with the thread variable

Workaround

The alternative I see is a workaround of using a proc as an arg to the static allow list, and having that check the Thread.current store for additional values and comparing. That is what I am about to do for now, but it feels pretty janky.

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

No branches or pull requests

1 participant