[webkit-reviews] review granted: [Bug 100805] garden-o-matic should work for local results : [Attachment 171569] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 100805: garden-o-matic should work for local results
https://bugs.webkit.org/show_bug.cgi?id=100805

Attachment 171569: Patch
https://bugs.webkit.org/attachment.cgi?id=171569&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=171569&action=review


This seems like a reasonable approach.	I'm surprised we didn't need to add
CORS headers.  Perhaps we do that already?

>
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/garden
-o-matic.html:35
> -					    img-src 'self'
https://ajax.googleapis.com http://build.chromium.org http://build.webkit.org
file:;
> +					    img-src 'self'
https://ajax.googleapis.com http://build.chromium.org http://build.webkit.org
file: http://127.0.0.1:8127;

You'll probably need to add this to media-src and frame-src as well.

>
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/script
s/results.js:192
> +    return resultsDirectoryListingURL(platform, builderName) +
'results/layout-test-results';

Why did you delete the kLayoutTestResultsPath constant?

> Tools/Scripts/webkitpy/tool/servers/gardeningserver.py:139
> +	   if not filesystem.isabs(path) and
self.server.options.results_directory:
> +	       fullpath =
filesystem.abspath(filesystem.join(self.server.options.results_directory,
path))

Can we add some to defend against directory traversal here?  Perhaps check that
fullpath is actually inside results_directory?