Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22780 )
Change subject: IMPALA-13746: Fix long ldap password handling in impala-shell+hs2-http ...................................................................... Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/22780/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22780/1//COMMIT_MSG@14 PS1, Line 14: Based on impyla fix https://github.com/cloudera/impyla/pull/562 by : https://github.com/paulmayer > nit: mention that his fix exist in Impyla 0.21a3 Done http://gerrit.cloudera.org:8080/#/c/22780/1/fe/src/test/java/org/apache/impala/testutil/LdapUtil.java File fe/src/test/java/org/apache/impala/testutil/LdapUtil.java: http://gerrit.cloudera.org:8080/#/c/22780/1/fe/src/test/java/org/apache/impala/testutil/LdapUtil.java@44 PS1, Line 44: public static final String TEST_PASSWORD_LONG = : "verylongpasswordthatcreatesalongbasic64encoding" > What is the maximum limit for this? Is this the maximum? I don't know if there is a maximum length. The current password is enough to trigger the new line issue. http://gerrit.cloudera.org:8080/#/c/22780/1/fe/src/test/java/org/apache/impala/testutil/LdapUtil.java@45 PS1, Line 45: n > Nit: "encoding". Done http://gerrit.cloudera.org:8080/#/c/22780/1/shell/impala_client.py File shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/22780/1/shell/impala_client.py@436 PS1, Line 436: user_passwd = "{0}:{1}".format(self.user, self.ldap_password) : # Produce RFC 2617-compliant basic credentials: : # - RFC 2045 encoding of username:password without limitations to 76 chars : # per line (and without trailing newline) : # - No translation of characters (+,/) for URL-safety : auth = base64.b64encode(user_passwd.encode()).decode() > This remove sys.version_info.major < 3 check. we have minimal ldap tests in https://github.com/apache/impala/blob/c5a0ec8bdffd958984c0f6d34e0fcb8acc138457/tests/shell/test_shell_commandline.py#L837 the issue is that without a running ldap server the test cannot check whether the authentication actually works. Tried with connecting to a running non-ldap impala, but the test hangs in this case, so I would prefer not to add it. -- To view, visit http://gerrit.cloudera.org:8080/22780 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d73d682cf2d1843d9801ef71b99d551b79deb19 Gerrit-Change-Number: 22780 Gerrit-PatchSet: 2 Gerrit-Owner: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Comment-Date: Tue, 15 Apr 2025 18:35:04 +0000 Gerrit-HasComments: Yes
