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

[C++]Fix libcurl miss auth header when broker return 307 #13112

Merged
merged 11 commits into from
Dec 9, 2021

Conversation

billowqiu
Copy link
Contributor

@billowqiu billowqiu commented Dec 3, 2021

Fixes #12888

Motivation

when broker restart, it will return 307 to client for lookup request.
but libcurl miss auth heander when follow new url which will issue the follow lookup req fail with 401.

Modifications

don't use libcurl CURLOPT_FOLLOWLOCATION to follow new url, instead use retry request with auth header.

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no )
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no )

Documentation

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

  • no-need-doc

@github-actions
Copy link

github-actions bot commented Dec 3, 2021

@billowqiu:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@wolfstudy
Copy link
Member

cc @BewareMyPower PTAL thanks

@BewareMyPower
Copy link
Contributor

Please use clang-format-5.0 to reformat your code first.

@github-actions
Copy link

github-actions bot commented Dec 3, 2021

@billowqiu:Thanks for providing doc info!

@github-actions github-actions bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Dec 3, 2021
Co-authored-by: Yunze Xu <xyzinfernity@163.com>
Copy link
Member

@wolfstudy wolfstudy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@315157973 315157973 merged commit dd8b473 into apache:master Dec 9, 2021
BewareMyPower pushed a commit that referenced this pull request Dec 10, 2021
* [C++]Fix libcurl miss auth header when broker return 307

Motivation
when broker restart, it will return 307 to client for lookup request.
but libcurl miss auth heander when follow new url which will issue the follow lookup req fail with 401.

Modifications
don't use libcurl CURLOPT_FOLLOWLOCATION to follow new url, instead use retry request with auth header.
@315157973 315157973 added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Dec 13, 2021
fxbing pushed a commit to fxbing/pulsar that referenced this pull request Dec 19, 2021
* [C++]Fix libcurl miss auth header when broker return 307

Motivation
when broker restart, it will return 307 to client for lookup request.
but libcurl miss auth heander when follow new url which will issue the follow lookup req fail with 401.

Modifications
don't use libcurl CURLOPT_FOLLOWLOCATION to follow new url, instead use retry request with auth header.
codelipenghui pushed a commit that referenced this pull request Dec 22, 2021
* [C++]Fix libcurl miss auth header when broker return 307

Motivation
when broker restart, it will return 307 to client for lookup request.
but libcurl miss auth heander when follow new url which will issue the follow lookup req fail with 401.

Modifications
don't use libcurl CURLOPT_FOLLOWLOCATION to follow new url, instead use retry request with auth header.

(cherry picked from commit dd8b473)
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Dec 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.2 release/2.9.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CPP SDK did not respond to request 307 during lookup
5 participants