-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Incorrectly builds urls for Github Enterprise API endpoints #831
Comments
Liucevse |
@tarebyte any thoughts here? |
@toddmohney @gordondiggs let me take a look to see if I can help you resolve this issue, apologies for the delay. |
@toddmohney @gordondiggs after playing around with this for a bit, I think the patch is going to have to be upstream to Faraday. Without some serious hackery I don't see how we can fix this on the Octokit side of things. |
pengwynn
added a commit
that referenced
this issue
Jan 5, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The default Github Enterprise installation exposes its API at endpoints
prefixed with
/api/v3
. The current version of Octokit, however, has a bugwhere when configuring the
api_endpoint
, as recommended, results in requests to anincorrect url.
Expected behavior
Configuring the Octokit client's
api_endpoint
value asthen calling
client.check_application_authorization("some-token")
should result in a request to
Actual behavior
A request is made to
Notice the omitted
/api/v3
Steps to reproduce
We've put together a repository with a simple demonstration of this bug. Run
rspec spec/test.rb
from the root of this repositoryInvestigation
The problem can be traced down to where the request url is built in Faraday
On these lines,
base
is aURI
instance and the+
operator is defined onURI
as an alias tomerge
which does not exhibit the same behavior as concatenation and is why the/api/v3
portion of the path gets lost.Proposed resolution
At the moment, I'm not certain how to correct this issue from Octokit's point of view, but am happy to help.
The text was updated successfully, but these errors were encountered: