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

[Go] Flight AuthenticateBasicToken is unreliable behind proxies #38198

Closed
phillipleblanc opened this issue Oct 11, 2023 · 0 comments · Fixed by #38199
Closed

[Go] Flight AuthenticateBasicToken is unreliable behind proxies #38198

phillipleblanc opened this issue Oct 11, 2023 · 0 comments · Fixed by #38199
Assignees
Milestone

Comments

@phillipleblanc
Copy link
Contributor

Describe the bug, including details regarding any error messages, version, and platform.

About 10-25% of Flight requests using the Go Flight client library will fail with the following error when behind a proxy like Cloudflare:

rpc error: code = Unknown desc = unexpected HTTP status code received from server: 520 (); malformed header: missing HTTP content-type

When comparing the implementations of the C++ library and Go library, I noticed that the C++ library closed sending side of the bi-directional Handshake stream immediately after opening it, whereas the Go library tries to get the headers before closing the sending stream.

After re-ordering the Go code to close the sending stream first, the above error disappeared.

Component(s)

Go

zeroshade pushed a commit that referenced this issue Oct 11, 2023
…es (#38199)

### Rationale for this change

Fixes a bug in the Go Flight client library that makes using AuthenticateBasicToken unreliable behind proxies.

### What changes are included in this PR?

Closes the sending side of the bi-directional stream for the Flight Handshake RPC call before trying to read the headers. This matches the C++ implementation.

<img width="609" alt="Screenshot 2023-10-11 at 6 18 40 PM" src="https://github.com/apache/arrow/assets/879445/05e23c6a-0ff8-41fc-825b-8add7fe938bc">

### Are these changes tested?

I've tested these changes against my service deployed behind CloudFlare and verified the error listed in the linked issue disappears.

### Are there any user-facing changes?

No
* Closes: #38198

Authored-by: Phillip LeBlanc <phillip@leblanc.tech>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
@zeroshade zeroshade added this to the 15.0.0 milestone Oct 11, 2023
llama90 pushed a commit to llama90/arrow that referenced this issue Oct 12, 2023
… proxies (apache#38199)

### Rationale for this change

Fixes a bug in the Go Flight client library that makes using AuthenticateBasicToken unreliable behind proxies.

### What changes are included in this PR?

Closes the sending side of the bi-directional stream for the Flight Handshake RPC call before trying to read the headers. This matches the C++ implementation.

<img width="609" alt="Screenshot 2023-10-11 at 6 18 40 PM" src="https://github.com/apache/arrow/assets/879445/05e23c6a-0ff8-41fc-825b-8add7fe938bc">

### Are these changes tested?

I've tested these changes against my service deployed behind CloudFlare and verified the error listed in the linked issue disappears.

### Are there any user-facing changes?

No
* Closes: apache#38198

Authored-by: Phillip LeBlanc <phillip@leblanc.tech>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
JerAguilon pushed a commit to JerAguilon/arrow that referenced this issue Oct 23, 2023
… proxies (apache#38199)

### Rationale for this change

Fixes a bug in the Go Flight client library that makes using AuthenticateBasicToken unreliable behind proxies.

### What changes are included in this PR?

Closes the sending side of the bi-directional stream for the Flight Handshake RPC call before trying to read the headers. This matches the C++ implementation.

<img width="609" alt="Screenshot 2023-10-11 at 6 18 40 PM" src="https://github.com/apache/arrow/assets/879445/05e23c6a-0ff8-41fc-825b-8add7fe938bc">

### Are these changes tested?

I've tested these changes against my service deployed behind CloudFlare and verified the error listed in the linked issue disappears.

### Are there any user-facing changes?

No
* Closes: apache#38198

Authored-by: Phillip LeBlanc <phillip@leblanc.tech>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
… proxies (apache#38199)

### Rationale for this change

Fixes a bug in the Go Flight client library that makes using AuthenticateBasicToken unreliable behind proxies.

### What changes are included in this PR?

Closes the sending side of the bi-directional stream for the Flight Handshake RPC call before trying to read the headers. This matches the C++ implementation.

<img width="609" alt="Screenshot 2023-10-11 at 6 18 40 PM" src="https://github.com/apache/arrow/assets/879445/05e23c6a-0ff8-41fc-825b-8add7fe938bc">

### Are these changes tested?

I've tested these changes against my service deployed behind CloudFlare and verified the error listed in the linked issue disappears.

### Are there any user-facing changes?

No
* Closes: apache#38198

Authored-by: Phillip LeBlanc <phillip@leblanc.tech>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
… proxies (apache#38199)

### Rationale for this change

Fixes a bug in the Go Flight client library that makes using AuthenticateBasicToken unreliable behind proxies.

### What changes are included in this PR?

Closes the sending side of the bi-directional stream for the Flight Handshake RPC call before trying to read the headers. This matches the C++ implementation.

<img width="609" alt="Screenshot 2023-10-11 at 6 18 40 PM" src="https://github.com/apache/arrow/assets/879445/05e23c6a-0ff8-41fc-825b-8add7fe938bc">

### Are these changes tested?

I've tested these changes against my service deployed behind CloudFlare and verified the error listed in the linked issue disappears.

### Are there any user-facing changes?

No
* Closes: apache#38198

Authored-by: Phillip LeBlanc <phillip@leblanc.tech>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants