Merge branch 'stable-3.4'
* stable-3.4:
Submit: Remove unused changeDataFactory attribute
Evaluate SubmitRules at most once when formatting ChangeInfo
Add test to show how often SubmitRules are run
Adapt SendMessage of the NoShell command to AsyncCommand type
gr-registration-dialog: Hide username field if it is not editable
Update jgit to 73f8acdc5c97e068143c86765995c4fb6923ee91
Clarify the CI validation process for security fixes
Add a section on reducing the impacts of Git gc
Fix registration redirect on OpenID
Fix gr-registration-dialog
FilePathAdapter: Add license header
Add a section about the impacts of Git gc
Add a repository-maintenance doc
Documentation/user-review-ui: drop change owner description
polygerrit-ui: add a tooltip for 'Updated' metadata field
Documentation/user-review: new screenshot for file level comments
Documentation/user-review: trim outdated info from diff prefs
Documentation/user-review: remove syntax coloring screenshot
Documentation/user-review: update diff preferences section
Fix the context control button rendering in Safari
Update jgit to f2e5bace4841758927d47db7d20e4a6f7353ce57
Update jgit to 00386272264f65c41e36406f7c2e9ea6e901276e
Documentation/user-review: remove VIM search reference
Documentation/user-review: remove scrollbar screenshot
Documentation/user-review: update toggle review screenshot
Documentation/user-review: remove mention of red bar
Documentation/user-review: update quick approve screenshot
Documentation/user-review: remove screenshot for last update
Documentation/user-review-ui: remove mention of VIM navigation
Log when a new SSH connection is rejected due to exceeded limit
Change-Id: Ic9aaa6f140691afc50fafebce65cbcc46ee9873d
diff --git a/Documentation/dev-processes.txt b/Documentation/dev-processes.txt
index 68e56ba..e43e021 100644
--- a/Documentation/dev-processes.txt
+++ b/Documentation/dev-processes.txt
@@ -296,14 +296,32 @@
test that verifies that the security vulnerability is no longer present.
+
Review and approval of the security fixes must be done by the Gerrit
-maintainers. Verifications must be done manually since the Gerrit CI doesn't
-build and test changes of the `gerrit-security-fixes` repository (and it
-shouldn't because everything on the CI server is public which would break
-the embargo).
+maintainers.
+
Once a security fix is ready and submitted, it should be cherry-picked to all
branches that should be fixed.
+. CI validation of the security fix:
++
+The validation of the security fixes does not happen on the regular Gerrit CI,
+because it would compromise the confidentiality of the fix and therefore break
+the embargo.
++
+The release manager maintains a private branch on the
+link:https://gerrit-review.googlesource.com/admin/repos/gerrit-ci-scripts[gerrit-ci-scripts,role=external,window=_blank] repository
+which contains a special build pipeline with special visibility restrictions.
++
+The validation process provides feedback, in terms of Code-Style, Verification
+and Checks, to the incoming security changes. The links associated
+with the build logs are exposed over the Internet but their access limited
+to only those who are actively participating in the development and review of
+the security fix.
++
+The maintainers that are willing to access the links to the CI logs need
+to request a time-limited (maximum 30 days) nominal X.509 certificate from a
+CI maintainer, which allows to access the build logs and analyze failures.
+The release manager may help obtaining that certificate from CI maintainers.
+
. Creation of fixed releases and announcement of the security vulnerability:
+
A release manager should create new bug fix releases for all fixed branches.
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-change-info-last-update.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info-last-update.png
deleted file mode 100644
index 93c296a..0000000
--- a/Documentation/images/gwt-user-review-ui-change-screen-change-info-last-update.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-change-info-owner.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info-owner.png
deleted file mode 100644
index 3d73ef7..0000000
--- a/Documentation/images/gwt-user-review-ui-change-screen-change-info-owner.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-quick-approve.png b/Documentation/images/gwt-user-review-ui-change-screen-quick-approve.png
deleted file mode 100644
index 638fc2f..0000000
--- a/Documentation/images/gwt-user-review-ui-change-screen-quick-approve.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-column.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-column.png
deleted file mode 100644
index b599f6d..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-column.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-dark-theme.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-dark-theme.png
deleted file mode 100644
index c041311..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-dark-theme.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png
deleted file mode 100644
index ea14a21..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png
deleted file mode 100644
index 8406ce8..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-commented.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-commented.png
deleted file mode 100644
index 1fd2033..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-commented.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences-popup.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences-popup.png
deleted file mode 100644
index 043c1ff..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences-popup.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png
deleted file mode 100644
index 7373b2f..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-red-bar.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-red-bar.png
deleted file mode 100644
index f817d66..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-red-bar.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png
deleted file mode 100644
index c767452..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png
deleted file mode 100644
index cbadd26..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-search.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-search.png
deleted file mode 100644
index e69bb0d..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-search.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-syntax-coloring.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-syntax-coloring.png
deleted file mode 100644
index a4b019a..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-syntax-coloring.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-quick-approve.png b/Documentation/images/user-review-ui-change-screen-quick-approve.png
new file mode 100644
index 0000000..f692d07
--- /dev/null
+++ b/Documentation/images/user-review-ui-change-screen-quick-approve.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png b/Documentation/images/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png
new file mode 100644
index 0000000..1eb7665
--- /dev/null
+++ b/Documentation/images/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-file-level-comment.png b/Documentation/images/user-review-ui-side-by-side-diff-screen-file-level-comment.png
new file mode 100644
index 0000000..66c46b7
--- /dev/null
+++ b/Documentation/images/user-review-ui-side-by-side-diff-screen-file-level-comment.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-preferences.png b/Documentation/images/user-review-ui-side-by-side-diff-screen-preferences.png
new file mode 100644
index 0000000..e008f2b
--- /dev/null
+++ b/Documentation/images/user-review-ui-side-by-side-diff-screen-preferences.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-reviewed.png b/Documentation/images/user-review-ui-side-by-side-diff-screen-reviewed.png
new file mode 100644
index 0000000..e2a7957
--- /dev/null
+++ b/Documentation/images/user-review-ui-side-by-side-diff-screen-reviewed.png
Binary files differ
diff --git a/Documentation/index.txt b/Documentation/index.txt
index e56e7ca..dc94b14 100644
--- a/Documentation/index.txt
+++ b/Documentation/index.txt
@@ -75,6 +75,7 @@
. link:config-reverseproxy.html[Reverse Proxy]
. link:config-auto-site-initialization.html[Automatic Site Initialization on Startup]
. link:pgm-index.html[Server Side Administrative Tools]
+. link:repository-maintenance.html[Repository Maintenance]
. link:user-request-tracing.html[Request Tracing]
. link:note-db.html[NoteDb]
. link:config-accounts.html[Accounts on NoteDb]
diff --git a/Documentation/repository-maintenance.txt b/Documentation/repository-maintenance.txt
new file mode 100644
index 0000000..1672436
--- /dev/null
+++ b/Documentation/repository-maintenance.txt
@@ -0,0 +1,116 @@
+= Gerrit Code Review - Repository Maintenance
+
+== Description
+
+Each project in Gerrit is stored in a bare Git repository. Gerrit uses
+the JGit library to access (read and write to) these Git repositories.
+As modifications are made to a project, Git repository maintenance will
+be needed or performance will eventually suffer. When using the Git
+command line tool to operate on a Git repository, it will run `git gc`
+every now and then on the repository to ensure that Git garbage
+collection is performed. However regular maintenance does not happen as
+a result of normal Gerrit operations, so this is something that Gerrit
+administrators need to plan for.
+
+Gerrit has a built-in feature which allows it to run Git garbage
+collection on repositories. This can be
+link:config-gerrit.html#gc[configured] to run on a regular basis, and/or
+this can be run manually with the link:cmd-gc.html[gerrit gc] ssh
+command, or with the link:rest-api-projects.html#run-gc[run-gc] REST API.
+Some administrators will opt to run `git gc` or `jgit gc` outside of
+Gerrit instead. There are many reasons this might be done, the main one
+likely being that when it is run in Gerrit it can be very resource
+intensive and scheduling an external job to run Git garbage collection
+allows administrators to finely tune the approach and resource usage of
+this maintenance.
+
+== Git Garbage Collection Impacts
+
+Unlike a typical server database, access to Git repositories is not
+marshalled through a single process or a set of inter communicating
+processes. Unfortuntatlely the design of the on-disk layout of a Git
+repository does not allow for 100% race free operations when accessed by
+multiple actors concurrently. These design shortcomings are more likely
+to impact the operations of busy repositories since racy conditions are
+more likely to occur when there are more concurrent operations. Since
+most Gerrit servers are expected to run without interruptions, Git
+garbage collection likely needs to be run during normal operational hours.
+When it runs, it adds to the concurrency of the overall accesses. Given
+that many of the operations in garbage collection involve deleting files
+and directories, it has a higher chance of impacting other ongoing
+operations than most other operations.
+
+=== Interrupted Operations
+
+When Git garbage collection deletes a file or directory that is
+currently in use by an ongoing operation, it can cause that operation to
+fail. These sorts of failures are often single shot failures, i.e. the
+operation will succeed if tried again. An example of such a failure is
+when a pack file is deleted while Gerrit is sending an object in the
+file over the network to a user performing a clone or fetch. Usually
+pack files are only deleted when the referenced objects in them have
+been repacked and thus copied to a new pack file. So performing the same
+operation again after the fetch will likely send the same object from
+the new pack instead of the deleted one, and the operation will succeed.
+
+=== Data Loss
+
+It is possible for data loss to occur when Git garbage collection runs.
+This is very rare, but it can happen. This can happen when an object is
+believed to be unreferenced when object repacking is running, and then
+garbage collection deletes it. This can happen because even though an
+object may indeed be unreferenced when object repacking begins and
+reachability of all objects is determined, it can become referenced by
+another concurrent operation after this unreferenced determination but
+before it gets deleted. When this happens, a new reference can be
+created which points to a now missing object, and this will result in a
+loss.
+
+== Reducing Git Garbage Collection Impacts
+
+JGit has a `preserved` directory feature which is intended to reduce
+some of the impacts of Git garbage collection, and Gerrit can take
+advantage of the feature too. The `preserved` directory is a
+subdirectory of a repository's `objects/pack` directory where JGit will
+move pack files that it would normally delete when `jgit gc` is invoked
+with the `--preserve-oldpacks` option. It will later delete these files
+the next time that `jgit gc` is run if it is invoked with the
+`--prune-preserved` option. Using these flags together on every `jgit gc`
+invocation means that packfiles will get an extended lifetime by one
+full garbage collection cycle. Since an atomic move is used to move these
+files, any open references to them will continue to work, even on NFS. On
+a busy repository, preserving pack files can make operations much more
+reliable, and interrupted operations should almost entirely disappear.
+
+Moving files to the `preserved` directory also has the ability to reduce
+data loss. If JGit cannot find an object it needs in its current object
+DB, it will look into the `preserved` directory as a last resort. If it
+finds the object in a pack file there, it will restore the
+slated-to-be-deleted pack file back to the original `objects/pack`
+directory effectively "undeleting" it and making all the objects in it
+available again. When this happens, data loss is prevented.
+
+One advantage of restoring preserved packfiles in this way when an
+object is referenced in them, is that it makes loosening unreferenced
+objects during Git garbage collection, which is a potentially expensive,
+wasteful, and performance impacting operation, no longer desirable. It
+is recommended that if you use Git for garbage collection, that you use
+the `-a` option to `git repack` instead of the `-A` option to no longer
+perform this loosening.
+
+When Git is used for garbage collection instead of JGit, it is fairly
+easy to wrap `git gc` or `git repack` with a small script which has a
+`--prune-preserved` option which behaves as mentioned above by deleting
+any pack files currently in the preserved directory, and also has a
+`--preserve-oldpacks` option which then hardlinks all the currently
+existing pack files from the `objects/pack` directory into the
+`preserved` directory right before calling the real Git command. This
+approach will then behave similarly to `jgit gc` with respect to
+preserving pack files.
+
+GERRIT
+------
+Part of link:index.html[Gerrit Code Review]
+
+SEARCHBOX
+---------
diff --git a/Documentation/user-review-ui.txt b/Documentation/user-review-ui.txt
index 98ec22d..1ea8bd8 100644
--- a/Documentation/user-review-ui.txt
+++ b/Documentation/user-review-ui.txt
@@ -93,13 +93,6 @@
image::images/gwt-user-review-ui-change-screen-change-info.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info.png"]
-- [[change-owner]]Change Owner:
-+
-The owner of the change is displayed as a link to a list of the owner's
-changes that have the same status as the currently viewed change.
-+
-image::images/gwt-user-review-ui-change-screen-change-info-owner.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info-owner.png"]
-
- [[reviewers]]Reviewers:
+
The reviewers of the change are displayed as chip tokens.
@@ -163,10 +156,6 @@
+
image::images/gwt-user-review-ui-change-screen-change-info-cannot-merge.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info-cannot-merge.png"]
-- [[update-time]]Time of Last Update:
-+
-image::images/gwt-user-review-ui-change-screen-change-info-last-update.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info-last-update.png"]
-
- [[actions]]Actions:
+
Depending on the change state and the permissions of the user, different
@@ -665,7 +654,7 @@
comments; a summary comment is only added if the reply popup panel is
open when the quick approve button is clicked.
-image::images/gwt-user-review-ui-change-screen-quick-approve.png[width=800, link="images/gwt-user-review-ui-change-screen-quick-approve.png"]
+image::images/user-review-ui-change-screen-quick-approve.png[width=800, link="images/gwt-user-review-ui-change-screen-quick-approve.png"]
[[history]]
=== History
@@ -746,28 +735,12 @@
image::images/gwt-user-review-ui-side-by-side-diff-screen-project-and-file.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-project-and-file.png"]
[[side-by-side-mark-reviewed]]
-The checkbox in front of the project name and the file name allows the
+The checkbox in front of the file name allows the
patch to be marked as reviewed. The link:#mark-reviewed[Mark Reviewed]
diff preference allows to control whether the files should be
automatically marked as reviewed when they are viewed.
-image::images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png"]
-
-[[scrollbar]]
-The scrollbar shows patch diffs and inline comments as annotations.
-This provides a good overview of the lines in the patch that are
-relevant for reviewing. By clicking on an annotation one can quickly
-navigate to the corresponding line in the patch.
-
-image::images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png"]
-
-[[gaps]]
-A gap between lines in the file content that is caused by aligning the
-left and right side or by displaying inline comments is shown as a
-vertical red bar in the line number column. This prevents a gap from
-being mistaken for blank lines in the file
-
-image::images/gwt-user-review-ui-side-by-side-diff-screen-red-bar.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-red-bar.png"]
+image::images/user-review-ui-side-by-side-diff-screen-reviewed.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png"]
[[patch-set-selection]]
In the header, on each side, the list of patch sets is shown. Clicking
@@ -926,52 +899,10 @@
[[file-level-comments]]
=== File Level Comments
-Comments that apply to a whole file can be added on file level.
+File level comments are added by clicking the 'File' header at the top
+of the file.
-File level comments are added by clicking on the comment icon in the
-header above the file.
-
-image::images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png"]
-
-Clicking on the comment icon opens a comment box for typing the file
-level comment.
-
-image::images/gwt-user-review-ui-side-by-side-diff-screen-file-level-commented.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-file-level-commented.png"]
-
-[[search]]
-=== Search
-
-For searching within a patch file, a Vim-like search is supported.
-Typing `/` opens the search box. Typing in the search box immediately
-highlights matches in the patch file with a yellow background. Using
-JavaScript regular expressions in the search term is supported. The
-search is case insensitive. After confirming the search by `ENTER` one
-can navigate between the matches by `n` / `N` to go to the next /
-previous match. Skipped lines are automatically expanded if they
-contain a match and one navigates to it.
-
-For additional possibilities to search please check the
-link:http://www.vim.org/docs.php[Vim documentation,role=external,window=_blank]. There are other
-useful ways to search, e.g. while the cursor is on a word, pressing `*`
-or `#` searches for the next or previous occurrence of the word.
-
-Searching by `Ctrl-F` finds matches only in the visible area of the
-screen unless the link:#render[Render] diff preference is set to `Slow`.
-
-image::images/gwt-user-review-ui-side-by-side-diff-screen-search.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-search.png"]
-
-[[key-navigation]]
-=== Key Navigation
-
-Vim-like commands can be used to navigate within a patch file:
-
-- `h` / `j` / `k` / `l` moves the cursor left / down / up / right
-- `0` / `$` moves the cursor to the start / end of the line
-- `gg` / `G` moves to cursor to the start / end of the file
-- `Ctrl-D` / `Ctrl-U` scrolls downwards / upwards
-
-Please check the link:http://www.vim.org/docs.php[Vim documentation,role=external,window=_blank]
-for further information.
+image::images/user-review-ui-side-by-side-diff-screen-file-level-comment.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png"]
[[diff-preferences]]
=== Diff Preferences
@@ -981,27 +912,10 @@
preferences. The diff preferences can be accessed by clicking on the
settings icon in the screen header.
-image::images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png"]
-
-The diff preferences popup allows to change the diff preferences.
-By clicking on the `Save` button changes to the diff preferences are
-saved permanently. Clicking on the `Apply` button applies the new
-diff preferences to the current screen, but they are discarded when the
-screen is refreshed. The `Save` button is only available if the user is
-signed in.
-
-image::images/gwt-user-review-ui-side-by-side-diff-screen-preferences-popup.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-preferences-popup.png"]
+image::images/user-review-ui-side-by-side-diff-screen-preferences.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png"]
The following diff preferences can be configured:
-- [[theme]]`Theme`:
-+
-Controls the theme that is used to render the file content.
-+
-E.g. users could choose to work with a dark theme.
-+
-image::images/gwt-user-review-ui-side-by-side-diff-screen-dark-theme.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-dark-theme.png"]
-
- [[ignore-whitespace]]`Ignore Whitespace`:
+
Controls whether differences in whitespace should be ignored or not.
@@ -1010,11 +924,11 @@
+
All differences in whitespace are highlighted.
+
-** `At Line End`:
+** `Trailing`:
+
Whitespace differences at the end of lines are ignored.
+
-** `Leading, At Line End`:
+** `Leading, Trailing`:
+
Whitespace differences at the beginning and end of lines are ignored.
+
@@ -1028,11 +942,7 @@
- [[columns]]`Columns`:
+
-Sets the preferred line length. At this position a vertical dashed line
-is displayed so that one can easily detect lines the exceed the
-preferred line length.
-+
-image::images/gwt-user-review-ui-side-by-side-diff-screen-column.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-column.png"]
+Sets the preferred line length. At this position, lines are wrapped.
- [[lines-of-context]]`Lines Of Context`:
+
@@ -1049,84 +959,28 @@
If many lines are skipped there are additional links to expand the
context by ten lines before and after the skipped block.
+
-image::images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png"]
+image::images/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png"]
- [[syntax-highlighting]]`Syntax Highlighting`:
+
Controls whether syntax highlighting should be enabled.
+
The language for the syntax highlighting is automatically detected from
-the file extension. The language can also be set manually by selecting
-it from the `Language` drop-down list.
-+
-image::images/gwt-user-review-ui-side-by-side-diff-screen-syntax-coloring.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-syntax-coloring.png"]
+the file extension.
-- [[whitespace-errors]]`Whitespace Errors`:
+- [[whitespace-errors]]`Show trailing whitespace`:
+
-Controls whether whitespace errors are highlighted.
+Controls whether trailing whitespace is highlighted.
- [[show-tabs]]`Show Tabs`:
+
Controls whether tabs are highlighted.
-- [[line-numbers]]`Line Numbers`:
-+
-Controls whether line numbers are shown.
-
-- [[empty-pane]]`Empty Pane`:
-+
-Controls whether empty panes are shown or not. The Left pane is empty when a
-file was added; the right pane is empty when a file was deleted.
-
-- [[left-side]]`Left Side`:
-+
-Controls whether the left side is shown. This preference is not
-persistent and is ignored by the `Save` button. Every time a
-patch diff is opened, this preference is reset to `Show`.
-
-- [[top-menu]]`Top Menu`:
-+
-Controls whether the top menu is shown.
-
-- [[auto-hide-diff-table-header]]`Auto Hide Diff Table Header`:
-+
-Controls whether the diff table header should be automatically hidden
-when scrolling down more than half of a page.
-
- [[mark-reviewed]]`Mark Reviewed`:
+
Controls whether the files of the patch set should be automatically
marked as reviewed when they are viewed.
-- [[expand-all-comments]]`Expand All Comments`:
-+
-Controls whether all comments should be automatically expanded.
-
-- [[render]]`Render`:
-+
-Controls how patch files that exceed the screen size are rendered.
-+
-If `Fast` is selected file contents which are outside of the visible
-area are not attached to the browser's DOM tree. This makes the
-rendering fast, but searching by `Ctrl+F` only finds content which is
-in the visible area.
-+
-If `Slow` is selected all file contents are attached to the browser's
-DOM tree, which makes the rendering slow for large files. The advantage
-of this setting is that `Ctrl+F` can be used to search in the complete
-file.
-+
-Large files that exceed 4000 lines will not be fully rendered.
-
-- [[line-wrapping]]`Line Wrapping`:
-+
-Controls whether to enable line wrapping or not.
-+
-If `false` is selected then line wrapping is disabled.
-This is the default option.
-+
-If `true` is selected then line wrapping is enabled.
-
[[keyboard-shortcuts]]
== Keyboard Shortcuts
diff --git a/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java b/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
index be975c5..b685011 100644
--- a/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
+++ b/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
@@ -477,8 +477,9 @@
final StringBuilder rdr = new StringBuilder();
rdr.append(urlProvider.get(req));
String nextToken = Url.decode(token);
- if (isNew && !token.startsWith(PageLinks.REGISTER + "/")) {
- rdr.append('#' + PageLinks.REGISTER);
+ String registerUri = PageLinks.REGISTER + "/";
+ if (isNew && !token.startsWith(registerUri)) {
+ rdr.append('#' + registerUri);
if (nextToken.startsWith("#")) {
// Need to strip the leading # off the token to fix registration page redirect
nextToken = nextToken.substring(1);
diff --git a/java/com/google/gerrit/server/patch/FilePathAdapter.java b/java/com/google/gerrit/server/patch/FilePathAdapter.java
index 7f34cf1..ccd1466 100644
--- a/java/com/google/gerrit/server/patch/FilePathAdapter.java
+++ b/java/com/google/gerrit/server/patch/FilePathAdapter.java
@@ -1,3 +1,17 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
package com.google.gerrit.server.patch;
import com.google.gerrit.entities.Patch.ChangeType;
diff --git a/java/com/google/gerrit/server/restapi/change/Submit.java b/java/com/google/gerrit/server/restapi/change/Submit.java
index 263d1e7..876c92c 100644
--- a/java/com/google/gerrit/server/restapi/change/Submit.java
+++ b/java/com/google/gerrit/server/restapi/change/Submit.java
@@ -104,7 +104,6 @@
private final GitRepositoryManager repoManager;
private final PermissionBackend permissionBackend;
- private final ChangeData.Factory changeDataFactory;
private final Provider<MergeOp> mergeOpProvider;
private final Provider<MergeSuperSet> mergeSuperSet;
private final AccountResolver accountResolver;
@@ -124,7 +123,6 @@
Submit(
GitRepositoryManager repoManager,
PermissionBackend permissionBackend,
- ChangeData.Factory changeDataFactory,
Provider<MergeOp> mergeOpProvider,
Provider<MergeSuperSet> mergeSuperSet,
AccountResolver accountResolver,
@@ -135,7 +133,6 @@
ChangeJson.Factory json) {
this.repoManager = repoManager;
this.permissionBackend = permissionBackend;
- this.changeDataFactory = changeDataFactory;
this.mergeOpProvider = mergeOpProvider;
this.mergeSuperSet = mergeSuperSet;
this.accountResolver = accountResolver;
diff --git a/java/com/google/gerrit/sshd/LogMaxConnectionsPerUserExceeded.java b/java/com/google/gerrit/sshd/LogMaxConnectionsPerUserExceeded.java
new file mode 100644
index 0000000..6f568b1
--- /dev/null
+++ b/java/com/google/gerrit/sshd/LogMaxConnectionsPerUserExceeded.java
@@ -0,0 +1,42 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.sshd;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import org.apache.sshd.common.Service;
+import org.apache.sshd.common.session.Session;
+import org.apache.sshd.common.session.SessionDisconnectHandler;
+
+@Singleton
+public class LogMaxConnectionsPerUserExceeded implements SessionDisconnectHandler {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ @Override
+ public boolean handleSessionsCountDisconnectReason(
+ Session session,
+ Service service,
+ String username,
+ int currentSessionCount,
+ int maxSessionCount)
+ throws IOException {
+ logger.atWarning().log(
+ "Max connection count for user %s exceeded, rejecting new connection."
+ + " currentSessionCount = %d, maxSessionCount = %d",
+ username, currentSessionCount, maxSessionCount);
+ return false;
+ }
+}
diff --git a/java/com/google/gerrit/sshd/NoShell.java b/java/com/google/gerrit/sshd/NoShell.java
index dd31e4c..e3f654b 100644
--- a/java/com/google/gerrit/sshd/NoShell.java
+++ b/java/com/google/gerrit/sshd/NoShell.java
@@ -27,10 +27,14 @@
import java.io.OutputStream;
import java.net.MalformedURLException;
import java.net.URL;
+import org.apache.sshd.common.io.IoInputStream;
+import org.apache.sshd.common.io.IoOutputStream;
+import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
import org.apache.sshd.server.Environment;
import org.apache.sshd.server.ExitCallback;
import org.apache.sshd.server.SessionAware;
import org.apache.sshd.server.channel.ChannelSession;
+import org.apache.sshd.server.command.AsyncCommand;
import org.apache.sshd.server.command.Command;
import org.apache.sshd.server.session.ServerSession;
import org.apache.sshd.server.shell.ShellFactory;
@@ -56,13 +60,19 @@
return shell.get();
}
- static class SendMessage implements Command, SessionAware {
+ /**
+ * When AsyncCommand is implemented by a command as below, the usual blocking streams aren't set.
+ *
+ * @see org.apache.sshd.server.command.AsyncCommand
+ */
+ static class SendMessage implements AsyncCommand, SessionAware {
private final Provider<MessageFactory> messageFactory;
private final SshScope sshScope;
- private InputStream in;
- private OutputStream out;
- private OutputStream err;
+ private IoInputStream in;
+ private IoOutputStream out;
+ private IoOutputStream err;
+
private ExitCallback exit;
private Context context;
@@ -73,21 +83,36 @@
}
@Override
- public void setInputStream(InputStream in) {
+ public void setIoInputStream(IoInputStream in) {
this.in = in;
}
@Override
- public void setOutputStream(OutputStream out) {
+ public void setIoOutputStream(IoOutputStream out) {
this.out = out;
}
@Override
- public void setErrorStream(OutputStream err) {
+ public void setIoErrorStream(IoOutputStream err) {
this.err = err;
}
@Override
+ public void setInputStream(InputStream in) {
+ // ignored
+ }
+
+ @Override
+ public void setOutputStream(OutputStream out) {
+ // ignore
+ }
+
+ @Override
+ public void setErrorStream(OutputStream err) {
+ // ignore
+ }
+
+ @Override
public void setExitCallback(ExitCallback callback) {
this.exit = callback;
}
@@ -107,8 +132,7 @@
} finally {
sshScope.set(old);
}
- err.write(Constants.encode(message));
- err.flush();
+ err.writeBuffer(new ByteArrayBuffer(Constants.encode(message)));
in.close();
out.close();
diff --git a/java/com/google/gerrit/sshd/SshDaemon.java b/java/com/google/gerrit/sshd/SshDaemon.java
index 9ae8660..fa20b9c 100644
--- a/java/com/google/gerrit/sshd/SshDaemon.java
+++ b/java/com/google/gerrit/sshd/SshDaemon.java
@@ -162,7 +162,8 @@
SshLog sshLog,
@SshListenAddresses List<SocketAddress> listen,
@SshAdvertisedAddresses List<String> advertised,
- MetricMaker metricMaker) {
+ MetricMaker metricMaker,
+ LogMaxConnectionsPerUserExceeded logMaxConnectionsPerUserExceeded) {
setPort(IANA_SSH_PORT /* never used */);
this.cfg = cfg;
@@ -235,6 +236,7 @@
setKeyPairProvider(hostKeyProvider);
setCommandFactory(commandFactory);
setShellFactory(noShell);
+ setSessionDisconnectHandler(logMaxConnectionsPerUserExceeded);
final AtomicInteger connected = new AtomicInteger();
metricMaker.newCallbackMetric(
diff --git a/javatests/com/google/gerrit/integration/ssh/BUILD b/javatests/com/google/gerrit/integration/ssh/BUILD
index dc8e68c..412aad8 100644
--- a/javatests/com/google/gerrit/integration/ssh/BUILD
+++ b/javatests/com/google/gerrit/integration/ssh/BUILD
@@ -5,3 +5,9 @@
group = "peer-keys-auth",
labels = ["ssh"],
)
+
+acceptance_tests(
+ srcs = ["NoShellIT.java"],
+ group = "no-shell",
+ labels = ["ssh"],
+)
diff --git a/javatests/com/google/gerrit/integration/ssh/NoShellIT.java b/javatests/com/google/gerrit/integration/ssh/NoShellIT.java
new file mode 100644
index 0000000..ccaf085
--- /dev/null
+++ b/javatests/com/google/gerrit/integration/ssh/NoShellIT.java
@@ -0,0 +1,96 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.integration.ssh;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.acceptance.GerritServer.TestSshServerAddress;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.StandaloneSiteTest;
+import com.google.gerrit.acceptance.UseSsh;
+import com.google.gerrit.extensions.api.GerritApi;
+import com.google.inject.Inject;
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import org.junit.Test;
+
+@NoHttpd
+@UseSsh
+public class NoShellIT extends StandaloneSiteTest {
+ private static final String[] SSH_KEYGEN_CMD =
+ new String[] {"ssh-keygen", "-t", "rsa", "-q", "-P", "", "-f"};
+
+ @Inject private GerritApi gApi;
+ @Inject private @TestSshServerAddress InetSocketAddress sshAddress;
+
+ private String identityPath;
+
+ @Test(timeout = 30000)
+ public void verifyCommandsIsClosed() throws Exception {
+ try (ServerContext ctx = startServer()) {
+ setUpTestHarness(ctx);
+
+ IOException thrown = assertThrows(IOException.class, () -> execute(cmd()));
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("Hi Administrator, you have successfully connected over SSH.");
+ }
+ }
+
+ private void setUpTestHarness(ServerContext ctx) throws Exception {
+ ctx.getInjector().injectMembers(this);
+ setUpAuthentication();
+ identityPath = sitePaths.data_dir.resolve(String.format("id_rsa_%s", "admin")).toString();
+ }
+
+ private void setUpAuthentication() throws Exception {
+ execute(
+ ImmutableList.<String>builder()
+ .add(SSH_KEYGEN_CMD)
+ .add(String.format("id_rsa_%s", "admin"))
+ .build());
+ gApi.accounts()
+ .id("admin")
+ .addSshKey(
+ new String(
+ java.nio.file.Files.readAllBytes(
+ sitePaths.data_dir.resolve(String.format("id_rsa_%s.pub", "admin"))),
+ UTF_8));
+ }
+
+ private ImmutableList<String> cmd() {
+ return ImmutableList.<String>builder()
+ .add("ssh")
+ .add("-tt")
+ .add("-o")
+ .add("StrictHostKeyChecking=no")
+ .add("-o")
+ .add("UserKnownHostsFile=/dev/null")
+ .add("-p")
+ .add(String.valueOf(sshAddress.getPort()))
+ .add("admin@" + sshAddress.getHostName())
+ .add("-i")
+ .add(identityPath)
+ .build();
+ }
+
+ private String execute(ImmutableList<String> cmd) throws Exception {
+ return execute(cmd, sitePaths.data_dir.toFile(), ImmutableMap.of());
+ }
+}
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
index 931579b..ca68926 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
@@ -153,7 +153,14 @@
<section
class$="[[_computeDisplayState(_showAllSections, change, _SECTION.UPDATED)]]"
>
- <span class="title">Updated</span>
+ <span class="title">
+ <gr-tooltip-content
+ has-tooltip=""
+ title="Last update of (meta)data for this change."
+ >
+ Updated
+ </gr-tooltip-content>
+ </span>
<span class="value">
<gr-date-formatter
has-tooltip=""
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts
index 738abc0..3763fbf 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts
@@ -72,7 +72,7 @@
_serverConfig?: ServerInfo;
@property({
- computed: '_computeUsernameMutable(_serverConfig, _account.username)',
+ computed: '_computeUsernameMutable(_account.username)',
type: Boolean,
})
_usernameMutable = false;
@@ -121,17 +121,14 @@
(this._account.username || '') !== (this._username || '');
}
- _computeUsernameMutable(config?: ServerInfo, username?: string) {
- // Polymer 2: check for undefined
- if (config === undefined) {
- return false;
- }
-
+ _computeUsernameMutable(username?: string) {
// Username may not be changed once it is set.
- return (
- config.auth.editable_account_fields.includes(
- EditableAccountField.USER_NAME
- ) && !username
+ return !username;
+ }
+
+ _computeUsernameEditable(config?: ServerInfo) {
+ return !!config?.auth.editable_account_fields.includes(
+ EditableAccountField.USER_NAME
);
}
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts
index 0e31bc9..4b86709 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts
@@ -84,20 +84,24 @@
</iron-input>
</span>
</section>
- <section>
- <span class="title">Username</span>
- <span hidden$="[[_usernameMutable]]" class="value">[[_username]]</span>
- <span hidden$="[[!_usernameMutable]]" class="value">
- <iron-input bind-value="{{_username}}">
- <input
- is="iron-input"
- id="username"
- bind-value="{{_username}}"
- disabled="[[_saving]]"
- />
- </iron-input>
- </span>
- </section>
+ <template is="dom-if" if="[[_computeUsernameEditable(_serverConfig)]]">
+ <section>
+ <span class="title">Username</span>
+ <span hidden$="[[_usernameMutable]]" class="value"
+ >[[_username]]</span
+ >
+ <span hidden$="[[!_usernameMutable]]" class="value">
+ <iron-input bind-value="{{_username}}">
+ <input
+ is="iron-input"
+ id="username"
+ bind-value="{{_username}}"
+ disabled="[[_saving]]"
+ />
+ </iron-input>
+ </span>
+ </section>
+ </template>
<hr />
<p>
More configuration options for Gerrit may be found in the
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts
index 22f21e1..6c21e58 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts
@@ -137,53 +137,28 @@
});
test('_computeUsernameMutable', () => {
+ assert.isTrue(element._computeUsernameMutable(undefined));
+ assert.isFalse(element._computeUsernameMutable('abc'));
+ });
+
+ test('_computeUsernameEditable', () => {
assert.isTrue(
- element._computeUsernameMutable(
- {
- ...createServerInfo(),
- auth: {
- auth_type: AuthType.HTTP,
- editable_account_fields: [EditableAccountField.USER_NAME],
- },
+ element._computeUsernameEditable({
+ ...createServerInfo(),
+ auth: {
+ auth_type: AuthType.HTTP,
+ editable_account_fields: [EditableAccountField.USER_NAME],
},
- undefined
- )
+ })
);
assert.isFalse(
- element._computeUsernameMutable(
- {
- ...createServerInfo(),
- auth: {
- auth_type: AuthType.HTTP,
- editable_account_fields: [EditableAccountField.USER_NAME],
- },
+ element._computeUsernameEditable({
+ ...createServerInfo(),
+ auth: {
+ auth_type: AuthType.HTTP,
+ editable_account_fields: [],
},
- 'abc'
- )
- );
- assert.isFalse(
- element._computeUsernameMutable(
- {
- ...createServerInfo(),
- auth: {
- auth_type: AuthType.HTTP,
- editable_account_fields: [],
- },
- },
- undefined
- )
- );
- assert.isFalse(
- element._computeUsernameMutable(
- {
- ...createServerInfo(),
- auth: {
- auth_type: AuthType.HTTP,
- editable_account_fields: [],
- },
- },
- 'abc'
- )
+ })
);
});
});
