Make the ALLOWED_HOSTS threadsafe

[nhorton]

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.

[bblimke]

@nhorton Thank you for the suggestion.

I believe this would be a useful feature. One of the things I would like to change in WebMock over time is to move away from globals, including stub declarations.

My main concern is that the configuration should be intuitive.

I wonder how the with_allowed_connections configuration should behave when allow_net_connect is already configured. Can with_allowed_connections be invoked without a previous disable_net_connect! invocation? My guess is that it should, but then with_allowed_connections needs to do more than just extend the allow set.

I'm also considering whether Thread.current is the right choice to store the additional allow values. While I understand the purpose and appreciate that the configuration won't leak between threads, it might not work as expected if the block starts any threads where each thread is expected to make a request, or if the HTTP client library uses threads for async requests (like the httpclient gem).

What are your thoughts on that?

[nhorton]

@bblimke - sorry for being insanely late in responding. I missed your note at the time and am coming back to this because we are getting bitten by it severely now.

The point about how Thread.current won't work with some of the gems that use threads for async requests is a very good one. I have been trying to figure out a way around that, and I don't see a reasonable one. You can't do anything like looking up parent threads so there is not a reasonable way I can see on that. I admit I am a little stumped on that. The only thing I can see is trying to stub out the behavior of the client libraries pre-thread-spawning, which is obviously a pretty big change.

I wish I had better ideas.