David Pursehouse [Tue, 12 Jun 2018 03:50:32 +0000 (12:50 +0900)]
Merge branch 'stable-5.0'
* stable-5.0:
Empty merge. The stable-4.9 branch was merged directly into stable-5.0
by [1], and then there were separate merges up through stable-4.10 to
stable-4.11 by [2] and [3].
When stable-4.11 was merged in to stable-5.0 in [4] it was an empty
merge, since the change had already been brought in by [1].
Michael Keppler [Sun, 10 Jun 2018 12:15:45 +0000 (14:15 +0200)]
Fix Javadoc typo
Change-Id: Ib4ebc57236bdea663f27295764886413e2550580 Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de> Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Thomas Wolf [Fri, 8 Jun 2018 13:47:05 +0000 (15:47 +0200)]
Ensure Jsch checks all configured algorithms
Jsch checks only for the availability of the algorithms given by
Jsch-internal config keys "CheckCiphers", "CheckKexes", and
"CheckSignatures". If the ssh config defines any algorithms
unknown to Jsch not listed in those keys, it'll still propose them
during the negotiation phase, and run into an NPE later on if the
server happens to propose such an algorithm and it gets chosen.
Jsch reads those "CheckCiphers" and the other values from either a
session-local config, or the global static Jsch config. It bypasses
~/.ssh/config for these values.
Therefore, copy these values from the config as read from
~/.ssh/config into the session-specific config. That makes Jsch
check _all_ configured algorithms up front, discarding any for
which it has no implementation. Thus it proposes only algorithms
it actually can handle.
Bug: 535672
Change-Id: I6a68e54f4d9a3267e895c536bcf3c58099826ad5 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Dave Borowitz [Fri, 5 Jan 2018 18:02:47 +0000 (13:02 -0500)]
Ensure DirectoryStream is closed promptly
From the javadoc for Files.list:
"The returned stream encapsulates a DirectoryStream. If timely disposal
of file system resources is required, the try-with-resources construct
should be used to ensure that the stream's close method is invoked
after the stream operations are completed."
This is the only call to Files#newDirectoryStream that is not already in
a try-with-resources.
Matthias Sohn [Fri, 8 Jun 2018 15:45:00 +0000 (17:45 +0200)]
Validate branch names on branch creation
Since v2.16.0-rc0~89^2~1 (branch: correctly reject
refs/heads/{-dash,HEAD}, 2017-11-14),
native git does not allow branch names
- refs/heads/HEAD
- starting with '-'
Bug: 535655
Change-Id: Ib1c4ec9ea844073901a4ebe6a29ff6cc8ae58e93 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Update maven plugins to fix Zip Slip vulnerability
Zip Slip [1] is an arbitrary file write generic vulnerability, that can
be achieved using a specially crafted zip (or bzip2, gzip, tar, xz, war)
archive, that holds path traversal filenames.
According to Maven's announcement [2] several plugins use plexus-archiver to
unpack dependencies to disk and have been identified as potential triggers
for exposing the vulnerability.
Of those, JGit uses the maven-dependency-plugin and the maven-javadoc-plugin.
Update them to the fixed versions reported in [2].
See the corresponding issues for the maven-dependency-plugin [3] and the
maven-javadoc-plugin [4] for details.
Jonathan Nieder [Tue, 5 Jun 2018 05:22:24 +0000 (22:22 -0700)]
Merge branch 'stable-5.0'
* stable-5.0:
Teach UploadPack "filter" in protocol v2 fetch
Refactor test of capabilities output
Refactor v2 advertisement into own function
Refactor parsing of "filter" into its own method
Disallow unknown args to "fetch" in protocol v2
Teach UploadPack shallow fetch in protocol v2
Refactor unshallowCommits to local variable
Add protocol v2 support in http
Give info/refs services more control over response
Change-Id: I1683902222e076e1091795e94790a264550afb7b Signed-off-by: Jonathan Nieder <jrn@google.com>
Jonathan Tan [Mon, 30 Apr 2018 20:21:43 +0000 (13:21 -0700)]
Teach UploadPack "filter" in protocol v2 fetch
If the configuration variable uploadpack.allowfilter is true, advertise
that "filter" is supported, and support it if the client sends such an
argument.
Change-Id: I7de66c0a0ada46ff71c5ba124d4ffa7c47254c3b Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Jonathan Tan [Wed, 2 May 2018 23:35:48 +0000 (16:35 -0700)]
Refactor test of capabilities output
A subsequent patch will dynamically generate the capability
advertisement, so the capability advertisements produced are not always
the same. Separate the checking of the advertisements into its own test
method.
Change-Id: I768d14b9d1a244d5d886c42ffd62ef3957b518fb Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Jonathan Tan [Wed, 2 May 2018 22:43:50 +0000 (15:43 -0700)]
Refactor v2 advertisement into own function
A subsequent patch needs dynamic generation of this advertisement
depending on a configuration variable in the underlying repository, so
refactor it into a function instead of using a constant list.
Change-Id: Ie00584add1fb56c9e88c7b57f75703981ea5bb85 Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Jonathan Tan [Thu, 3 May 2018 00:17:04 +0000 (17:17 -0700)]
Disallow unknown args to "fetch" in protocol v2
JGit's implementation of the fetch command of protocol v2, unlike its
implementation of ls-refs, currently tolerates unknown arguments.
Tighten fetch to not allow unrecognized arguments and add tests to
verify this behavior for both ls-refs and fetch.
Change-Id: I321161d568bd638252fab1a47b06b924d472a669 Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Jonathan Tan [Thu, 15 Mar 2018 22:56:50 +0000 (15:56 -0700)]
Teach UploadPack shallow fetch in protocol v2
Add support for the "shallow" and "deepen" parameters in the "fetch"
command in the fetch-pack/upload-pack protocol v2. Advertise support for
this in the capability advertisement.
Change-Id: I7ffd80d6c38872f9d713ac7d6e0412106b3766d7 Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Jonathan Nieder <jrn@google.com>
Jonathan Tan [Tue, 22 May 2018 22:19:04 +0000 (15:19 -0700)]
Refactor unshallowCommits to local variable
This reduces the amount of state held as instance variables in
UploadPack, and makes it easier for a future patch to contain a clearer
version of UploadPack#processShallow.
Change-Id: I6df80b42f9e5118fda1420692e02e417670cced3 Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Jonathan Nieder <jrn@google.com>
Jonathan Tan [Wed, 28 Feb 2018 22:36:44 +0000 (14:36 -0800)]
Add protocol v2 support in http
Teach UploadPack to support protocol v2 with non-bidirectional pipes,
and add support to the HTTP protocol for v2. This is only activated if
the repository's config has "protocol.version" equal to 2.
Change-Id: I093a14acd2c3850b8b98e14936a716958f35a848 Helped-by: Matthias Sohn <matthias.sohn@sap.com> Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Jonathan Nieder <jrn@google.com>
Jonathan Tan [Wed, 25 Apr 2018 20:59:52 +0000 (13:59 -0700)]
Give info/refs services more control over response
Currently, SmartServiceInfoRefs always prints "# service=serviceName"
followed by a flush packet in response to an info/refs request, and then
hands it off to the specific service class. Printing of "#
service=serviceName" is mandated for protocol v0, but not v2.
Therefore, the existing code works for protocol v0, but whenever a
service that supports protocol v2 receives an info/refs request, it must
first determine which protocol version is to be used (depending on, for
example, the request and any relevant configuration variables), and then
decide if "# service=serviceName" needs to be printed.
Create a new method that v2-supporting service classes can override,
covering the printing of both "# service=serviceName" and everything
that the #advertise method prints. This will be used in a subsequent
commit in which UploadPackServlet (and the other classes it uses) is
updated to support protocol v2.
Change-Id: Ia026b06e96a6b15937514096babd024ef77df1ea Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Jonathan Nieder <jrn@google.com>
Change-Id: Ie149c2fee6c552b7a595f029c267292840734192 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Dave Borowitz [Wed, 30 May 2018 00:35:14 +0000 (17:35 -0700)]
Move DfsPackDescription comparators to common location
There are several ways of comparing DfsPackDescriptions for different
purposes, such as object lookup search order and reftable ordering. Some
of these are later compounded into comparators on other objects, so they
appear in the code as Comparator<DfsReftable>, for example.
Put all the DfsPackDescription comparators in static methods on
DfsPackDescription itself. Stop implementing Comparable, to avoid giving
the impression that there is always one true and correct way of sorting
packs.
Dave Borowitz [Wed, 30 May 2018 19:02:35 +0000 (12:02 -0700)]
Use Comparators for PackSource
Rather than requiring callers to do their own computations based on the
package-private "category" number, provide an actual
Comparator<PackSource> instance, and explicitly discourage usage of
default Enum comparison.
Construct the default comparator using a builder pattern based on
defining equivalence classes. This gives us the same behavior as the old
category field in PackSource, with an abstraction that does not leak the
implementation detail of comparing rank numbers.
Dave Borowitz [Wed, 30 May 2018 00:53:26 +0000 (17:53 -0700)]
DfsPackDescription: Disallow null PackSource
In normal operation, the source of a pack should never be null; the DFS
implementation should always know where a pack came from. Existing
implementations in InMemoryRepository and at Google always have the
source available at construction time.
The problem with null PackSources in the previous implementation was it
made the DfsPackDescription#compareTo method intransitive. Specifically,
it skips comparing the sources at all if *either* operand is null.
Suppose we have three descriptions A, B, and C, where all fields are
equal except the PackSource, and:
* A's source is INSERT
* B's source is null
* C's source is RECEIVE
In this case, A.compareTo(B) == 0, and B.compareTo(C) == 0, since all
fields are equal except the source, which is skipped. But
A.compareTo(C) != 0, since A and B have different sources.
Avoid this problem in compareTo by enforcing that the source is never
null. We could of course assign an arbitrary category number to a null
source in order to make comparison transitive[1], but it's simpler to
implement and reason about if the field is non-nullable, and there is no
real-world use case to make it null.
Although a non-null source is required at construction time, the field
is currently still mutable: DfsPackDecscription#setPackSource is used by
DfsInserterTest to mark packs as garbage. This could probably be
avoided as well, allowing us to convert packSource to a final field, but
doing so is beyond the scope of this change.
[1] The astute reader will notice this is already done by
DfsObjDatabase#reftableComparator(). In fact, the reason that
different comparator implementations non-obviously have different
semantics for this nullable field is another reason why it's clearer
to avoid null entirely.
Thomas Wolf [Thu, 24 May 2018 14:26:36 +0000 (16:26 +0200)]
Don't prune symbolic refs when fetch.prune = true
The canonical implementation also doesn't. Compare current
code in remote.c, function get_stale_heads_cb.[1] Not handling
symrefs in this case was introduced in canonical git in [2]
in 2008.
David Pursehouse [Wed, 23 May 2018 01:57:22 +0000 (10:57 +0900)]
Repository: Deprecate the #getAllRefs method
Callers should instead use getRefDatabase().getRefs(), which does not
swallow the IOException.
Replace @link with @code in the Javadoc of FileRepository, since linking
to the deprecated method causes an error:
Javadoc: The method getAllRefs() from the type Repository is deprecated
Existing callers of the deprecated method are not adapted in this commit
because many of them require more refactoring. They will be done in
separate follow-up commits.
Bug: 534731
Change-Id: Id84e70e4cd7be3d1ca1795512950c6abe3d18ffd Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Matthias Sohn [Tue, 22 May 2018 21:00:37 +0000 (23:00 +0200)]
Merge branch 'master' into stable-5.0
* master:
DescribeCommand: Refactor to not use deprecated Repository#peel
Repository: Deprecate #peel method
Repository: Make #exactRef and #findRef final
Skip ignored directories in FileTreeIterator
Repository: Deprecate getTags method
InfoRefsServlet: Refactor to not use deprecated methods
RefAdvertiser: Add send(Collection<Ref>) and deprecate send(Map<String, Ref>)
Remove deprecated Repository#notifyIndexChanged
Implementors should override Repository#notifyIndexChanged(boolean)
Revive Repository#notifyIndexChanged()
Remove further unnecessary 'final' keywords
Execute AdvertiseRefsHook only for protocol v0 and v1
Add protocol v2 support in "jgit daemon"
Teach UploadPack "ofs-delta" in "fetch"
Teach UploadPack "include-tag" in "fetch"
Avoid using #refs in UploadPack#sendPack
FileRepository: Don't use deprecated RefDatabase#getRefs(String)
BatchRefUpdate: Don't use deprecated RefDatabase#getRefs(String)
Change-Id: I16c5da62d09262d3f4070aa0f466dd6c8352b5ea Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
David Pursehouse [Tue, 22 May 2018 02:27:30 +0000 (11:27 +0900)]
Repository: Deprecate #peel method
Callers should use getRefDatabase().peel(ref) instead since it
doesn't swallow the IOException.
Adapt all trivial callers to user the alternative.
DescribeCommand still uses the deprecated method and is not adapted in
this change since it will require more refactoring to add handling of
the IOException.
Change-Id: I14d4a95a5e0570548753b9fc5c03d024dc3ff832 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Pursehouse [Tue, 22 May 2018 00:47:16 +0000 (09:47 +0900)]
Repository: Make #exactRef and #findRef final
This means less cognitive overhead for both implementors and callers,
since this way we can guarantee that they are always synonyms for
RefDatabase#exactRef and RefDatabase#findRef, respectively.
Change-Id: Ic8aeb52fc7ed65672f3f6cd1da39a66908d88baa Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Thomas Wolf [Tue, 27 Mar 2018 20:22:09 +0000 (22:22 +0200)]
Skip ignored directories in FileTreeIterator
Make FileTreeIterator not enter ignored directories by default. We
only need to enter ignored directories if we do some operation against
git, and there is at least one tracked file underneath an ignored
directory.
Walking ignored directories should be avoided as much as possible as
it is a potential performance bottleneck. Some projects have a lot of
files or very deep hierarchies in ignored directories; walking those
may be costly (especially so on Windows). See for instance also bug
500106.
Provide a FileTreeIterator.setWalkIgnoredDirectories() operation to
force the iterator to iterate also through otherwise ignored
directories. Useful for tests (IgnoreNodeTest, CGitIgnoreTest), or
to implement things like "git ls-files --ignored".
Add tests in DirCacheCheckoutTest, and amend IndexDiffTest to test a
little bit more.
Bug: 388582
Change-Id: I6ff584a42c55a07120a4369fd308409431bdb94a Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Jonathan Nieder [Fri, 18 May 2018 16:16:16 +0000 (09:16 -0700)]
Implementors should override Repository#notifyIndexChanged(boolean)
Declare Repository#notifyIndexChanged() final and
Repository#notifyIndexChanged(boolean) abstract to force implementors
to switch to overriding the latter method. This makes Repository less
error-prone to extend since implementors no longer need to remember to
override one of those two methods.
Change-Id: I721db0f4a4865db3b35212ee0a2045d5b31c96af Signed-off-by: Jonathan Nieder <jrn@google.com>
Jonathan Nieder [Fri, 18 May 2018 16:15:30 +0000 (09:15 -0700)]
Revive Repository#notifyIndexChanged()
e9e150fdd24d (Store in IndexChangedEvent if it was caused by JGit
itself, 2018-05-13) modified Repository#notifyIndexChanged to take a
boolean argument to indicate whether the index change happened under
the current process's control or externally, for use by EGit. In
other words, the function signature changed from
public abstract void notifyIndexChanged();
to
public abstract void notifyIndexChanged(boolean internal);
Callers outside JGit itself notifying a Repository about index changes
are expected to be rare, so this is not very disruptive to them. In
most cases they would be notifying about changes that they made
themselves, so treating their notifyIndexChanged() calls as
notifyIndexChanged(true) should be relatively safe.
Implementors have the opposite problem: adding the new "abstract void
notifyIndexChanged(boolean)" method means they are obligated to
override it. Add a default implementation that calls their existing
override of notifyIndexChanged() to make their migration easier.
The main downside is that authors of new Repository subclasses that
do not realize they need to override notifyIndexChanged would end up
with a default implementation which calls notifyIndexChanged(true),
in turn calling notifyIndexChanged() again and so on, resulting in
StackOverflowException. Add an implementors' note to the class
Javadoc to avoid this issue. A followup commit will force
implementors to adapt to the new API by changing the methods to
@Deprecated
public final void notifyIndexChanged() {
notifyIndexChanged(true);
}
public abstract void notifyIndexChanged(boolean internal);
Change-Id: I7d014890ee19abf283ea824d9baa9044bfdde130 Signed-off-by: Jonathan Nieder <jrn@google.com>
Masaya Suzuki [Mon, 14 May 2018 21:13:11 +0000 (14:13 -0700)]
Execute AdvertiseRefsHook only for protocol v0 and v1
Refs are not advertised as part of the protocol v2 capability
advertisement. Don't call AdvertiseRefsHook.
Noticed because many implementations of AdvertiseRefsHook read all
refs in order to call UploadPack#setAdvertisedRefs, causing the
capability advertisement to be as slow as a v0 ref advertisement with
some RefDatabase implementations.
Such an AdvertiseRefsHook is of dubious utility (a better place to
determine which refs are advertised is in the RefDatabase
implementation itself, as in Gerrit), but at any rate since it's not
bringing about any benefit here, we can skip the hook call.
Jonathan Tan [Sat, 24 Feb 2018 00:55:03 +0000 (16:55 -0800)]
Add protocol v2 support in "jgit daemon"
With this patch, a server spawned by "jgit daemon" can be accessed using
protocol v2 from a Git client that supports it (for example, "git" with
the appropriate patches). This is only activated if the repository's
config has "protocol.version" be 2.
This required a change to the package-private #execute methods in
DaemonService to allow passing of extra parameters.
This has been tested with a patched Git.
Change-Id: Icf043efec7ce956d72b075fc6dc7a87d5a2da82a Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Jonathan Tan [Tue, 13 Mar 2018 18:07:36 +0000 (11:07 -0700)]
Teach UploadPack "include-tag" in "fetch"
Add support for the "include-tag" parameter in the "fetch" command in
the fetch-pack/upload-pack protocol v2.
In order to determine which tags to include, only objects pointed to by
refs starting with "refs/tags/" are checked. This restriction is for
performance reasons and to match the behavior of Git (see add_ref_tag()
in builtin/pack-objects.c).
Change-Id: I7d70aa09bcc8a525218ff1559e286c2a610258ca Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Jonathan Nieder <jrn@google.com>
Jonathan Tan [Mon, 23 Apr 2018 20:03:04 +0000 (13:03 -0700)]
Avoid using #refs in UploadPack#sendPack
When OPTION_INCLUDE_TAG is set, UploadPack#sendPack uses the #refs
instance variable as a source of information of tags. A subsequent patch
will need to supply this information to #sendPack without
modifying #refs, so refactor #sendPack to take in this information
through a parameter instead.
Note that prior to this patch, #refs was used twice in #sendPack: once
to generate the argument to PackWriter#setTagTargets, and once to
determine if any tags need to be included in the packfile. This patch
only updates the latter use, since the former is meant not only for
"true" tag targets but any object that should be hoisted earlier during
packing (see the documentation of PackWriter#setTagTargets).
This patch does not introduce any functionality change.
Change-Id: I70ed65a1041334abeda8d4bac98cce7cae7efcdf Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Jonathan Nieder <jrn@google.com>
Introduce new RawText constructor and RawParseUtils.lineMapOrBinary
This makes binary detection exact in ResolveMerger and DiffFormatter
This has the same intention as
Id4342a199628d9406bfa04af1b023c27a47d4014 but preserves backward
compatibility of the signature of RawParseUtils.lineMap.
Change-Id: Ia24a4e716592bab3363ae24e3a46315a7511154f Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Fri, 11 May 2018 12:06:53 +0000 (14:06 +0200)]
Use a secure random generator to seed nonce for digest authentication
https://tools.ietf.org/html/rfc7616 says:
5.12. Parameter Randomness
The security of this protocol is critically dependent on the
randomness of the randomly chosen parameters, such as client and
server nonces. These should be generated by a strong random or
properly seeded pseudorandom source (see [RFC4086]).
Change-Id: I4da5316cb1eb3f59ae06c070ce1c3335e9ee87d6 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
On a local non-NFS filesystem the .git/config file will be orphaned if
it is replaced by a new process while the current process is reading the
old file. The current process successfully continues to read the
orphaned file until it closes the file handle.
Since NFS servers do not keep track of open files, instead of orphaning
the old .git/config file, such a replacement on an NFS filesystem will
instead cause the old file to be garbage collected (deleted). A stale
file handle exception will be raised on NFS clients if the file is
garbage collected (deleted) on the server while it is being read. Since
we no longer have access to the old file in these cases, the previous
code would just fail. However, in these cases, reopening the file and
rereading it will succeed (since it will open the new replacement file).
Since retrying the read is a viable strategy to deal with stale file
handles on the .git/config file, implement such a strategy.
Since it is possible that the .git/config file could be replaced again
while rereading it, loop on stale file handle exceptions, up to 5 extra
times, trying to read the .git/config file again, until we either read
the new file, or find that the file no longer exists. The limit of 5 is
arbitrary, and provides a safe upper bounds to prevent infinite loops
consuming resources in a potential unforeseen persistent error
condition.
Change-Id: I6901157b9dfdbd3013360ebe3eb40af147a8c626 Signed-off-by: Nasser Grainawi <nasser@codeaurora.org> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Jonathan Nieder [Fri, 4 May 2018 22:09:22 +0000 (15:09 -0700)]
Replace http://errorprone.info with https://errorprone.info
That site serves from https now.
Reported-by: Nicholas Glorioso <glorioso@google.com>
Change-Id: I2150a18425a1fe3ab5a022882ffe06ccbde17f16 Signed-off-by: Jonathan Nieder <jrn@google.com>
- suppress "Failed to load class "org.slf4j.impl.StaticLoggerBinder"
output
- add org.eclipse.jgit.pgm.test tests to bazel build
- supply package information so that "jgit version" can work