Cache user global and system-wide git configurations
So far the git configuration and the system wide git configuration were
always reloaded when jgit accessed these global configuration files to
access global configuration options which are not in the context of a
single git repository. Cache these configurations in SystemReader and
only reload them if their file metadata observed using FileSnapshot
indicates a modification.
Change-Id: I092fe11a5d95f1c5799273cacfc7a415d0b7786c
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Use Instant instead of milliseconds for filesystem timestamp handling
This enables higher file timestamp resolution on filesystems like ext4,
Mac APFS (1ns) or NTFS (100ns) providing high timestamp resolution on
filesystem level.
Note:
- on some OSes Java 8,9 truncate milliseconds, see
https://bugs.openjdk.java.net/browse/JDK-8177809, fixed in Java 10
- UnixFileAttributes truncates timestamp resolution to microseconds when
converting the internal representation to FileTime exposed in the API,
see https://bugs.openjdk.java.net/browse/JDK-8181493
- WindowsFileAttributes also provides only microsecond resolution
Change-Id: I25ffff31a3c6f725fc345d4ddc2f26da3b88f6f2
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
UploadPack: Filter refs used for deepen-not resolution
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>
UploadPack: Filter refs used for want-ref resolution
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>
UploadPack: Defer want-ref resolution to after parsing
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>
BasePackConnection: Check for expected length of ref advertisement
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>
Add a method to get all values of HTTP header defined as list
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 reverts the workaround introduced by
1c6c73c5a9b8dd700be45d658f165a464265dba7, 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 more ssh tests: pushing, known_host file handling, etc.
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>
Move the bulk of the basic parsing and host entry handling into a
new class OpenSshConfigFile that has no dependencies on any concrete
ssh implementation. Make the existing OpenSshConfig use the new
parser.
Introduce a new class SshConstants collecting all the various ssh-
related string literals. Also use TreeMaps with a case-insensitive
key comparator instead of converting keys to uppercase. Add a test
to verify that keys are matched case-insensitively.
Most of the parsing code was simply moved, except that the new
parser supports looking up entries given host name, port, and user
name, and can thus handle more %-substitutions correctly. This
feature is not yet used and cannot be used with JSch since JSch
only has a ConfigRepository.getConfig(String) interface.
The split is still worth the trouble as it opens the way to using
another ssh client altogether. Apache MINA sshd, for instance,
resolves host entries giving host name, port, and user name.
(Apache MINA has a built-in ssh config handling, but that has
problems, too: its pattern matching is case-insensitive, and its
merging of host entries if several match is not the same as in
OpenSsh. But with this refactoring, it will be possible to plug in
OpenSshConfigFile into an Apache MINA sshd client without dragging
along JSch.)
One test case that doesn't make sense anymore has been removed. It
tested that repeatedly querying for a host entry returned the same
object. That is no longer true since the caching has been moved to
a deeper level.
Bug: 520927
Change-Id: I6381d52b29099595e6eaf8b05c786aeeaefbf9cc
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This allows clients to use the --shallow-exclude parameter (producing a
"deepen-not <ref>" line when communicating with the server) in their fetch
commands when fetching against a JGit server using protocol v2.
Note that the implementation in this commit is somewhat inefficient, as
described in the TODO comment in DepthGenerator.
Change-Id: I9fad3ed9276b624d8f668356ffd99a067dc67ef7
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Throw error when deepen-since excludes all commits
In C Git, when a client fetches with "git fetch --shallow-since=<date>
origin <ref>", and all commits reachable from <ref> are older than
<date>, the server dies with a message "no commits selected for shallow
requests". That is, (1) the --shallow-since filter applies to the commit
pointed to by the ref itself, and (2) there is a check that at least one
commit is not filtered out. (The pack-protocol.txt documentation does
not describe this, but the C implementation does this.)
The implementation in commit 1bb430dc21 ("UploadPack: support
deepen-since in protocol v2", 2018-09-27) does neither (1) nor (2), so
do both of these.
Change-Id: I9946327a71627626ecce34ca2d017d2add8867fc
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
UploadPack v0: Extract "agent" client capability at parse time
The request receives a list of capabilities and takes out the "agent" to
offer it on its own setter (getAgent).
Do this at parse time: when reading the line if the capability is
"agent" set it directly in the builder.
This makes the treatment of "agent" consistent in v0/v1 and v2.
Change-Id: Ie4f9f2cad8639adeeaef4921df49a30a8ce5b42f
Signed-off-by: Ivan Frade <ifrade@google.com>
Use Integer.valueOf() to avoid the warning by implicit conversion due to
usage as argument object in String.format().
Change-Id: Ib314f629d54ae1ce9729c3837d66ce8982a1898a
Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de>
UploadPack: Return correct peer user agent on v2 requests
UploadPack.getPeerUserAgent() doesn't produce the expected results for
protocol v2 requests. In v2, the agent reported in the request (in an
"agent=" line) is not in the clientCapabilities but in a field on its
own. This makes getPeerUserAgent default to the transport user agent.
Making "agent" a shared property between protocol v0/v1 and v2 fixes the
problem, simplifies the function and harmonizes the implementation
between protocol versions.
In a follow up commit the "agent" will be identified on parsing time,
instead of taking it from the client capabilities.
Change-Id: Idf9825ec4e0b81a1458c8e3701f3e28aafd8a32a
Signed-off-by: Ivan Frade <ifrade@google.com>
Accept protocol v2 server options on fetch and ls-refs requests
In protocol v2, a command request can be followed by server options
(lines like "agent=<>" and "server-option=<>"), but current code
doesn't accept those lines.
Advertise the "server-option" capability, parse the lines and add
them to the request objects.
Other code in JGit can see this options and act accordingly via the
protocol v2 hooks.
This should not require any change in the client side.
Change-Id: If3946390f9cc02d29644b6ca52534b6f757bda9f
Signed-off-by: Ivan Frade <ifrade@google.com>
UploadPack.FirstLine: Revert renaming of getOptions into getCapabilities
This is public facing, stable API.
Fortunately, this class is deprecated and will be removed in the next
major version bump.
Change-Id: I91193964732e9d1943e9dc613256196e9c9d1274
Signed-off-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
JSch prefers ssh-rsa key type. When the remote server supports ssh-rsa
key type then this key type will be used even if the known_hosts file
contains a host key for that host, but with different key type.
This caused an unexpected UnknownHostKey error.
To fix the issue first scan the known_hosts, the HostKeyRepository in
JSch API, for any already existing host keys for the target host and
modify the default session settings to prefer their algorithms. However,
do this only if there is no HostKeyAlgorithms setting active.
Change-Id: I236df2a860ddd9289a0a820ddf09c2dea3673d36
UploadPack: Use request instead of field for depth
One more step in removing state from UploadPack, using the request
object instead.
Unfortunately, hooks get from UploadPack information about the current
request. Changing the hooks to receive the request is a public API
change, so at the moment lets keep a reference to the current request.
This kills half the benefit of using a request object vs fields, but
at least we still get better modularity.
Change-Id: I86803d876a621b727c66ee73f2880c93190629e9
Signed-off-by: Ivan Frade <ifrade@google.com>
UploadPack: Use request in computeShallowUnshallow
All data required in this function is available in the request object.
Use that object instead of class members. This reduces class state and
is more readable.
Make the function use a request object and remove the now unnecessary
field "deepenNotRefs".
Change-Id: If861e44c2860a78cf19f456d1b3feb7ddc314cce
Signed-off-by: Ivan Frade <ifrade@google.com>
Move deepenSince and deepenNotRefs up to FetchRequest
These properties are protocol v2 specific, but they have clear default
no-op values and having them in the common superclass simplifies client
code.
Move properties deepenSince and deepenNotRefs up to FetchRequest. In
FetchV0Request, they are initialized with their no-op values (0 for
deepenSince and empty list for deepenNotRefs)
Change-Id: I9d46a6dfbe29ebd794b5a6482033cdc70d411a23
Signed-off-by: Ivan Frade <ifrade@google.com>
UploadPack: Use request instead of field for filterBlobLimt
filterBlobLimit is not part of the UploadPack state, and as field
of the class is difficult to see where it is set or accessed.
Use the request object instead of a field. This reduces
UploadPack state and makes clearer how the value is used.
Change-Id: I96a04a5a8b31bf2243de701e1fd7ebb4080b49e2
Signed-off-by: Ivan Frade <ifrade@google.com>
Mark fetch requests fields as final and @NonNull when possible
Mark reference fields as final, annotate constructor parameters and
getters as @NonNull when appropiate and assert the incoming references
are non-null.
Change-Id: I0ef9a513a99313bf461fe9629ce6cc8b409bdedb
Signed-off-by: Ivan Frade <ifrade@google.com>
Create FetchRequest superclass with common elements
Some code apply to both, v1 and v2 requests, so it should receive
just a request instance.
Move all common fields to an abstract superclass that can be passed
to "version neutral" functions.
Change-Id: I47c22fb12065bc93767f78175e2b36cc43ccb5c5
Signed-off-by: Ivan Frade <ifrade@google.com>
In FetchV0Request, the fields "wantsIds" and "options" are called
"wantIds" and "clientCapabilities". Those names describe them better.
Rename FetchV2Request fields to follow fetch v0. This will make easier
to extract a superclass later.
Take also the chance to polish the javadoc.
Change-Id: Ia17dbbab8084f39cc529fef9ca5c65e189073767
Signed-off-by: Ivan Frade <ifrade@google.com>
Move protocol v0/v1 parsing to its own class and request objects
Protocol v0/v1 parsing code doesn't have any real dependency on UploadPack.
Move it to its class and use a request object to read the data in
UploadPack.
This makes the code easier to test, keeps similar structure than protocol v2,
reduces the line count of UploadPack and paves the way to remove the
members as implicit parameters in it.
Change-Id: I8188da8bd77e90230a7e37c02d800ea18463694f
Signed-off-by: Ivan Frade <ifrade@google.com>
Uncaught exceptions are handled by java.lang.Thread's handler, which
prints it to stderr.
This is useful because InternalPushConnection is used in tests, and
during development, the server side may have programming errors that
manifest as RuntimeExceptions.
Before this change, all types of failures would lead to a uniform
failure message "test://test/conn0: push not permitted" on the client.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Change-Id: I0983cbeb86d36fa7a9313373f5fce54971f804ec
First-want line parsing accepts lines with an optional whitespace, when
the spec is strict requiring a white space.
Validate the line enforcing that there is a white space between oid and
capabilities list.
Change-Id: I45ada67030e0720f9b402c298be18c7518c799b1
Signed-off-by: Ivan Frade <ifrade@google.com>
Move first line parsing for v0/v1 pack negotiation out of UploadPack
In protocol v0/v1 pack negotiation, the first want line contains the
options the client wants in effect. This parsing is done in UploadPack
but it doesn't have any interaction with that class.
Move the code to its own class and package, mark the current one
as deprecated (it is public API) and add unit tests.
Take the chance to move the parsing code from the constructor to a
factory method, making the class a simple container of results.
Change-Id: I1757f535dda78a4111a1c12c3a3b455a4b6f0c51
Signed-off-by: Ivan Frade <ifrade@google.com>
Format @Nullable on return value as method annotation
For example, instead of using
public @Nullable String getMyFavoriteString() { ... }
use
@Nullable
public String getMyFavoriteString() { ... }
This makes the style more consistent (the existing JGit code base
tends to lean toward the second style) and makes the source code
better reflect how the annotation is parsed, as a METHOD annotation.
Longer term, we should switch to a TYPE_USE annotation and switch to
the first style.
Noticed using a style checker that follows
https://google.github.io/styleguide/javaguide.html#s4.8.5-annotations
Change-Id: I07f4e67cc149fb8007f696a4663e10d4bfc57e3a
Reported-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
The fsck test needs more detail about the error than an IOException
with an explanatory message.
Add an error identifier to the SubmoduleValidatorException and make
it the only throwable exception when parsing a file.
Change-Id: Ic3f0955b497e1681b25e681e1282e876cdf3d2c5
Signed-off-by: Ivan Frade <ifrade@google.com>
RefAdvertiser: Add Javadoc for usage of send(Collection)
This was not updated with the original introduction of the new method.
Bug: 534731
Change-Id: Ic4589c3a209109a829fbb706a9bf38845134e904
Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de>
The main concern are submodule urls starting with '-' that could pass as
options to an unguarded tool.
Pass through the parser the ids of blobs identified as .gitmodules
files in the ObjectChecker. Load the blobs and parse/validate them
in SubmoduleValidator.
Change-Id: Ia0cc32ce020d288f995bf7bc68041fda36be1963
Signed-off-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn
David Pursehouse
Jonathan Nieder
Masaya Suzuki
Thomas Wolf
Ivan Frade
Michael Keppler
Jonathan Tan
Saša Živkov
Han-Wen Nienhuys