11 Commits

Author SHA1 Message Date
Mike Samuel
e07c4b2ea1 Add polygerrit-ui/app/test/common-test-setup.html
This is a partial roll-forward of c/106190

This replaces all loads of iron-test-helpers with a load of a file
that wraps it, and adds that file to test files that do not currently
load iron-test-helpers.

A future CL will also install polymer-resin via common-test-helpers.html.

I tested by running

$ WCT_ARGS="-l chrome" ./polygerrit-ui/app/run_test.sh

Change-Id: Ifb3cd2c8db13d724f57e56e7e78045470d103a43
2017-06-05 22:10:12 +00:00
Wyatt Allen
c601abccc3 Revert "Polygerrit now loads polymer-resin"
This reverts commit 0895052c01ac5ac657a9763d2ad9967d9ae55c18.

Reason for revert: issue 6387

Change-Id: I14e00addeab53606952aa3ea2d45a74eac7a9d8a
2017-06-02 09:37:37 -07:00
Mike Samuel
0895052c01 Polygerrit now loads polymer-resin
polymer-resin intercepts polymer property assignments
before they reach XSS-vulnerable sinks like `href="..."`
and text nodes in `<script>` elements.

This follows the instructions in WORKSPACE for adding a new bower
dependency with kaspern's tweak to use the dependency in a rule so
that it's found.  //lib/js/bower_components.bzl has already been
rolled-back per those instructions.

The license is the polymer license as can be seen at
https://github.com/Polymer/polymer-resin/blob/master/LICENSE though
I'm not sure that //tools/js/bower2bazel.py recognizes it as such.

Docs for the added component are available at
https://github.com/Polymer/polymer-resin/blob/master/README.md
https://github.com/Polymer/polymer-resin/blob/master/getting-started.md

With this change, when I introduce an XSS vulnerability as below,
polymer-resin intercepts and stops it.

Patch that introduces a strawman vulnerability.

--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
@@ -55,6 +55,10 @@
         url: '/q/status:abandoned',
         name: 'Abandoned',
       },
+      {
+        url: location.hash.replace(/^#/, '') || 'http://example.com/#fragment_echoed_here',
+        name: 'XSS Me',
+      },
     ],
   }];

---

Address kaspern's and paladox's comments.

---

Undo version bumps for bower dependencies.

---

Change Soy index template to parallel app/index.html.

---

update polymer-resin to version 1.1.1-beta

----

Load polymer-resin into polygerrit-ui/**/*_test.html

After this, I ran the tests with
  -l chrome
  -l firefox

I ran a handful of tests with -p and observed that the
console shows "initResin" is called before test cases start
executing.

These changes were done programmaticly by running the script below
(approximately) thus:
```
gerrit/ $ cd polygerrit-ui/app
app/ $ find . -name \*test.html | xargs perl hack-tests.pl
```

```
use strict;

sub removeResin($) {
  my $s = $_[0];
  $s =~ s@<link rel="import" href="[^"]*/polymer-resin/[^"]*"[^>]*>\n?@@;
  $s =~ s@<script src="[^"]*/polymer-resin/[^"]*"></script>\n?@@;
  $s =~ s@<script>\s*security\.polymer_resin.*?</script>\n?@@s;
  return $s;
}

for my $f (@ARGV) {
  next if $f =~ m@/bower_components/|/node_modules/@;

  system('git', 'checkout', $f);
  print "$f\n";

  my @lines = ();
  open(IN, "<$f") or die "$f: $!";
  my $maxLineOfMatch = 0;
  while (<IN>) {
    push(@lines, $_);
    # Put a marker after core loading directives.
    $maxLineOfMatch = scalar(@lines)
      if m@/webcomponentsjs/|/polymer[.]html\b|/browser[.]js@;
  }
  close(IN) or die "$f: $!";

  die "$f missing loading directives" unless $maxLineOfMatch;

  # Given ./a/b/c/my_test.html, $pathToRoot is "../../.."
  # assuming no non-leading . or .. components in the path from find.
  my $pathToRoot = $f;
  $pathToRoot =~ s@^\.\/@@;
  $pathToRoot =~ s@^(.*?/)?app/@@;
  $pathToRoot =~ s@\/[^\/]*$@@;
  $pathToRoot =~ s@[^/]+@..@g;

  my $nLines = scalar(@lines);
  open(OUT, ">$f") or die "$f: $!";

  # Output the lines up to the last polymer-resin dependency
  # loaded explicitly by this test.
  my $before = join '', @lines[0..($maxLineOfMatch - 1)];
  $before = removeResin($before);
  print OUT "$before";

  # Dump out the lines that load polymer-resin and configure it for
  # polygerrit.
  if (1) {
      print OUT qq'<link rel="import" href="$pathToRoot/bower_components/polymer-resin/standalone/polymer-resin-debug.html"/>
<script>
security.polymer_resin.install({allowedIdentifierPrefixes: [\'\']});
</script>
    ';
  }

  # Emit any remaining lines.
  my $after = join '', @lines[$maxLineOfMatch..$#lines];
  $after = removeResin($after);
  $after =~ s/^\n*//;
  print OUT "$after";

  close(OUT) or die "$f: $!";
}
```

---

update polymer-resin to version 1.2.1-beta

---

update Soy index template to new style polymer-resin initialization

----

fix lint warnings

----

Load test/common-test-setup.html into *_test.html

Instead of inserting instructions to load and initialize polymer-resin into
every test file, add a common-test-setup.html that does that and also fold
iron-test-helpers loading into it.

----

imported files do not need to load webcomponentsjs

Change-Id: I71221c36ed8a0fe7f8720c1064a2fcc9555bb8df
2017-05-30 23:16:09 -04:00
Kasper Nilsson
160716846a ES6ify /gr-diff-comment-thread-group/*
Bug: Issue 6179
Change-Id: I2f25f8aeae703c3a3ee7b22103902773065d845c
2017-05-16 14:01:18 -07:00
Viktar Donich
29e1ce5e84 Collection of prospective test flake fixes
Potentially related:
https://github.com/Polymer/web-component-tester/issues/505

Bug: Issue 5792
Change-Id: I9ab6e8e40d9811dd52906335426764c052907609
2017-03-30 13:46:58 -07:00
Becky Siegel
a88f8c82d4 Fix corner case where storage gets duplicated for patchsets
Previously, there was an issue where if you create a draft comment in
side by side view and switch to unified view, the comment thinks it's in
the later patch set rather than the earlier one and a second copy gets
added to local storage with the later patchset as a component of the
key.

This was because the the thread group assumed all threads inside of it
had the same patch number. This change fixes that, so in the event that
a user switches from side by side to unified, the patch number will get
taken from the comment rather than the thread group.

Bug: Issue 5493
Change-Id: I7f00997bcb2e6f1001a5d58ac206acf5af3367d2
2017-02-10 14:52:57 -08:00
Becky Siegel
2c602323e1 Move reply buttons to comment thread
Move all buttons that generate a reply of some sort (done, ack, reply,
quote) to the comment thread instead of the comment [1].

When there is a draft for a particular comment thread, all reply buttons
are hidden [2].  For example, if you click reply, you cannot click done
on the same thread, unless you remove the draft.

Each thread can have up to 1 draft. It's also worth noting that if a
thread has a draft, and the user clicks on the line or 'c' at the same
range, the existing draft will switch to 'editing' form.

[1] With the exception of "please fix" for robot comments.
[2] In this case, The please fix button will be disabled when other
reply buttons are hidden.

Feature: Issue 5410
Change-Id: Id847ee0cba0d0ce4e5b6476f58141866d41ffdad
2017-02-09 16:07:32 -08:00
Wyatt Allen
4a9de6fa4d Merge "Don't merge threads on same line left/right" 2017-02-08 00:30:57 +00:00
Becky Siegel
41bdc04cb8 Don't merge threads on same line left/right
Goes along with c/95273/. Adds commentSide attribute to comments to see
which side of the diff view they belong on. This is also used as part
of the locationRange for the gr-diff-comment-thread-group, so that two
thread groups can be on the same line or range for the unified group (
one for the right, one for the left).

Note: there is already a 'side' attribute on the gr-diff-comment, which
is confusing. This side actually references 'PARENT' or 'REVISION', to
identify whether the comment belongs to the parent or any revision. On
diffs where two revisions are compared to each other, this cannot be
used to determine left/right. However, because 'side' is part of
the CommentInfo entity[1], it is difficult to change the name and make
more sense out of that.

[1] https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#comment-info

Bug: Issue 5114
Change-Id: I5cc4c17d4bb134e31e5cc07ff9b08ed349488c97
2017-02-07 21:00:52 +00:00
Becky Siegel
41e811942d Get tests passing in safari/firefox
Change-Id: I1d85a24ce12bcd2f556abc14e028d9926180b978
2017-02-06 18:15:14 -08:00
Becky Siegel
562a30493e Allow multiple threads per line, with different ranges
- Add concept of diff comment thread groups, which are all of the
  threads at a particular line number.

- The thread group is responsible for breaking up comments into threads
  based on the range of the comment.

- Thread groups are ordered by the updated time of the first comment in
  the group.

- Thread groups are given a key, based on comment range, which is used
  to determine what thread group a new comment should go in (or if it
  needs a new one).

Feature: Issue 5292
Change-Id: If544e8bb879262de3ce5397e86124837b66ada04
2017-02-01 16:10:08 -08:00