Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22573 )
Change subject: KUDU-3646 fix Base64Decode() ...................................................................... Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/22573/5/src/kudu/util/url-coding.h File src/kudu/util/url-coding.h: http://gerrit.cloudera.org:8080/#/c/22573/5/src/kudu/util/url-coding.h@59 PS5, Line 59: input > nit: input. period not needed here http://gerrit.cloudera.org:8080/#/c/22573/5/src/kudu/util/url-coding.cc File src/kudu/util/url-coding.cc: http://gerrit.cloudera.org:8080/#/c/22573/5/src/kudu/util/url-coding.cc@43 PS5, Line 43: libs > nit: libs. period not needed here http://gerrit.cloudera.org:8080/#/c/22573/5/src/kudu/util/url-coding.cc@195 PS5, Line 195: data > nit: data. period not needed here http://gerrit.cloudera.org:8080/#/c/22573/5/src/kudu/util/url-coding.cc@197 PS5, Line 197: output > nit: output. period not needed here http://gerrit.cloudera.org:8080/#/c/22573/5/src/kudu/util/url-coding.cc@212 PS5, Line 212: "non-trailing '=' > Would it make sense to log this for release build? No, I don't think so. The idea is to avoid going through all the characters of the input string for a release build. As of now, I don't think there is any use case that supplies such concatenated data, and the NOTE in the inline docs for Base64Decode() clearly states that such input isn't handled properly. Even if anything is added in the future, there should be a test scenario to cover new functionality, and such a condition should be caught by this DCHECK at least in one of non-release builds (DEBUG, ASAN, TSAN) -- that's what this DCHECK() is for. -- To view, visit http://gerrit.cloudera.org:8080/22573 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f5d41eb4e41897d520f00b12c6e487ecc5a00fc Gerrit-Change-Number: 22573 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Thu, 06 Mar 2025 19:57:54 +0000 Gerrit-HasComments: Yes
