Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2693953006:
Isolate: Download files as their filename instead of hash (Closed)
DescriptionIsolate: Download files as their filename instead of hash
BUG=689728
R=maruel@chromium.org
Review-Url: https://codereview.chromium.org/2693953006
Committed: https://github.com/luci/luci-py/commit/07e9f35001395b3ce620471b4896c6fb29020330
Patch Set 1 #
Total comments: 14
Patch Set 2 : Isolate: Download files as their filename instead of hash #
Total comments: 7
Patch Set 3 : Isolate: Download files as their filename instead of hash #
Total comments: 15
Patch Set 4 : Isolate: Download files as their filename instead of hash #Patch Set 5 : Isolate: Download files as their filename instead of hash #Patch Set 6 : Isolate: Download files as their filename instead of hash #
Total comments: 6
Patch Set 7 : Isolate: Download files as their filename instead of hash #
Messages
Total messages: 30 (5 generated)
Change can be tried here: https://2621-60d946d-tainted-jonesmi-dot-isolateserver-dev.appspot.com/browse...
https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... File appengine/isolate/handlers_frontend.py (right): https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... appengine/isolate/handlers_frontend.py:199: u'as': unicode(save_as), keep keys sorted https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... appengine/isolate/handlers_frontend.py:279: content = cgi.escape(content) $ python -c 'import cgi,json; print json.loads(cgi.escape(u"\"\\u003cgotcha\\u003e\""))' <gotcha> you may want to double check your assumptions. I recommend to escape every single string found in the parsed data, just before doing the html injection below but after checking that it's actually worth processing. https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... appengine/isolate/handlers_frontend.py:285: if 'files' in data: Let's add some more checks. Ref: https://github.com/luci/luci-py/blob/master/appengine/isolate/doc/Design.md#i... if not isinstance(data, dict): return content actual = set(data) if not actual.issubset(('algo', 'command', 'files', 'includes', 'read_only', 'relative_cwd', 'version')): return content if 'files' not in data: return content https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... appengine/isolate/handlers_frontend.py:290: anchor = (r'<a target="_blank" ' remove the quotes https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... appengine/isolate/handlers_frontend.py:291: r'href="/browse?namespace=%s&digest=%s&as=%s">' remove the quotes, and use urllib.quote(save_as) so you don't need escaping; save_as could itself contain a double-quote. https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... appengine/isolate/handlers_frontend.py:297: # The string representation from json.dumps force-escapes all quotes, This is not needed anymore, as there's no quotes with the above requested changes.
https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... File appengine/isolate/handlers_frontend.py (right): https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... appengine/isolate/handlers_frontend.py:199: u'as': unicode(save_as), On 2017/02/14 21:01:16, M-A Ruel wrote: > keep keys sorted Done. https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... appengine/isolate/handlers_frontend.py:279: content = cgi.escape(content) On 2017/02/14 21:01:16, M-A Ruel wrote: > $ python -c 'import cgi,json; print > json.loads(cgi.escape(u"\"\\u003cgotcha\\u003e\""))' > <gotcha> > > you may want to double check your assumptions. I recommend to escape every > single string found in the parsed data, just before doing the html injection > below but after checking that it's actually worth processing. Sounds good. Now doing the checks in a static method, then cgi.escape, then html inject. https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... appengine/isolate/handlers_frontend.py:285: if 'files' in data: On 2017/02/14 21:01:16, M-A Ruel wrote: > Let's add some more checks. > Ref: > https://github.com/luci/luci-py/blob/master/appengine/isolate/doc/Design.md#i... > > if not isinstance(data, dict): > return content > actual = set(data) > if not actual.issubset(('algo', 'command', 'files', 'includes', 'read_only', > 'relative_cwd', 'version')): > return content > if 'files' not in data: > return content Done, although it seems superfluous to check actual.issubset() as well as 'files'. We're validating that 'files' is in data, and just having only ('files',) is also a valid subset, so the subset check provides no extra value as far as I can tell. https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... appengine/isolate/handlers_frontend.py:290: anchor = (r'<a target="_blank" ' On 2017/02/14 21:01:16, M-A Ruel wrote: > remove the quotes Done. https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... appengine/isolate/handlers_frontend.py:291: r'href="/browse?namespace=%s&digest=%s&as=%s">' On 2017/02/14 21:01:16, M-A Ruel wrote: > remove the quotes, and use urllib.quote(save_as) so you don't need escaping; > save_as could itself contain a double-quote. Done. https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... appengine/isolate/handlers_frontend.py:297: # The string representation from json.dumps force-escapes all quotes, On 2017/02/14 21:01:16, M-A Ruel wrote: > This is not needed anymore, as there's no quotes with the above requested > changes. Done.
you forgot to run "git cl upload"
https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... File appengine/isolate/handlers_frontend.py (right): https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... appengine/isolate/handlers_frontend.py:279: content = cgi.escape(content) On 2017/02/14 23:01:18, jonesmi wrote: > On 2017/02/14 21:01:16, M-A Ruel wrote: > > $ python -c 'import cgi,json; print > > json.loads(cgi.escape(u"\"\\u003cgotcha\\u003e\""))' > > <gotcha> > > > > you may want to double check your assumptions. I recommend to escape every > > single string found in the parsed data, just before doing the html injection > > below but after checking that it's actually worth processing. > > Sounds good. Now doing the checks in a static method, then cgi.escape, then html > inject. There is still no guarantee that someone uploaded a file with a file name "\"\u003cgotcha\u003e\"" https://codereview.chromium.org/2693953006/diff/20001/appengine/isolate/handl... File appengine/isolate/handlers_frontend.py (right): https://codereview.chromium.org/2693953006/diff/20001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:70: 'version' include trailing comma, so when a new line is added, it doesn't change the previous line. https://codereview.chromium.org/2693953006/diff/20001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:292: if actual.issubset(_ISOLATED_ROOT_MEMBERS) and 'files' in actual: return actual.issubset(_ISOLATED_ROOT_MEMBERS) and 'files' in actual https://codereview.chromium.org/2693953006/diff/20001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:311: if 'h' in metadata: # Only linkify files that have a digest property if metadata.get('h'): is safer. no need for the comment.
tansell@chromium.org changed reviewers: + tansell@chromium.org
Generally LGTM. One comment, I think maybe using a template to render the .json file might be a potential future solution. https://codereview.chromium.org/2693953006/diff/20001/appengine/isolate/handl... File appengine/isolate/handlers_frontend.py (right): https://codereview.chromium.org/2693953006/diff/20001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:297: def _format_isolated_content(content, namespace): I feel like just giving the isolate json to a template file might be a better solution here?
https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... File appengine/isolate/handlers_frontend.py (right): https://codereview.chromium.org/2693953006/diff/1/appengine/isolate/handlers_... appengine/isolate/handlers_frontend.py:279: content = cgi.escape(content) On 2017/02/14 23:46:14, M-A Ruel wrote: > On 2017/02/14 23:01:18, jonesmi wrote: > > On 2017/02/14 21:01:16, M-A Ruel wrote: > > > $ python -c 'import cgi,json; print > > > json.loads(cgi.escape(u"\"\\u003cgotcha\\u003e\""))' > > > <gotcha> > > > > > > you may want to double check your assumptions. I recommend to escape every > > > single string found in the parsed data, just before doing the html injection > > > below but after checking that it's actually worth processing. > > > > Sounds good. Now doing the checks in a static method, then cgi.escape, then > html > > inject. > > There is still no guarantee that someone uploaded a file with a file name > "\"\u003cgotcha\u003e\"" Ahh I see. Nice catch, thanks! We have to do cgi.escape() *after* loads() in order to protect against gotcha, which means we need to recursively iterate through every string in the nested dicts. https://codereview.chromium.org/2693953006/diff/20001/appengine/isolate/handl... File appengine/isolate/handlers_frontend.py (right): https://codereview.chromium.org/2693953006/diff/20001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:70: 'version' On 2017/02/14 23:46:14, M-A Ruel wrote: > include trailing comma, so when a new line is added, it doesn't change the > previous line. Done. https://codereview.chromium.org/2693953006/diff/20001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:292: if actual.issubset(_ISOLATED_ROOT_MEMBERS) and 'files' in actual: On 2017/02/14 23:46:14, M-A Ruel wrote: > return actual.issubset(_ISOLATED_ROOT_MEMBERS) and 'files' in actual Done. https://codereview.chromium.org/2693953006/diff/20001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:311: if 'h' in metadata: # Only linkify files that have a digest property On 2017/02/14 23:46:14, M-A Ruel wrote: > if metadata.get('h'): > is safer. no need for the comment. Done
On 2017/02/16 00:50:52, mithro wrote: > Generally LGTM. > > One comment, I think maybe using a template to render the .json file might be a > potential future solution. > > https://codereview.chromium.org/2693953006/diff/20001/appengine/isolate/handl... > File appengine/isolate/handlers_frontend.py (right): > > https://codereview.chromium.org/2693953006/diff/20001/appengine/isolate/handl... > appengine/isolate/handlers_frontend.py:297: def > _format_isolated_content(content, namespace): > I feel like just giving the isolate json to a template file might be a better > solution here? It's an interesting thought, as it does sound cleaner in general to use templates for generating our pages. In this particular case however, it seems there's a precedent of displaying the json content as-is with the exception of the friendly hyperlinks for each filename and added whitespace. Coming up with a template that recursively populates the json dict tree-like structure is do-able, but if we're going to go down this path I wonder if we'd want to come up with a custom view instead of simply outputting the stringified json format. For example, we could output the files as rows in a pretty table. WDYT?
On 2017/02/16 19:42:46, jonesmi wrote: > On 2017/02/16 00:50:52, mithro wrote: > > Generally LGTM. > > > > One comment, I think maybe using a template to render the .json file might be > a > > potential future solution. > > > > > https://codereview.chromium.org/2693953006/diff/20001/appengine/isolate/handl... > > File appengine/isolate/handlers_frontend.py (right): > > > > > https://codereview.chromium.org/2693953006/diff/20001/appengine/isolate/handl... > > appengine/isolate/handlers_frontend.py:297: def > > _format_isolated_content(content, namespace): > > I feel like just giving the isolate json to a template file might be a better > > solution here? > > It's an interesting thought, as it does sound cleaner in general to use > templates for generating our pages. In this particular case however, it seems > there's a precedent of displaying the json content as-is with the exception of > the friendly hyperlinks for each filename and added whitespace. Coming up with a > template that recursively populates the json dict tree-like structure is > do-able, but if we're going to go down this path I wonder if we'd want to come > up with a custom view instead of simply outputting the stringified json format. > For example, we could output the files as rows in a pretty table. WDYT? Here's a sample to give an idea: http://jsbin.com/takeqekenu
On 2017/02/16 19:42:46, jonesmi wrote: > On 2017/02/16 00:50:52, mithro wrote: > https://codereview.chromium.org/2693953006/diff/20001/appengine/isolate/handl... > > appengine/isolate/handlers_frontend.py:297: def > > _format_isolated_content(content, namespace): > > I feel like just giving the isolate json to a template file might be a better > > solution here? > > It's an interesting thought, as it does sound cleaner in general to use > templates for generating our pages. In this particular case however, it seems > there's a precedent of displaying the json content as-is with the exception of > the friendly hyperlinks for each filename and added whitespace. Coming up with a > template that recursively populates the json dict tree-like structure is > do-able, but if we're going to go down this path I wonder if we'd want to come > up with a custom view instead of simply outputting the stringified json format. > For example, we could output the files as rows in a pretty table. WDYT? I'm fine too but I don't want to spend too much time on this, whatever that works; in the end I'll want to replace all of this to a polymer frontend, so this parsing will be done in javascript in the end, hence my "not too much time on this." So a few fixes then it should be fine to commit. https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... File appengine/isolate/handlers_frontend.py (right): https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:290: if isinstance(json_data, dict): for "checker" functions, use early return instead; if not isinstance(json_data, dict): return False ... https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:295: @staticmethod @classmethod def _escape_all_dict_strings(cls, d): https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:299: if isinstance(v, str): basestring https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:301: elif isinstance(v, dict): you need to iterate in lists too; so the function should look like this instead: def _escapce_all_strings(cls, d): if isinstance(d, list): return [cls._escapce_all_strings(i) for i in d] if isinstance(d, dict): return {cls._escapce_all_strings(k): cls._escapce_all_strings(v) for k, v in d.iteritems()} if isinstance(d, basestring): return cgi.escape(d) return d this changes the condition at the call site, instead of inside the loop, which is easier to manage conceptually as a state machine. https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:302: d[k] = ContentHandler._escape_all_dict_strings(v) d[k] = cls._escape_all_dict_strings(v) https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:305: @staticmethod classmethod https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:313: if 'files' in json_data: check is not needed anymore.
https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... File appengine/isolate/handlers_frontend.py (right): https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:295: @staticmethod On 2017/02/16 21:29:12, M-A Ruel wrote: > @classmethod > def _escape_all_dict_strings(cls, d): Guide from security - DON'T TRY AND ESCAPE STUFF YOURSELF! https://g3doc.corp.google.com/security/g3doc/ise/xss/prevent.md?cl=head#use-a... https://wiki.corp.google.com/twiki/bin/view/Main/ISETeamSanitizers
Based on mithro's comment, looks like we should stop the existing precedent of using cgi.escape, and instead rely on jinja2 templating and its autoescape feature for the html response. Working on that now, unless any objections. https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... File appengine/isolate/handlers_frontend.py (right): https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:290: if isinstance(json_data, dict): On 2017/02/16 21:29:12, M-A Ruel wrote: > for "checker" functions, use early return instead; > > if not isinstance(json_data, dict): > return False > ... Done. https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:295: @staticmethod On 2017/02/16 23:21:35, mithro wrote: > On 2017/02/16 21:29:12, M-A Ruel wrote: > > @classmethod > > def _escape_all_dict_strings(cls, d): > > Guide from security - DON'T TRY AND ESCAPE STUFF YOURSELF! > > https://g3doc.corp.google.com/security/g3doc/ise/xss/prevent.md?cl=head#use-a... > > https://wiki.corp.google.com/twiki/bin/view/Main/ISETeamSanitizers > > > > Looks like we've already configured our jinja2 to use its autoescape functionality. Looking at refactoring this to use jinja2 template to take the security team's advise of using a template system for generating the response, and autoescaping. https://github.com/luci/luci-py/blob/master/appengine/components/components/t... https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:299: if isinstance(v, str): On 2017/02/16 21:29:12, M-A Ruel wrote: > basestring Ack, however getting rid of this method in favor of using jinja2's autoescaper https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:301: elif isinstance(v, dict): On 2017/02/16 21:29:12, M-A Ruel wrote: > you need to iterate in lists too; so the function should look like this instead: > > > def _escapce_all_strings(cls, d): > if isinstance(d, list): > return [cls._escapce_all_strings(i) for i in d] > if isinstance(d, dict): > return {cls._escapce_all_strings(k): cls._escapce_all_strings(v) for k, v in > d.iteritems()} > if isinstance(d, basestring): > return cgi.escape(d) > return d > > this changes the condition at the call site, instead of inside the loop, which > is easier to manage conceptually as a state machine. Ack, however getting rid of this method in favor of using jinja2's autoescaper https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:302: d[k] = ContentHandler._escape_all_dict_strings(v) On 2017/02/16 21:29:12, M-A Ruel wrote: > d[k] = cls._escape_all_dict_strings(v) Ack, however getting rid of this method in favor of using jinja2's autoescaper https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:305: @staticmethod On 2017/02/16 21:29:12, M-A Ruel wrote: > classmethod Done. https://codereview.chromium.org/2693953006/diff/40001/appengine/isolate/handl... appengine/isolate/handlers_frontend.py:313: if 'files' in json_data: On 2017/02/16 21:29:12, M-A Ruel wrote: > check is not needed anymore. Done.
On 2017/02/22 21:43:18, jonesmi wrote: > Based on mithro's comment, looks like we should stop the existing precedent of > using cgi.escape, and instead rely on jinja2 templating and its autoescape > feature for the html response. Working on that now, unless any objections. > Done - refactored to use jinja2 template, which follows security guidelines of having the template system take care of the autoescape of strings rather than doing it ourselves. Here's what it currently looks like with a simple isolated archive (it had no 'command', but you can see that the hyperlink works for downloading files as basename): https://2632-634db8d-tainted-jonesmi-dot-isolateserver-dev.appspot.com/browse... And here's what it currently looks like with a large isolated archive with many files (note: only the isolated file is archived on this -dev server, the hyperlinks won't work because the linked files aren't actually on the -dev server): https://2632-634db8d-tainted-jonesmi-dot-isolateserver-dev.appspot.com/browse...
Had a quick look at the output and I think it needs some CSS tweaks to make the iframe 100% height + 100% width but otherwise looks pretty good. Can we sort the table via the filename? Will review the code a bit later....
Yeah I noticed that as well, that the iframe height and width was changed from a recent CL, not sure where. I too would be happier to have it stretch larger like it used to be. Yes the table sorting can be done by converting the JSON dict to a collections.OrderedDict before passing it to jinja. On Sat, Feb 25, 2017, 11:51 PM <tansell@chromium.org> wrote: > Had a quick look at the output and I think it needs some CSS tweaks to > make the > iframe 100% height + 100% width but otherwise looks pretty good. > > Can we sort the table via the filename? > > Will review the code a bit later.... > > > https://codereview.chromium.org/2693953006/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2017/02/26 04:51:27, mithro wrote: > Had a quick look at the output and I think it needs some CSS tweaks to make the > iframe 100% height + 100% width but otherwise looks pretty good. > > Can we sort the table via the filename? > > Will review the code a bit later.... Done - now sorting by filename. Furthermore, added hyperlinking to the digests within 'includes'. Example of small isolate: https://2708-e2ffa7a-tainted-jonesmi-dot-isolateserver-dev.appspot.com/browse... https://screenshot.googleplex.com/g9GPVcjjxq9.png Example of large isolate: https://2708-e2ffa7a-tainted-jonesmi-dot-isolateserver-dev.appspot.com/browse... https://screenshot.googleplex.com/mnDrNPDcSZf.png PTAL
Awesome, very small things. Kevin, any idea why https://2708-e2ffa7a-tainted-jonesmi-dot-isolateserver-dev.appspot.com/browse... is not full length on a 1200x1920 monitor? https://codereview.chromium.org/2693953006/diff/100001/appengine/isolate/hand... File appengine/isolate/handlers_frontend.py (right): https://codereview.chromium.org/2693953006/diff/100001/appengine/isolate/hand... appengine/isolate/handlers_frontend.py:8: from collections import OrderedDict just "import collections" https://codereview.chromium.org/2693953006/diff/100001/appengine/isolate/hand... appengine/isolate/handlers_frontend.py:281: json_data['files'] = OrderedDict( collections.OrderedDict https://codereview.chromium.org/2693953006/diff/100001/appengine/isolate/temp... File appengine/isolate/templates/isolated.html (right): https://codereview.chromium.org/2693953006/diff/100001/appengine/isolate/temp... appengine/isolate/templates/isolated.html:26: <tr><th>file</th><th>digest</th><th>mode</th><th>size</th><th>symlink</th><th>filetype</th></tr> <thead> ?
https://codereview.chromium.org/2693953006/diff/100001/appengine/isolate/hand... File appengine/isolate/handlers_frontend.py (right): https://codereview.chromium.org/2693953006/diff/100001/appengine/isolate/hand... appengine/isolate/handlers_frontend.py:8: from collections import OrderedDict On 2017/03/23 21:23:42, M-A Ruel wrote: > just "import collections" Done. https://codereview.chromium.org/2693953006/diff/100001/appengine/isolate/hand... appengine/isolate/handlers_frontend.py:281: json_data['files'] = OrderedDict( On 2017/03/23 21:23:42, M-A Ruel wrote: > collections.OrderedDict Done. https://codereview.chromium.org/2693953006/diff/100001/appengine/isolate/temp... File appengine/isolate/templates/isolated.html (right): https://codereview.chromium.org/2693953006/diff/100001/appengine/isolate/temp... appengine/isolate/templates/isolated.html:26: <tr><th>file</th><th>digest</th><th>mode</th><th>size</th><th>symlink</th><th>filetype</th></tr> On 2017/03/23 21:23:42, M-A Ruel wrote: > <thead> ? Done.
The CQ bit was checked by maruel@chromium.org
lgtm Thanks!
The patchset sent to the CQ was uploaded after l-g-t-m from tansell@chromium.org Link to the patchset: https://codereview.chromium.org/2693953006/#ps120001 (title: "Isolate: Download files as their filename instead of hash")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 120001, "attempt_start_ts": 1490309170880370,
"parent_rev": "67ec52b0fc09d8ba654f4ea12eab9d339a5bc35f", "commit_rev":
"07e9f35001395b3ce620471b4896c6fb29020330"}
Message was sent while issue was closed.
Description was changed from ========== Isolate: Download files as their filename instead of hash BUG=689728 R=maruel@chromium.org ========== to ========== Isolate: Download files as their filename instead of hash BUG=689728 R=maruel@chromium.org Review-Url: https://codereview.chromium.org/2693953006 Committed: https://github.com/luci/luci-py/commit/07e9f35001395b3ce620471b4896c6fb29020330 ==========
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as https://github.com/luci/luci-py/commit/07e9f35001395b3ce620471b4896c6fb29020330
Message was sent while issue was closed.
On 2017/03/23 22:49:18, commit-bot: I haz the power wrote: > Committed patchset #7 (id:120001) as > https://github.com/luci/luci-py/commit/07e9f35001395b3ce620471b4896c6fb29020330 This is awesome! Thanks for doing this work jonesmi! Any idea when this will be deployed? Tim 'mithro' Ansell
Message was sent while issue was closed.
tomorrow morning unless someone beats me to it. Le 23 mars 2017 8:20 PM, <tansell@chromium.org> a écrit : > On 2017/03/23 22:49:18, commit-bot: I haz the power wrote: > > Committed patchset #7 (id:120001) as > > > https://github.com/luci/luci-py/commit/07e9f35001395b3ce620471b4896c6 > fb29020330 > > This is awesome! Thanks for doing this work jonesmi! > > Any idea when this will be deployed? > > Tim 'mithro' Ansell > > https://codereview.chromium.org/2693953006/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Message was sent while issue was closed.
It's live. 2017-03-23 20:26 GMT-04:00 Marc-Antoine Ruel <maruel@chromium.org>: > tomorrow morning unless someone beats me to it. > > Le 23 mars 2017 8:20 PM, <tansell@chromium.org> a écrit : > >> On 2017/03/23 22:49:18, commit-bot: I haz the power wrote: >> > Committed patchset #7 (id:120001) as >> > >> https://github.com/luci/luci-py/commit/07e9f35001395b3ce6204 >> 71b4896c6fb29020330 >> >> This is awesome! Thanks for doing this work jonesmi! >> >> Any idea when this will be deployed? >> >> Tim 'mithro' Ansell >> >> https://codereview.chromium.org/2693953006/ >> > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Message was sent while issue was closed.
> Kevin, any idea why https://2708-e2ffa7a-tainted-jonesmi-dot-isolateserver-dev.appspot.com/browse... is not full length on a 1200x1920 monitor? It might have something to do with iframe permissions - on pages that don't go full height, I see Blocked script execution in 'https://2708-e2ffa7a-tainted-jonesmi-dot-isolateserver-dev.appspot.com/brow…e=default-gzip&digest=0758550e3f84d71fb07c7da64340ae7f6690fb06&as=faq.html' because the document's frame is sandboxed and the 'allow-scripts' permission is not set. I can look into this. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
