When Error Prone checks are enabled, the "ClassCanBeStatic" warning is
triggered:
Inner class is non-static but does not reference enclosing class
see https://errorprone.info/bugpattern/ClassCanBeStatic
Change-Id: I5a0e3bf0cf8c28176d9c98914c1c0dfab9c5736f
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
This change is needed to implement permission aware ref database in
Gerrit: [1], that is a pre-requisite to re-enable Git v2 protocol in
Gerrit: [2].
Background: Last year Git v2 protocol was enabled in Gerrit. The fact,
that JGit layer was not calling ref advertise filter for Git v2
protocol, introduced security vulnerability.
The lesson learned from this security incident: Gerrit should not rely
on ref advertise filter being called by JGit to implement crictical
security checks. Instead, the idea is to use the same approach as
currently used by Google's internal code on googlesource.com that
didn't suffer from this vulnerability: provide a custom repository to
JGit. The repository provides a RefDatabase that is permission-aware
and will only ever return refs that the user has access to.
However, due to hard coded instanceof operator usages in JGit code
base, some tests in Gerrit are failing with: [1] in place. This change
addresses this problem.
[1] https://gerrit-review.googlesource.com/c/gerrit/+/212874
[2] https://gerrit-review.googlesource.com/c/gerrit/+/226754
Change-Id: I67c0f53ca33b149442e7ee3e51910d19e3f348d5
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
If the attributes of FileSnapshot don't detect modification of a
packfile read the packfile's checksum and compare it against the
checksum cached in the loaded packfile.
Since reading the checksum needs less IO than reloading the complete
packfile this may help to reduce the overhead to detect modficiation
when a gc completes while ObjectDirectory scans for packfiles in another
thread.
Bug: 546891
Change-Id: I9811b497eb11b8a85ae689081dc5d949ca8c4be5
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
If
- pack.waitPreventRacyPack = true (default is false)
- packfile size > pack.minSizePreventRacyPack (default is 100 MB)
wait after a new packfile was written and before it is opened until it
cannot be racy anymore.
If a new packfile is accessed while it's still racy at least the pack's
index will be reread by ObjectDirectory.scanPacksImpl(). Hence it may
save resources to wait one tick of the file system timer to avoid this
reloading. On filesystems with a coarse timestamp resolution it may be
beneficial to skip this wait for small packfiles.
Bug: 546891
Change-Id: I0e8bf3d7677a025edd2e397dd2c9134ba59b1a18
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Deprecate the constant with the intention of making it private in
a future release.
All existing usage of the constant within JGit code has already been
replaced with the recommended alternatives in preceding commits.
Change-Id: I10eb95f3f92cb74f93a26bf1a6edd24615b75c6f
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Allows callers to read all lines in the input stream until the
END marker is reached, without having to explicitly check for
the END marker.
Replace all remaining usage of the END marker with the new method.
Change-Id: I51f419c7f569ab7ed01e1aaaf6b40ed8cdc2116b
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
PacketLineIn.END is only referenced in tests. Replace most of those
with a new package visible end() method.
Remaining usages of PacketLineIn.END are in the form:
while ((line = pckIn.readString()) != PacketLineIn.END) {
and are not trivial replacements, hence are not touched in this change.
Change-Id: Id77c5321ddcad127130b246bde8f08736e60e1ea
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Deprecate DELIM with the intention of making it private in a future
release.
Callers that want to test if a packet line string is the delimiter
should use the isDelimiter(String) method.
The only other references to DELIM in the JGit code are in tests. For
those, introduce a package visible delimiter() method.
Change-Id: I21e8bbac0ffb9ef710c9753e23435416b09a4891
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Replace reference comparisons of PacketLineIn's DELIM and END strings
with usage of the helper methods isDelimiter() and isEnd().
Change-Id: I52dcfc4ee9097f1bd6970601c716701847d9eebd
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Also now use JcaKeyBoxBuilder constructor in
BouncyCastleGpgKeyLocator.readKeyBoxFile(Path).
CQ: 19868
CQ: 19869
CQ: 19870
Change-Id: I45bd80e158aecd90448b0c7e59615db27aaef892
Signed-off-by: Brandon Weeks <bweeks@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Omit the default value of "false" for "useTags" like already done for
"longDesc".
Change-Id: I25aaacae028fc8cf27f4040ba45fe79609318aa1
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
These methods will allow clients to check for END and DELIM without
doing a reference comparison on the String objects, which raises
warnings from Error Prone.
Change-Id: I9e7e59843553ed4488ee8e864033198bbb60d67c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Initialize it using the repository's config already in the constructor.
Change-Id: I4ea620a7db72956e7109f739990f09644640206b
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This allows to verify the expected behavior in
FileSnapshotTest#testSimulatePackfileReplacement and enables extending
FileSnapshot for packfiles to read the packfile's checksum as another
criterion to detect modifications without reading the full content.
Also add another field capturing the result of the last check if
lastModified was racily clean.
Remove unnecessary determination of raciness in the constructor. It was
determined twice in all relevant cases.
Change-Id: I100a2f49d7949693d7b72daa89437e166f1dc107
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This resolves a regression introduced in fef78212.
Change-Id: Ibb4521635a87012520566efc70870c59d11be874
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
Fill out the description of when IOException is thrown.
Also fix a typo in the description for IncorrectObjectTypeException.
Change-Id: I9fafd19d68ddc4fe4e95e8516c2c38484b941a3a
Signed-off-by: Ivan Frade <ifrade@google.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Offer a version of findBranchesReachableFrom method with progress
monitor callback. This is required to allow UI clients to cancel long
running operations and show progress.
Bug: 547642
Change-Id: I31d1de54dbaa6ffb11e03da4c447963e8defa1d0
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
The default implementation of openSystemConfig has special handling for
when the FS returns null from getGitSystemConfig: it should return a
"real" FileBasedConfig instance that never actually tries to load a
file. However, this codepath was not respecting the passed-in parent
config.
Change-Id: Id0bcdc93bb42f9ebe3f5ee4c6b4be8863e0133f9
Every caller would need to check if bitmaps are available in the repo to
instantiate a reachability checker.
Offer a method to create the reachability checker in the walk: the
caller has already a walk over the repo, and the walk has all the
information required.
This allows us to make the implementation classes package-private.
Change-Id: I355e47486fcd9d55baa7cb5700ec08dcc230eea5
Signed-off-by: Ivan Frade <ifrade@google.com>
The new ReachabilityChecker interface and its implementations are marked
as @since 5.5, but they will make it to the 5.4 release.
Change-Id: I88c31b3300ccf35d18c35faddb2517f0a57bdcfd
Signed-off-by: Ivan Frade <ifrade@google.com>
Due to finite filesystem timestamp resolution the last modified
timestamp of files cannot detect file changes which happened in the
immediate past (less than one filesystem timer tick ago).
Some filesystems expose unique file identifiers, e.g. inodes in Posix
filesystems which are named filekeys in Java's BasicFileAttributes. Use
them as another means to detect file modifications based on stat
information.
Running git gc on a repository yields a new packfile with the same id as
a packfile which existed before the gc if these packfiles contain the
same set of objects. The content of the old and the new packfile might
differ if a different PackConfig was used when writing the packfile.
Considering filekeys in FileSnapshot may help to detect such packfile
modifications.
Bug: 546891
Change-Id: I711a80328c55e1a31171d540880b8e80ec1fe095
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
FileSnapshot.notRacyClean() assumed a worst case filesystem timestamp
resolution of 2.5 sec (FAT has a resolution of 2 sec). Instead measure
timestamp resolution to avoid unnecessary IO caused by false positives
in detecting the racy git problem caused by finite filesystem timestamp
resolution [1].
Cache the measured resolution per FileStore since timestamp resolution
depends on the respective filesystem type. If timestamp resolution
cannot be measured or fails due to an exception fallback to the worst
case FAT timestamp resolution and avoid caching this value.
Add a 10% safety margin in FileSnapshot.notRacyClean(), though running
FsTest.testFsTimestampResolution() 1000 times which is not using a
safety margin didn't fail on Mac using APFS and Java 8, 11, 12.
Measured Java file timestamp resolution: [2]
[1] https://github.com/git/git/blob/master/Documentation/technical/racy-git.txt
[2] https://docs.google.com/spreadsheets/d/1imy0y6WmRqBf0kjCxzxj2X7M50eIVfa7oaUIzEOHmjo
Bug: 546891
Change-Id: I493f3b57b6b306285ffa7d392339d253e5966ab8
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* fix equals() and hashCode() methods to consider size
* fix toString() to show size
Change-Id: I5485db55eda5110121efd65d86c7166b3b2e93d0
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Check whether the value of the git config user.signingKey is a suffix
of the full fingerprint of the key. This was already used for finding
keys in secring.gpg, but not in pubring.kbx. This mechanism allows a
user to use any unique suffix to identify keys; to avoid needless
collisions it's recommended to use at least the last 16 characters of
the hex representation of the fingerprint, which is the key id.[1]
[1] https://tools.ietf.org/html/rfc4880#section-12.2
Bug: 545673
Change-Id: If6fb4879502b6ee4b8c26c21b2714aeac4e4670c
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
In 7b96bd812e ("UploadPack: Use reachability checker to validate
non-advertised wants", 2019-05-16), a "walk.setRetainBody(false);"
statement was inadvertently deleted. (An earlier version of this commit
had this line in another part of the code and a review comment suggested
to move it back here; the line was then deleted from the other part of
the code but not readded.) Restore this line.
Change-Id: I96ff6106ba9e4eef429388c83e898b3363295f69
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
In "Reachable commit" request validators, we need to check that a "want"
in the request, that hasn't been advertised, is reachable from the refs
visible to the user.
Current code has intermixed the translation of ObjectIds to RevCommits
(and its error handling) with the actual walk, with the delegation to
bitmaps in restricted circunstances.
Refactor the code to make it "flatter" and more readable. Move ObjectIds
to RevCommits translation to its own functions. Use the reachability
checker instead of a newly defined walk.
Before the non-advertised wants were validated with bitmaps only if any
"want" refered to an non-commit. Now they will be validated with bitmaps
also if the "wants" refer all to commits.
Change-Id: Ib925a48cde89672b07a88bba4e24d0457546baff
Signed-off-by: Ivan Frade <ifrade@google.com>
The "basic" reachability check walks the graph starting from the tips
marking things as "uninteresting". If the target commit is marked as
"uninteresting" it was reached; it is reachable from those tips.
This requires a lot of walking and can be solved directly with bitmaps.
Most of the time the bitmaps are already calculated or a short walk
away.
This should improve the performance of reachability checks, for example
in Gitiles.
Change-Id: I83d33271f58d95d2dc9ed151967b3eda513c99f7
Signed-off-by: Ivan Frade <ifrade@google.com>
To make reachability checks with bitmaps, we need to get the
reachability bitmap of a commit, which is not always precalculated.
There is already a class returning such bitmap (BitmapWalker) but it
does too much unnecessary work: it calculates ALL reachable objects from
a commit (i.e. including trees and blobs), when for reachability the
commits are just enough.
Introduce BitmapCalculator to get the bitmap of a commit: either because
it is precalculated or generating it with a walk only over commits.
Change-Id: Ibb6c78affe9eeaf1fa362a06daf4fd2d91c1caea
Signed-off-by: Ivan Frade <ifrade@google.com>
It is common to check if a certain commit is reachable from some
starting points. For example gitiles does it to check if a commit
is visible to a user based on its permissions.
Offer this functionality in JGit.
Split the interface as the next commit will introduce an implementation
using bitmap indices.
Change-Id: I0933b305c8d734f7a64502910ff4d9ef4fc92ae1
Signed-off-by: Ivan Frade <ifrade@google.com>
- ObjectWalk.getTreeDepth() returns int hence there is no need to use
long depths in the lowestDepthVisited map.
- Also fix boxing warnings introduced in 0a15cb3a.
Change-Id: I6d73b6f41d5d20975d02f376c8588e411eaff0ec
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Replace:
in the "core section"
in the "diff section"
in the "merge section"
with:
in the "core" section
in the "diff" section
in the "merge" section
Change-Id: Ided7bf73e9c8aae4fc4e43d5d5b9f6d7e3066f0a
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse
Matthias Sohn
David Ostrovsky
Brandon Weeks
Sebastian Schuberth
Marc Strapetz
Ivan Frade
Andrey Loskutov
Dave Borowitz
Thomas Wolf
Jonathan Tan
Carsten Hammer