httpclient
httpclient copied to clipboard
Problems with base_url
Currently, the docs say that you can use base_url in this way: https://github.com/nahi/httpclient/blob/837d59f1ad19d5940c810e34e408d15897d5bacb/lib/httpclient.rb#L395
Reproduced here:
# After you set :base_url, all resources you pass to get, post and other
# methods are recognized to be prefixed with base_url. Say base_url is
# 'https://api.example.com/v1, get('/users') is the same as
# get('https://api.example.com/v1/users') internally.
However, this is actually untrue - the base url must end with a slash or it will be ignored:
[24] pry(main)> http.base_url = 'https://test.com/v1'
"https://test.com/v1"
[25] pry(main)> http.send :to_resource_url, 'test'
{
:scheme => "https",
:user => nil,
:password => nil,
:host => "test.com",
:port => 443,
:path => "/test",
:query => nil,
:fragment => nil
}
[26] pry(main)> http.send :to_resource_url, '/test'
{
:scheme => "https",
:user => nil,
:password => nil,
:host => "test.com",
:port => 443,
:path => "/test",
:query => nil,
:fragment => nil
}
This problem occurs on version 2.7.0.1 and may be related to https://github.com/nahi/httpclient/pull/269.
However, note that if you set the trailing slash in base_url, everything seems to work more or less how you'd expect:
[27] pry(main)> http.base_url = 'https://test.com/v1/'
"https://test.com/v1/"
[28] pry(main)> http.send :to_resource_url, 'test'
{
:scheme => "https",
:user => nil,
:password => nil,
:host => "test.com",
:port => 443,
:path => "/v1/test",
:query => nil,
:fragment => nil
}
[29] pry(main)> http.send :to_resource_url, '/test'
{
:scheme => "https",
:user => nil,
:password => nil,
:host => "test.com",
:port => 443,
:path => "/test",
:query => nil,
:fragment => nil
}
Note too that the appended resource cannot have a leading slash.
[24] pry(#<JSONClient>)> urify("https://localhost:8443/context/") + "/hi"
=> #<URI::HTTPS https://localhost:8443/hi>
[25] pry(#<JSONClient>)> urify("https://localhost:8443/context/") + "hi"
=> #<URI::HTTPS https://localhost:8443/context/hi>
This issue looks to be a regression since it was not present in version 2.5.2 which I had been using.
Looks like to_resource_url went from
urify(@base_url + uri)
to
urify(@base_url) + uri
Thanks @vincentwoo and @awood, and sorry for the regression. Obviously the document is obviously untrue.
Expected behavior for me and OK/NG of 2.7.0.1 implementation.
1. http://foo/bar/baz + path -> http://foo/bar/bazpath => NG (http://foo/bar/path)
2. http://foo/bar/baz/ + path -> http://foo/bar/baz/path => OK
3. http://foo/bar/baz + ../path -> http://foo/path => OK
4. http://foo/bar/baz/ + ../path -> http://foo/bar/path => OK
5. http://foo/bar/baz + ./path -> http://foo/bar/path => OK
6. http://foo/bar/baz/ + ./path -> http://foo/bar/baz/path => OK
7. http://foo/bar/baz + /path -> http://foo/path => OK
I'm unsure if my above expectation No.1 is right or not. I'm now thinking that current implementation URI.join(@base_url, path) is acceptable if I write it clear at the document.
Anyway it was a breaking change and sorry for any inconvenience it caused.
I fixed the document: https://github.com/nahi/httpclient/commit/f8a4680836dfa8ee23a2382360aee990c9280231 How do you think?
EDIT: oops, I introduced unnecessary change in the commit. I reverted and created a new commit here: https://github.com/nahi/httpclient/commit/357df32b87417714b3f1b6219d04e610ca1e9385
I think the feature is now more confusing, but if you believe this to be the correct decision, it's your call. I think the docs do not cover what happens when:
http://foo/bar + path
Should you get
http://foo/bar/path or http://foo/barpath? Ambiguous from the examples.
http://foo/bar + /path => http://foo/path
Is a severe regression against 2.6.1 which produced http:/foo/bar/path. Is there any intention maintain backwards compatibility?