You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org> on 2019/08/15 23:05:23 UTC
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Thomas Tauber-Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14076
Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
......................................................................
IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Recent work has added support for HTTP authentication both for the
thrift hs2 interface and for the webserver. In both cases, we
mishandle HTTP keep-alive semantics when returning a 401 because we
close the connection but don't return a 'Connection: close' header,
even though we're using HTTP/1.1 where keep-alive is assumed, which
can cause clients to incorrectly believe that the connection has
remained open.
For the webserver, the fix is to enable keep-alive in squeasel so
that the connection isn't closed after the 401 is returned.
For the thrift hs2 interface, we throw an exception after the 401
which results in the connection being closed because otherwise it's
tricky with the way thrift is structured to ensure that the
unauthorized request isn't processed. So, the fix here is to return a
'Connection: close' header.
Testing:
- Ran existing HTTP auth tests.
- Manually tested in a cluster with connections to Impala proxied
through Apache Knox.
Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa
---
M be/src/transport/THttpServer.cpp
M be/src/util/webserver.cc
2 files changed, 5 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/14076/1
--
To view, visit http://gerrit.cloudera.org:8080/14076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa
Gerrit-Change-Number: 14076
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14076 )
Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/14076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa
Gerrit-Change-Number: 14076
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 21 Aug 2019 04:09:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/14076 )
Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa
Gerrit-Change-Number: 14076
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 20 Aug 2019 02:53:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14076 )
Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4824/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/14076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa
Gerrit-Change-Number: 14076
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 21 Aug 2019 00:01:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14076 )
Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4822/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/14076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa
Gerrit-Change-Number: 14076
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 20 Aug 2019 17:24:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14076 )
Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
......................................................................
Patch Set 2: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4822/
--
To view, visit http://gerrit.cloudera.org:8080/14076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa
Gerrit-Change-Number: 14076
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 20 Aug 2019 21:31:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/14076 )
Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
......................................................................
Patch Set 3: Code-Review+2
gvo failed due to unrelated IMPALA-8880
--
To view, visit http://gerrit.cloudera.org:8080/14076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa
Gerrit-Change-Number: 14076
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 21 Aug 2019 00:01:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Todd Lipcon, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14076
to look at the new patch set (#3).
Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
......................................................................
IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Recent work has added support for HTTP authentication both for the
thrift hs2 interface and for the webserver. In both cases, we
mishandle HTTP keep-alive semantics when returning a 401 because we
close the connection but don't return a 'Connection: close' header,
even though we're using HTTP/1.1 where keep-alive is assumed, which
can cause clients to incorrectly believe that the connection has
remained open.
For the webserver, the fix is to enable keep-alive in squeasel so
that the connection isn't closed after the 401 is returned.
For the thrift hs2 interface, we throw an exception after the 401
which results in the connection being closed because otherwise it's
tricky with the way thrift is structured to ensure that the
unauthorized request isn't processed. So, the fix here is to return a
'Connection: close' header.
Testing:
- Ran existing HTTP auth tests.
- Manually tested in a cluster with connections to Impala proxied
through Apache Knox.
Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa
---
M be/src/transport/THttpServer.cpp
M be/src/util/webserver.cc
2 files changed, 5 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/14076/3
--
To view, visit http://gerrit.cloudera.org:8080/14076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa
Gerrit-Change-Number: 14076
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14076 )
Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa
Gerrit-Change-Number: 14076
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 20 Aug 2019 17:24:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14076 )
Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
......................................................................
IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Recent work has added support for HTTP authentication both for the
thrift hs2 interface and for the webserver. In both cases, we
mishandle HTTP keep-alive semantics when returning a 401 because we
close the connection but don't return a 'Connection: close' header,
even though we're using HTTP/1.1 where keep-alive is assumed, which
can cause clients to incorrectly believe that the connection has
remained open.
For the webserver, the fix is to enable keep-alive in squeasel so
that the connection isn't closed after the 401 is returned.
For the thrift hs2 interface, we throw an exception after the 401
which results in the connection being closed because otherwise it's
tricky with the way thrift is structured to ensure that the
unauthorized request isn't processed. So, the fix here is to return a
'Connection: close' header.
Testing:
- Ran existing HTTP auth tests.
- Manually tested in a cluster with connections to Impala proxied
through Apache Knox.
Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa
Reviewed-on: http://gerrit.cloudera.org:8080/14076
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Thomas Tauber-Marshall <tm...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/transport/THttpServer.cpp
M be/src/util/webserver.cc
2 files changed, 5 insertions(+), 1 deletion(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
Thomas Tauber-Marshall: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/14076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa
Gerrit-Change-Number: 14076
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14076 )
Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4274/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/14076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa
Gerrit-Change-Number: 14076
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 15 Aug 2019 23:46:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14076 )
Change subject: IMPALA-8869: Fix handling of HTTP keep-alive when returning 401
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4317/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/14076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d5f80dbcde5b623a1d0586b5d763a062dd21afa
Gerrit-Change-Number: 14076
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 21 Aug 2019 00:44:17 +0000
Gerrit-HasComments: No