When Addressable::URI is used, the `port` property will be nil unless
port was explicitly specified. This made Addressable unusable in HTTPS
scenarios with Net::HTTP adapters, which would default to port 80 even
when the scheme was "https".
This came up while debugging an issue with the latest Berkshelf
release. It seems that the net_http.rb adapter did this in the exact
same way (calls #set_default_paths on the X509::Store) but for some
reason it was neglected when httpclient.rb adapter was written.
It is expectation that, by default, the system certificate store is utilized.
Without this change, a connection's header and each request's header
all reference the same `@names` hash object so any request that modified
it via delete or other mutating methods, would affect the headers of other
requests and the connection.
In [travis.rb](9b3eb05542/lib/travis/client/session.rb (L206-209)), it was doing:
```ruby
connection.public_send(verb, url, *args) do |request|
next if request.path !~ /^https?:/ or request.path.start_with? api_endpoint
request.headers.delete("Authorization")
end
```
This caused the headers of multiple requests (and the connection) to share
the same `@names` hash object, resulting in `@names` and the Header hash
to go out of sync:
thing| @names | Header
--- | --- | --- | ---
connection | no Authorization | Has Authorization
request 1 | no Authorization | no Authorization
request 2 | no Authorization | Has Authorization
Because Headers uses the `@names` to determine if it `includes?` a key or
if there's a key to `delete`, you would have Authorization in the headers
but it wouldn't be removed because `@names` didn't have it.
Synopsis
--------
The `Env` that gets stored on the `Response` does not contain
a reference to that response. This env is a copy of the env that
was passed through the middleware stack. Thus if the env
passed to `app.call` is used, it has the response, but if the one
from the `Response` is used, then it does not. This happens when,
for example, you invoke the `on_complete` hook on the response,
after it has finished.
Fixes Longstanding VCR issue
----------------------------
Fixes https://github.com/vcr/vcr/issues/386
A longstanding issue in VCR's integration with v0.9.x
It was worked around in this commit: https://github.com/vcr/vcr/pull/439
And is merged into master, but is not released.
Merging this will fix the issue, without the changes to VCR needing to
be released.
The old code, which is currently released, and gets its env from the
Request can be seen
[here](cb9696c44d%5E/lib/vcr/middleware/faraday.rb#L99-105)
Fixes Faraday Issue
-------------------
Fixes https://github.com/lostisland/faraday/issues/441
which identifies this same problem.
Closes Faraday PR
-----------------
I'm going to recommend closing https://github.com/lostisland/faraday/pull/360/files
as the issue was probably introduced
[here](04a8514cba (diff-fc9726b31994223d11a212b5169100fdL68))
when `Response#finish` changed from storing the `env` directly,
to storing a copy:
```
- @env = env
+ @env = Env.from(env)
```
Presumably this behaviour is still desired, but the mentioned pull
essentially undoes this by returning the same `env` from `Env.from`
The commit that introduced the issue is probably good, but is called by
the app in the middleware stack that creates the response, before it
sets the rsponse onto the env. This means that the copy does not have
the link to the response.
81f16593a0/lib/faraday/rack_builder.rb (L152-155)
Not sure about the test name/location
-------------------------------------
I had difficulty figuring out where to put this test, and what to name
it. I eventually went with the test on the `middleware_stack_test.rb`,
because it deals with the `RackBuilder`, but if there's a better place,
or a better name, let me know and I'll update it.
One other weird thing
---------------------
Depending on when the `Request#on_complete` callback is invoked,
you're either getting the original env, or the copy:
81f16593a0/lib/faraday/response.rb (L64-65)
In this order the @env would not be changed by the "on complete"
callbacks, meaning that the response would not change after parallel
requests finish (e.g. JSON would not be parsed).
This would work
def on_complete(env)
env[:body].upcase!
end
But this wouldn't
def on_complete(env)
env[:body] = env[:body].upcase
end
The headers hash contains some local state inside the @names instance variable as a means of doing case-insensitive hashes. This has the side-effect that one has to remember to perform mirror operations on the @names hash in addition to the object itself(Headers).
A case in point is the replace method – which did not clear out the @names hash which was causing side-effects by not performing a mirror operation (clear) on the @names hash.
Only run retries if the method is known to be
idempotent. If it isn't, switch to use the retry_if
block to make the final decision, if it is known to
be idempotent allow it to be retried by default.
retry_if allows users to provide a block that
will receive the env and exception objects
and should decide if the call should be retried
or not.
Fixes#298.
