Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14573 )
Change subject: webserver: add support for Knox URL rewriting ...................................................................... Patch Set 6: (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 enc To clarify, misapplied encoding (i.e. encoding when we shouldn't have) won't "break" Knox; it'll just mean the rewriting done by Knox will create broken links. Anyway, I'm gonna punt, as URL encoding in the web UI right now only happens as part of thread group names, so doing more validation here just for that feels like overkill. 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 e I'm tempted to say no because of the additional effort required to collect all those IDs, and the possibility of that stuff becoming stale as the web UI evolves. If it helps, any HTTP 404 will trigger an assert failure. -- 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: Sat, 02 Nov 2019 05:21:18 +0000 Gerrit-HasComments: Yes
