* stable-5.1:
ssh: Prefer algorithms of the known host keys
Change-Id: I4868359c9df75cb2c51331789cb9f34473801569
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
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
Eclipse Photon supports separating sources and test sources.
There are no functional changes in the IDE, except for test source
folders having a different icon color.
Users of different IDEs than Eclipse are not affected, since the
attribute on the classpath entries will be ignored by their IDE.
Bug: 539933
Change-Id: Iac6dcdf0c0730ca775bae90df6a685303dc95380
Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
The code base has several @SuppressWarnings annotations to suppress
warnings raised by Error Prone, but those are not recognized by
Eclipse and there is currently no way to tell it about them [1].
Suppress them for now.
[1] https://bugs.eclipse.org/bugs/show_bug.cgi?id=392045
Change-Id: I3de7cfa8ad4370ca5be71e1303879c73ab6829c1
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Error Prone reports that the unsynchronized method skip overrides the
synchronized method in ByteArrayInputStream [1].
This is a test class, so we can just suppress the warning as recommended
in [1].
Note that the suppression causes a warning in Eclipse, because it doesn't
recognize the "UnsynchronizedOverridesSynchronized" as a valid value for
the @SuppressWarnings annotation [2].
[1] https://errorprone.info/bugpattern/UnsynchronizedOverridesSynchronized
[2] https://bugs.eclipse.org/bugs/show_bug.cgi?id=392045
Change-Id: I3e798b448211f1363729091f72fb0ef6a873e599
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
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>
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>
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>
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 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>
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>
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
5c134f4d removed closing the input stream when we reached end of the
stream. This caused file handle leaks.
Bug: 540049
Change-Id: I48082b537077c7471fc160f59aa04deb99687d9b
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-4.6:
Replace Findbugs with Spotbugs in org.eclipse.jgit/pom.xml
Replace FindBugs with SpotBugs
Change-Id: I24417e4ebbba31f7ff6896d585ef807327411392
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.5:
Replace Findbugs with Spotbugs in org.eclipse.jgit/pom.xml
Replace FindBugs with SpotBugs
Change-Id: I1c077e8f3530ac717b1603d3307fd15d4335b8fe
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
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>
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>
The usage of non-short-circuit logic is intentional, per the inline
comment added in change Ib4b35e357 as a follow-up to Ie3761ffb4 which
was a previously rejected attempt to "fix" a similar warning that had
been raised by FindBugs.
Change-Id: I3f6729f954d45d30ce697356d2ab3cc877d3ad54
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
This reverts commit 37c7fbd661.
These filters weren't unused. Without them Eclipse raises 4 API errors.
Change-Id: I5ce443d40b5f517be4a315479e81246d40af1983
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Check the bazel version using the checker from bazel_skylib, and
require at least version 0.17.1 which is the minimum version that
does not suffer from the Java API mismatch issue [1].
The implementation is borrowed from the Gerrit project.
[1] https://github.com/bazelbuild/bazel/issues/6027
Change-Id: I224250088a1f5072fcaa3ec81228f4d2cb8cb389
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* changes:
Upgrade wagon-ssh to 3.2.0
Upgrade maven-jxr-plugin to 3.0.0
Upgrade maven-shade-plugin to 3.2.0
Upgrade jacoco-maven-plugin to 0.8.2
Upgrade maven-project-info-reports-plugin to 3.0.0
Upgrade maven-enforcer-plugin to 3.0.0-M2
Upgrade maven surefire plugins to 2.22.0
Ensure same version of maven-surefire-plugin and maven-surefire-report-plugin
For example, instead of using
public @NonNull String getMyFavoriteString() { ... }
use
@NonNull
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: I9b9fa08035d805ca660520f812a84d2f47eff507
Reported-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
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>
As described in the javadoc for org.eclipse.jgit.annotations.Nullable:
Warning: Please do not use this annotation on arrays. Different
annotation processors treat `@Nullable Object[]` differently: some
treat it as an array of nullable objects, for consistency with
versions of `Nullable` defined with `@Target TYPE_USE`, while others
treat it as a nullable array of objects. JGit therefore avoids using
this annotation on arrays altogether.
See the checker-framework manual[1] for details.
[1] http://types.cs.washington.edu/checker-framework/current/checker-framework-manual.html#faq-array-syntax-meaning
Change-Id: I14ffcf80adbb8145d797998de2f2fa6ab84c3ae3
Signed-off-by: Jonathan Nieder <jrn@google.com>
Previous commits block the addition to the repo of dangerous .gitmodules
files, but some could have been committed before those safeguards where
in place.
Add a check in DfsFsck to validate the .gitmodules files in the repo.
Use the same validation than the ReceivePack, translating the
results to FsckErrors.
Note that *all* .gitmodules files in the storage will be checked, not
only the latest version.
Change-Id: I040cf1f31a779419aad0292ba5e6e76eb7f32b66
Signed-off-by: Ivan Frade <ifrade@google.com>
errorType is already null in the caller and callee when unknown, so we
can replace a conditional call to a setter in the only caller with an
unconditionally provided @Nullable constructor parameter.
As a bonus, this lets us mark the field as final.
Change-Id: Ie2f929180e74ffa1aba8ec6caccfa81fbd8bfc04
Signed-off-by: Ivan Frade <ifrade@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>
David Pursehouse
Ivan Frade
Saša Živkov
Michael Keppler
Han-Wen Nienhuys
Matthias Sohn
Jonathan Nieder