getRef and exactRef can produce recoverable exceptions --- for
example, a corrupt loose ref that cannot be parsed. If readRef was
called and updated looseRefs in the process, RefsChangedEvent should
still be fired.
Noticed while improving the implementation of getRef. This commit
only affects exactRef and getRef. Other methods might be similarly
skipping firing RefsChangedEvent in their error handling code, and
this change does not fix them.
Change-Id: I0f460f6c8d9a585ad8453a4a47c1c77e24a1fb83
Signed-off-by: Jonathan Nieder <jrn@google.com>
Both getRef and exactRef look for a ref or pseudoref in the $GIT_DIR
directory, with careful error handling to handle non-refs like
.git/config.
Avoid the duplication by factoring out a helper that takes care of
this. This should make the code easier to understand and manipulate.
Change-Id: I2ea67816d2385e84e2d3394b897e23df5826ba50
Signed-off-by: Jonathan Nieder <jrn@google.com>
Clients can use --shallow-exclude to obtain information about what
commits are reachable from refs they are not supposed to be able to
see. Plug the hole by allowing the AdvertiseRefsHook and RefFilter to
take effect here, too.
Change-Id: If2b8e95344fa49e10a6a202144318b60d002490e
Signed-off-by: Jonathan Nieder <jrn@google.com>
The AdvertiseRefsHook can be called twice if the following conditions
hold:
1. This AdvertiseRefsHook doesn't set this.refs.
2. getAdvertisedOrDefaultRefs is called after getFilteredRefs.
For example, this can happen when fetchV2 is called after lsRefsV2
when using a stateful bidirectional transport.
The second call does not accomplish anything useful. Guard it with
'if (!advertiseRefsHookCalled)' to avoid wasted work.
Reported-by: Jonathan Tan <jonathantanmy@google.com>
Change-Id: Ib746582e4ef645b767a5b3fb969596df99ac2ab5
Signed-off-by: Jonathan Nieder <jrn@google.com>
Now the reference carries its updateIndex, so the cursor doesn't need
to expose it.
Change-Id: Icbfca46f92a13f3d8215ad10b2a166a6f40b0b0f
Signed-off-by: Ivan Frade <ifrade@google.com>
In DFS implementations the reference table can fall out of sync, but
it is not possible to check this situation in the current API.
Add a property to the Refs indicating the order of its updates. This
version is set only by RefDatabase implementations that support
versioning (e.g reftable based).
Caller is responsible to check if the reference db creates versioned
refs before accessing getUpdateIndex(). E.g:
Ref ref = refdb.exactRef(...);
if (refdb.hasVersioning()) {
ref.getUpdateIndex();
}
Change-Id: I0d5ec8e8df47c730301b2e12851a6bf3dac9d120
Signed-off-by: Ivan Frade <ifrade@google.com>
In the longer term, we can add support for this to the
RequestValidator interface. In the short term, this is a minimal
band-aid to ensure any refs the client requests are visible to the
client.
Change-Id: I0683c7a00e707cf97eef6c6bb782671d0a550ffe
Reported-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
ProtocolV2Parser explains:
// TODO(ifrade): This validation should be done after the
// protocol parsing. It is not a protocol problem asking for an
// unexisting ref and we wouldn't need the ref database here.
Do so. This way all ref database accesses are in one place, in the
UploadPack class.
No user-visible change intended --- this is just to make the code
easier to manipulate.
Change-Id: I68e87dff7b9a63ccc169bd0836e8e8baaf5d1048
Signed-off-by: Jonathan Nieder <jrn@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
AdvertiseRefsHook is used to limit the visibility of the refs in Gerrit.
If this hook is not called, then all refs are treated as visible.
In protocol v2, the hook is not called, causing the server to advertise
all refs. This bug was introduced in v5.0.0.201805221745-rc1~1^2~9
(Execute AdvertiseRefsHook only for protocol v0 and v1, 2018-05-14).
Even before then, the hook was not called in requests after the
capability advertisement, so in transports like HTTP that do not retain
state between round-trips, the server would advertise all refs in
response to an ls-refs (ls-remote) request.
Fix both cases by using getAdvertisedOrDefaultRefs to retrieve the
advertised refs in lsRefs, ensuring the hook is called in all cases that
use its result.
[jn: backported to stable-5.0; split out from a larger patch that also
fixes protocol v0; avoided filtering this.refs by ref prefix]
Change-Id: I64bce0e72d15b90baccc235c067e57b6af21b55f
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
AdvertiseRefsHook is used to limit the visibility of the refs in Gerrit.
If this hook is not called, then all refs are treated as visible,
causing the server to serve commits reachable from branches the client
should not be able to access, if asked to via a request naming a guessed
object id.
This bug was introduced in v2.0.0.201206130900-r~123 (Modify refs in
UploadPack/ReceivePack using a hook interface, 2012-02-08). Stateful
bidirectional transports are not affected.
Fix it by moving the AdvertiseRefsHook call to
getAdvertisedOrDefaultRefs, ensuring the hook is called in all cases.
[jn: backported to stable-4.5 by splitting out tests and the protocol v2
specific parts]
Change-Id: I159f396216354f2eda3968d17802e166d8c8ec2d
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
When a server sends a ref advertisement using protocol v2 it contains
lines other than ref names and sha1s. Attempting to get the sha1 out
of such a line using the substring method can result in a SIOOB error
when it doesn't actually contain the sha1 and ref name.
Add a check that the line is of the expected length, and subsequently
that the extracted object id is valid, and if not throw an exception.
Change-Id: Id92fe66ff8b6deb2cf987d81929f8d0602c399f4
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
UploadPack has a setTransferConfig method which allows to set the
transfer config, however since the constructors of TransferConfig
have the default package visibility it is not possible for any
application using UploadPack, for example Gerrit, to actually set
a transfer config.
Make the constructors public. This is consistent with the public
constructors for example on PackConfig.
Change-Id: I07080255838421871403b2b2bcc294aa8f621c57
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
This makes the implementation consistent with the other similar
methods in this class.
Change-Id: I007876aad883615d696c8eabc886818ae00b10ee
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
The setProtocolV2Hook sets the protocolV2Hook to whatever value is
passed, which could be null, but the invocations of protocolV2Hook's
methods are not guarded by null-checks.
Annotate the parameter as @Nullable and set ProtocolV2Hook.DEFAULT
when null is passed. This makes the implementation consistent with
other similar methods that set a hook or filter with possible null
value.
Change-Id: I70919a3248d4c2658783941a37c47e437cff0baa
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
The class has several methods where passing a null parameter is
valid. Annotate those parameters as @Nullable.
Change-Id: Ie08893ee3ab34c1ffb2db875b4ab049ad065c697
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Set GIT_DIR and GIT_WORK_TREE when calling hooks.
Bug: 541622
Change-Id: I6153d8a6a934ec37a3a5e7319c2d0e516f539ab7
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
After cloning a repo with a submodule, non-recursively, JGit would
encounter in its TreeWalk in IndexDiff:
* first, a missing gitlink (in index & HEAD, not in working tree)
* second, the untracked folder (not in index and head, in working tree)
As a result, it would report the submodule as missing. Canonical git
reports a clean workspace.
The root cause of this is that the path of a gitlink "x" did
not compare equal to the path of a tree "x" in JGit.
Correct Paths.compare() to account for that. If two paths are otherwise
equal, then let gitlinks match both trees and files. Matching trees
solves the bug. Matching files is necessary to handle the case where
the gitlink directory was replaced by a file; see the new test case
IndexDiffSubmoduleTest.testSubmoduleReplacedByFile(). Comparisons of
unequal paths are left untouched, so the sort order is unchanged.
After the fix, another bug(?) in WorkingTreeIterator became apparent:
with core.dirNoGitLinks = true, it was no longer possible to overwrite
a gitlink in the index. This is now fixed in WorkingTreeIterator.
Add new test cases for the bug itself and for some related cases
(submodule directory deleted or replaced by a file) in
IndexDiffSubmoduleTest. Add a test for missing files in IndexDiffTest,
and adapt the PathsTest to test matching gitlinks.
Bug: 467631
Change-Id: I0549d10d46b1858e5eec3cc15095aa9f1d5f5280
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
when multiple match options are given in git describe the result must
not depend on the order of the match options. JGit wrongly picked the
first match using the match options in the order they were defined. Fix
this by concatenating the streams of matching tags for all match options
and then choosing the first match on the concatenated stream sorted in
tie break order.
See https://git-scm.com/docs/git-describe#git-describe---matchltpatterngt
Change-Id: Id01433d35fa16fb4c30526605bee041ac1d954b2
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
According to RFC 2616 [1] header field names are case insensitive.
Header fields defined as a comma separated list can have multiple header
fields with the same field name. Add a method to HttpConnection which
retrieves all values with a given header field name with the field name
compared case insensitive.
[1] https://tools.ietf.org/html/rfc2616#section-4.2"
Change-Id: I7f601b21cda99e84f43f866c7c7cb4cb0e3cf5c3
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This partially reverts commit a551b646: revert the changes in
RawParseUtils.lineMap(). Forcing all blobs containing a NUL byte
as a single line causes blame to produce useless results as soon
as it hits any version containing a NUL byte.
Doing binary detection at this level also has the problem that the
user cannot control it. Not by setting the text attribute nor in any
other way.
This came up in bug 541036, where a Java source inadvertently
contained NUL bytes in strings. Even fixing this by using escapes
"\000" will not fix JGit's blame for this file because the past
versions will still contain the NUL byte.
Native git can blame that file from bug 541036 fine.
Added new tests verifying that blaming a text file containing a NUL
byte produces sensible results.
Bug: 541036
Change-Id: I8991bec88e9827cc096868c6026ea1890b6d0d32
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This continues what commit d9ac7ddf10
(Remove unnecessary modifiers from interfaces, 2018-11-15) started.
Change-Id: I89720985a5a986722a0dcb9b5e9bbc25996bd5b3
This reverts the workaround introduced by
1c6c73c5a9, which is a patch for dealing
with a buggy C Git client v1.7.5 in 2012. We'll stop supporting very old
C Git clients.
Change-Id: I94999a39101c96f210b5eca3c2f620c15eb1ac1b
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
From Oracle's "Defining an interface":
"All abstract, default, and static methods in an interface are
implicitly public, so you can omit the public modifier."
(Without any modifier, the interface methods are also abstract, so we
omit also the "abstract")
"In addition, an interface can contain constant declarations. All
constant values defined in an interface are implicitly public, static,
and final. Once again, you can omit these modifiers."
This makes the code more consistent. Now all interfaces under
org.eclipse.jgit follow the guidelines.
Change-Id: I4fe6deb111899ec1b4318ab5a6050f3851fa1fd3
Signed-off-by: Ivan Frade <ifrade@google.com>
Add a new ssh client implementation based on Apach MINA sshd 2.0.0.
This implementation uses JGit's own config file parser and host entry
resolver. Code inspection of the Apache MINA implementation revealed
a few bugs or idiosyncrasies that immediately would re-introduce bugs
already fixed in the past in JGit.
Apache MINA sshd is not without quirks either, and I had to configure
and override more than I had expected. But at least it was all doable
in clean ways.
Apache MINA boasts support for Bouncy Castle, so in theory this should
open the way to using more ssh key algorithms, such as ed25519.
The implementation is in a separate bundle and is still not used in
the core org.eclipse.jgit bundle. The tests re-use the ssh tests from
the core test bundle.
Bug: 520927
Change-Id: Ib35e73c35799140fe050d1ff4fb18d0d3596580e
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Add support for git-receive-pack to the ssh git server and add two
new tests for pushing.
This actually uncovered an undocumented requirement in TransportSftp:
the FTP rename operation assumes POSIX semantics, i.e., that the
target is removed. This works as written only for servers that
support and advertise the "posix-rename@openssh.com" FTP extension.
Our little Apache MINA server does not advertise this extension.
Fix the FtpChannel implementation for Jsch to handle this case in a
meaningful way so that it can pass the new "push over sftp" test.
Add more tests to test the behavior of server host key checking.
Also refactor the tests generally to separate better the test
framework from the actual tests.
Bug: 520927
Change-Id: Ia4bb85e17ddacde7b36ee8c2d5d454bbfa66dfc3
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Introduce an FtpChannel abstraction, which can be obtained from a
RemoteSession. In JSchSession, wrap a JSch ChannelSftp as such an
FtpChannel. The JSch-specific SftpException is also mapped to a
generic FtpException. Rewrite TransportSftp to use only the new
abstraction layer.
This makes it possible to provide alternate ssh/sftp implementations.
Bug: 520927
Change-Id: I379026f7d4122f34931df909a28e73c02cd8a1da
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
The lock is obtained in receivePackAndCheckConnectivity. It seems to me
the structure that requres the caller to unlock the lock is wrong, but
at least by calling in finally ensures it is called even if an exception
is thrown.
Change-Id: I123841b017baf5acffe0064d1004ef11a0a5e6c2
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
Do not add an artificial line break to the message, since it may become
much wider due to the embedded exception messages anyway.
The layout shall be controlled by the egit supplied message dialog using
layout constraints.
Bug: 540537
Change-Id: I4257b52e5e59689dfcbab47bd7c075b3fd031837
Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de>
Jonathan Nieder
Ivan Frade
Matthias Sohn
Masaya Suzuki
David Pursehouse
Mincong HUANG
Thomas Wolf
Gunnar Wagenknecht
Michael Keppler