Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14573 )
Change subject: webserver: add support for Knox URL rewriting ...................................................................... Patch Set 6: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/14573/6/src/kudu/integration-tests/webserver-crawl-itest.cc File src/kudu/integration-tests/webserver-crawl-itest.cc: http://gerrit.cloudera.org:8080/#/c/14573/6/src/kudu/integration-tests/webserver-crawl-itest.cc@235 PS6, Line 235: link = StringReplace(link, kKnoxPrefix, "", /* replace_all= */ false); Is there a way we could check whether 'link' at this point is _not_ URL encoded? Since this block is in charge of verifying the correctness of the links sent to Knox, probably worth some kind of validation if not doing so would break Knox. E.g. maybe Knoxifying the URL, and if it's different, checking that the pre-Knoxified link is broken? http://gerrit.cloudera.org:8080/#/c/14573/6/src/kudu/integration-tests/webserver-crawl-itest.cc@314 PS6, Line 314: vector<string> sorted_urls(urls_seen.begin(), urls_seen.end()); : std::sort(sorted_urls.begin(), sorted_urls.end()); : LOG(INFO) << "Dumping visited URLs"; : for (const auto& u : sorted_urls) { : LOG(INFO) << u; : } : } Do you think it's worth even a basic check that we've seen some pages for every tserver ID, every tablet ID, and our table ID? -- To view, visit http://gerrit.cloudera.org:8080/14573 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee92cb094b81609356acf858b7c549b6c281a7e5 Gerrit-Change-Number: 14573 Gerrit-PatchSet: 6 Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 01 Nov 2019 18:44:24 +0000 Gerrit-HasComments: Yes
