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/27 16:56:32 UTC

[Impala-ASF-CR] Fix THttpServer to not call the cookie function with an empty cookie 

Thomas Tauber-Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14149


Change subject: Fix THttpServer to not call the cookie function with an empty cookie
......................................................................

Fix THttpServer to not call the cookie function with an empty cookie

This patch checks if the value passed in the 'Cookie' header to the
http hs2 server is blank, and if so it ignores it.

The reason to do this is so that a client sending an empty cookie
header isn't counted as a failed cookie attempt, which is incorrect.

Change-Id: I04e96fe97baae474a82fd30f2cd55ccce80570b4
---
M be/src/transport/THttpServer.cpp
1 file changed, 7 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/14149/1
-- 
To view, visit http://gerrit.cloudera.org:8080/14149
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I04e96fe97baae474a82fd30f2cd55ccce80570b4
Gerrit-Change-Number: 14149
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] Fix THttpServer to not call the cookie function with an empty cookie

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14149 )

Change subject: Fix THttpServer to not call the cookie function with an empty cookie
......................................................................


Patch Set 2: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/14149
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04e96fe97baae474a82fd30f2cd55ccce80570b4
Gerrit-Change-Number: 14149
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@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-Comment-Date: Tue, 27 Aug 2019 21:46:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Fix THttpServer to not call the cookie function with an empty cookie

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Attila Jeges, Tim Armstrong, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/14149

to look at the new patch set (#2).

Change subject: Fix THttpServer to not call the cookie function with an empty cookie
......................................................................

Fix THttpServer to not call the cookie function with an empty cookie

This patch checks if the value passed in the 'Cookie' header to the
http hs2 server is blank, and if so it ignores it.

The reason to do this is so that a client sending an empty cookie
header isn't counted as a failed cookie attempt, which is incorrect.

Change-Id: I04e96fe97baae474a82fd30f2cd55ccce80570b4
---
M be/src/transport/THttpServer.cpp
1 file changed, 9 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/14149/2
-- 
To view, visit http://gerrit.cloudera.org:8080/14149
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I04e96fe97baae474a82fd30f2cd55ccce80570b4
Gerrit-Change-Number: 14149
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] Fix THttpServer to not call the cookie function with an empty cookie

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14149 )

Change subject: Fix THttpServer to not call the cookie function with an empty cookie
......................................................................


Patch Set 1: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/14149
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04e96fe97baae474a82fd30f2cd55ccce80570b4
Gerrit-Change-Number: 14149
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@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-Comment-Date: Tue, 27 Aug 2019 20:39:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Fix THttpServer to not call the cookie function with an empty cookie

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/14149 )

Change subject: Fix THttpServer to not call the cookie function with an empty cookie
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14149/1/be/src/transport/THttpServer.cpp
File be/src/transport/THttpServer.cpp:

http://gerrit.cloudera.org:8080/#/c/14149/1/be/src/transport/THttpServer.cpp@179
PS1, Line 179:     // If a 'Cookie' header was provided with an empty value, we ignore it rather than
> Maybe leave a one line comment to explain the significance of the check, so
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/14149
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04e96fe97baae474a82fd30f2cd55ccce80570b4
Gerrit-Change-Number: 14149
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@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-Comment-Date: Tue, 27 Aug 2019 17:12:30 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] Fix THttpServer to not call the cookie function with an empty cookie

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14149 )

Change subject: Fix THttpServer to not call the cookie function with an empty cookie
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/4392/ : 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/14149
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04e96fe97baae474a82fd30f2cd55ccce80570b4
Gerrit-Change-Number: 14149
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@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-Comment-Date: Tue, 27 Aug 2019 17:13:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Fix THttpServer to not call the cookie function with an empty cookie

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14149 )

Change subject: Fix THttpServer to not call the cookie function with an empty cookie
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/4393/ : 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/14149
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04e96fe97baae474a82fd30f2cd55ccce80570b4
Gerrit-Change-Number: 14149
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@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-Comment-Date: Tue, 27 Aug 2019 17:59:21 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Fix THttpServer to not call the cookie function with an empty cookie

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/14149 )

Change subject: Fix THttpServer to not call the cookie function with an empty cookie
......................................................................

Fix THttpServer to not call the cookie function with an empty cookie

This patch checks if the value passed in the 'Cookie' header to the
http hs2 server is blank, and if so it ignores it.

The reason to do this is so that a client sending an empty cookie
header isn't counted as a failed cookie attempt, which is incorrect.

Change-Id: I04e96fe97baae474a82fd30f2cd55ccce80570b4
Reviewed-on: http://gerrit.cloudera.org:8080/14149
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/transport/THttpServer.cpp
1 file changed, 9 insertions(+), 5 deletions(-)

Approvals:
  Tim Armstrong: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/14149
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I04e96fe97baae474a82fd30f2cd55ccce80570b4
Gerrit-Change-Number: 14149
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@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>

[Impala-ASF-CR] Fix THttpServer to not call the cookie function with an empty cookie

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14149 )

Change subject: Fix THttpServer to not call the cookie function with an empty cookie
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/14149
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04e96fe97baae474a82fd30f2cd55ccce80570b4
Gerrit-Change-Number: 14149
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@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-Comment-Date: Tue, 27 Aug 2019 17:14:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Fix THttpServer to not call the cookie function with an empty cookie

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14149 )

Change subject: Fix THttpServer to not call the cookie function with an empty cookie
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14149/1/be/src/transport/THttpServer.cpp
File be/src/transport/THttpServer.cpp:

http://gerrit.cloudera.org:8080/#/c/14149/1/be/src/transport/THttpServer.cpp@179
PS1, Line 179:     if (!cookie_value_.empty()) {
Maybe leave a one line comment to explain the significance of the check, so that future people don't attempt to simplify it :)



-- 
To view, visit http://gerrit.cloudera.org:8080/14149
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04e96fe97baae474a82fd30f2cd55ccce80570b4
Gerrit-Change-Number: 14149
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Aug 2019 17:08:12 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] Fix THttpServer to not call the cookie function with an empty cookie

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14149 )

Change subject: Fix THttpServer to not call the cookie function with an empty cookie
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4859/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/14149
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04e96fe97baae474a82fd30f2cd55ccce80570b4
Gerrit-Change-Number: 14149
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@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-Comment-Date: Tue, 27 Aug 2019 17:16:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Fix THttpServer to not call the cookie function with an empty cookie

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14149 )

Change subject: Fix THttpServer to not call the cookie function with an empty cookie
......................................................................


Patch Set 1:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4858/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/14149
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04e96fe97baae474a82fd30f2cd55ccce80570b4
Gerrit-Change-Number: 14149
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Aug 2019 16:58:15 +0000
Gerrit-HasComments: No