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

Crash on response 'Link" header parsing #1325

Closed
ivailop opened this issue Feb 16, 2021 · 3 comments
Closed

Crash on response 'Link" header parsing #1325

ivailop opened this issue Feb 16, 2021 · 3 comments

Comments

@ivailop
Copy link
Contributor

ivailop commented Feb 16, 2021

Since yesterday, the "Link" header returned when doing Web application flow, i.e.

POST  login/oauth/access_token

is formatted in a way that results in a crash when calling exchange_code_for_token:

NoMethodError: undefined method `captures' for nil:NilClass
.../vendor/bundle/ruby/2.6.0/gems/sawyer-0.8.2/lib/sawyer/response.rb:52:in `block in process_rels'
.../vendor/bundle/ruby/2.6.0/gems/sawyer-0.8.2/lib/sawyer/response.rb:51:in `map'
.../vendor/bundle/ruby/2.6.0/gems/sawyer-0.8.2/lib/sawyer/response.rb:51:in `process_rels'
.../vendor/bundle/ruby/2.6.0/gems/sawyer-0.8.2/lib/sawyer/response.rb:20:in `initialize'
.../vendor/bundle/ruby/2.6.0/gems/sawyer-0.8.2/lib/sawyer/agent.rb:107:in `new'
.../vendor/bundle/ruby/2.6.0/gems/sawyer-0.8.2/lib/sawyer/agent.rb:107:in `call'
.../vendor/bundle/ruby/2.6.0/gems/octokit-4.19.0/lib/octokit/connection.rb:156:in `request'
.../vendor/bundle/ruby/2.6.0/gems/octokit-4.19.0/lib/octokit/connection.rb:28:in `post'
.../vendor/bundle/ruby/2.6.0/gems/octokit-4.19.0/lib/octokit/client/users.rb:57:in `exchange_code_for_token'

Besides questioning why GitHub API changed, apparently sawyer has a bug!

FYI, to address the bug, in sawyer, I opened lostisland/sawyer#68 issue and related lostisland/sawyer#69 PR.

@osowskit
Copy link

@swinton any insight into recent changes that may have broken this?

@jeremie-stripe
Copy link

If it helps anyone, we are working around this by introducing a Faraday middleware step to strip the Link header in this situation (since the JS libraries are irrelevant in this case):

class LinkHeaderRemover < Faraday::Response::Middleware
  OAUTH_LOGIN_URL_PATH = "/login/oauth/access_token"

  def on_complete(env)
    request_path = URI(env.url).path
    return if request_path != OAUTH_LOGIN_URL_PATH

    response_headers = env.response_headers
    return if response_headers.nil?

    response_headers.delete("link")
  end
end

Setup in your pipeline like so:

middleware = Octokit::Default.middleware.dup
middleware.insert_before(middleware.handlers.length - 1, LinkHeaderRemover)
Octokit.middleware = middleware

@tarebyte
Copy link
Member

I think this has been fixed on the GitHub side, is anyone running into these issues still?

@tarebyte tarebyte closed this as completed Aug 2, 2021
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

4 participants