This is a preparation change to Bazel build implementation. Error
Prone rejects the code with variable crypto algorithm as insecure
see: [1].
[1] http://errorprone.info/bugpattern/InsecureCryptoUsage
Change-Id: I92db70a7da454bc364597a995e8be5dccc2d6427
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
Only testonly targets (such as tests) need to use junit.
In particular this involves making the toplevel :all rule testonly.
It's not clear to me what that rule is for --- "bazel build //..."
already works to build all targets. In any case it appears to be for
testing, so marking it as testonly shouldn't be harmful.
Change-Id: I28ff508ab8ce2ec0a0111109110aa9680d30600e
This provides a place to declare visibility restrictions and
transitive dependencies for each library.
Other targets should only declare dependencies on what they directly
use, making dependencies easier to maintain.
Trim the dependencies of org.eclipse.jgit:jgit to follow that rule.
It declares dependencies on Apache httpcomponents and the servlet
API but doesn't use them.
Tested:
* 'bazel build //...' succeeds
* applying the change https://gerrit-review.googlesource.com/90843
to a copy of Gerrit, following the instructions there, and running
'bazel test //...' in that copy of Gerrit still succeeds
Change-Id: I3ab958ce8b3227019cdbe4cc81e0f042e1541034
With the filename suffix "Tests", the module was not included in tests
when building with Maven, and without the @Test annotations the tests
didn't get executed under Eclipse or buck test.
testRacyGitDetection was failing because the index file did not exist.
Add the missing configuration, the missing annotations, and add a call
to reset() in testRacyGitDetection to force creation of the index file.
Change-Id: I29dd8f89c36fef4ab40bedce7f4a26bd9b2390e4
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
This fixes test flagged by error prone:
LongObjectIdTest.java:294: error: [SelfComparison] An object is
compared to itself
assertEquals(0, id1.compareTo(id1));
Change-Id: Ife339c92b9570db2471a2fc029b3bfa114044790
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
This fixes error flagged by error prone:
Java compilation in rule '//org.eclipse.jgit.test:jgit' failed: Worker
process sent response with exit code: 1.
org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevFlagSetTest.java:149:
error: [CollectionIncompatibleType] Argument '"bob"' should not be
passed to this method; its type String is not compatible with its
collection's type argument RevFlag
assertFalse(set.contains("bob"));
Change-Id: I4a971ce92fee55e28b2ab0c7b716ac20fa9c6709
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
This fixes Bazel build:
in srcs attribute of java_library rule //org.eclipse.jgit:jgit:
file '//org.eclipse.jgit:src/org/eclipse/jgit/util/sha1/SHA1.recompress'
is misplaced here (expected .java, .srcjar or .properties).
Another option that was considered is to exclude the non source files.
Change-Id: I7083f27a4a49bf6681c85c7cf7b08a83c9a70c77
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
* stable-4.6:
Don't remove pack when FileNotFoundException is transient
Change-Id: I82941a98385cda27c89e1e6750b7b6db4e39f414
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-4.5:
Don't remove pack when FileNotFoundException is transient
Change-Id: Ic17c542d78a4cad48ff1ed77dcdc853a4ef2dc06
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
The FileNotFoundException is typically raised in three conditions:
1. file doesn't exist
2. incompatible read vs. read/write open modes
3. filesystem locking
4. temporary lack of resources (e.g. too many open files)
1. is already managed, 2. would never happen as packs are not
overwritten while with 3. and 4. it is worth logging the exception and
retrying to read the pack again.
Log transient errors using an exponential backoff strategy to avoid
flooding the logs with the same error if consecutive retries to access
the pack fail repeatedly.
Bug: 513435
Change-Id: I03c6f6891de3c343d3d517092eaa75dba282c0cd
Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
The submodule.name.fetchRecurseSubmodules value was being read from the
configuration of the submodule, but it should be read from the config
of the parent repository.
Also, the fetch.recurseSubmodules value from the parent repository's
configuration was not being considered at all.
Fix both of these and add tests. Now the precedence of the recurse mode
is determined as follows:
1. Value passed to the API
2. Value configured in submodule.name.fetchRecurseSubmodules
3. Value configured in fetch.recurseSubmodules
4. Default to "on demand"
Change-Id: Ic23b7c40b5f39135fb3fd754c597dd4bcc94240c
Breaking API for API implementors (not clients) in a minor release is ok
following OSGi semantic versioning rules.
Change-Id: I14bb4a3084b237b4cd893a130e148148cd1e5df9
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
We detect API changes during the Maven build using japicmp. Update the
reference version to the latest minor version 4.6.0.
Change-Id: I70f65f4a0680671612f21ee1afac7f7ef0152cbb
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
We updated the version we use for packaging the p2 repository in
efbecb97 but missed to update the version we compile against in the
Maven build.
Bug: 513354
Change-Id: I49afd89b43cfd8735568a86ef273723909d694a3
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This allows implementations to reject operations that do not
include proper authentication.
Change-Id: If301476d8fb56a0899e424be3789c7576097d185
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.6:
Update Jetty to 9.4.1.v20170120 in buck build
Update Jetty to 9.4.1.v20170120
Update build to use Tycho 1.0.0
Update minimum JDK version in README
Change-Id: I735697c112094e883986ce13026d967291d88494
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
5e8e2179b2 (Update Jetty to
9.4.1.v201470120, 2017-01-26) updated Jetty in the maven build.
Update the buck build to match so buck builds work again.
The buck build will go away soon, but in the meantime (until the bazel
build gets the same level of support) it is convenient as a faster way
of running tests than using maven.
The bazel build doesn't need this change since it doesn't build or run
http tests yet.
Change-Id: Ibbdaf2880e76b32fc9f6b5605a2ff29e3deffda2
(cherry picked from commit 2470f01d0f)
MappedLoginService is no longer available in Jetty 9.4 therefore base
TestLoginService on AbstractLoginService.
Apparently Jetty now uses slf4j hence adapt RecordingLogger accordingly
so we can log error messages containing slf4j style formatting anchors
"{}".
Change-Id: Ibb36aba8782882936849b6102001a88b699bb65c
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
(cherry picked from commit 5e8e2179b2)
Extend FetchCommand to expose a new method, setRecurseSubmodules(mode),
which allows to set the mode to ON, OFF or ON_DEMAND.
After fetching a repository, its submodules are recursively fetched:
- When the mode is YES, submodules are always fetched.
- When the mode is NO, submodules are not fetched.
- When the mode is ON_DEMAND, submodules are only fetched when the
parent repository receives an update of the submodule and the new
revision is not already in the submodule.
The mode is determined in the following order of precedence:
- Value specified in the API call using setRecurseSubmodules.
- Value specified in the repository's config under the key
submodule.name.fetchRecurseSubmodules
- Defaults to ON_DEMAND if neither of the previous is set.
Extend FetchResult to recursively include results for submodules, as
a map of the submodule path to an instance of FetchResult.
Test setup is based on testCloneRepositoryWithNestedSubmodules.
Change-Id: Ibc841683763307cb76e78e142e0da5b11b1add2a
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
This operation was added recently with the goal to provide some
way to auto-correct invalid user input, or to provide a correction
suggestion to the user -- EGit uses it now that way. But the initial
implementation was very restrictive; it removed all non-ASCII
characters and even slashes.
Understandably end users were not happy with that. Git has no such
restriction to ASCII-only; nor does JGit. Branch names should be
meaningful to the end user, and if a user-supplied branch name is
invalid for technical reasons, a "normalized" name should still
be meaningful to the user.
Rewrite to attempt a minimal fix such that the result will pass
isValidRefName.
* Replace all Unicode whitespace by underscore.
* Replace troublesome special characters by dash.
* Collapse sequences of underscores, dots, and dashes.
* Remove underscores, dots, and dashes following slashes, and
collapse sequences of slashes.
* Strip leading and trailing sequences of slashes, dots, dashes,
and underscores.
* Avoid the ".lock" extension.
* Avoid the Windows reserved device names.
* If input name is null return an empty String so callers don't need to
check for null.
This still allows branch names with single slashes as separators
between components, avoids some pitfalls that isValidRefName() tests
for, and leaves other character untouched and thus allows non-ASCII
branch names.
Also move the function from the bottom of the file up to where
isValidRefName is implemented.
Bug: 512508
Change-Id: Ia0576d9b2489162208c05e51c6d54e9f0c88c3a7
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Update SHA1 class to include a Java port of sha1dc[1]'s ubc_check,
which can detect the attack pattern used by the SHAttered[2] authors.
Given the shattered example files that have the same SHA-1, this
modified implementation can identify there is risk of collision given
only one file in the pair:
$ jgit ...
[main] WARN org.eclipse.jgit.util.sha1.SHA1 - SHA-1 collision 38762cf7f55934b34d179ae6a4c80cadccbb7f0a
When JGit detects probability of a collision the SHA1 class now warns
on the logger, reporting the object's SHA-1 hash, and then throws a
Sha1CollisionException to the caller.
From the paper[3] by Marc Stevens, the probability of a false positive
identification of a collision is about 14 * 2^(-160), sufficiently low
enough for any detected collision to likely be a real collision.
git-core[4] may adopt sha1dc before the system migrates to an entirely
new hash function. This commit enables JGit to remain compatible with
that move to sha1dc, and help protect users by warning if similar
attacks as SHAttered are identified.
Performance declined about 8% (detection off), now:
MessageDigest 238.41 MiB/s
MessageDigest 244.52 MiB/s
MessageDigest 244.06 MiB/s
MessageDigest 242.58 MiB/s
SHA1 216.77 MiB/s (was ~240.83 MiB/s)
SHA1 220.98 MiB/s
SHA1 221.76 MiB/s
SHA1 221.34 MiB/s
This decline in throughput is attributed to the step loop unrolling in
compress(), which was necessary to easily fit the UbcCheck logic into
the hash function. Using helper functions s1-s4 reduces the code
explosion, providing acceptable throughput.
With detection enabled (default):
SHA1 detectCollision 180.12 MiB/s
SHA1 detectCollision 181.59 MiB/s
SHA1 detectCollision 181.64 MiB/s
SHA1 detectCollision 182.24 MiB/s
sha1dc (native C) ~206.28 MiB/s
sha1dc (native C) ~204.47 MiB/s
sha1dc (native C) ~203.74 MiB/s
Average time across 100,000 calls to hash 4100 bytes (such as a commit
or tree) for the various algorithms available to JGit also shows SHA1
is slower than MessageDigest, but by an acceptable margin:
MessageDigest 17 usec
SHA1 18 usec
SHA1 detectCollision 22 usec
Time to index-pack for git.git (217982 objects, 69 MiB) has increased:
MessageDigest SHA1 w/ detectCollision
------------- -----------------------
20.12s 25.25s
19.87s 25.48s
20.04s 25.26s
avg 20.01s 25.33s +26%
Being implemented in Java with these additional safety checks is
clearly a penalty, but throughput is still acceptable given the
increased security against object name collisions.
[1] https://github.com/cr-marcstevens/sha1collisiondetection
[2] https://shattered.it/
[3] https://marc-stevens.nl/research/papers/C13-S.pdf
[4] https://public-inbox.org/git/20170223230621.43anex65ndoqbgnf@sigill.intra.peff.net/
Change-Id: I9fe4c6d8fc5e5a661af72cd3246c9e67b1b9fee6
The TreeWalk filtering classes need to support the three different
meanings of the return value the path comparison generates.
A new path comparison method (isPathMatch) is created with
three distinct return values (isPathPrefix use value '0' to
encode two of these) which will makes it possible for the logical
operators (especially NOT) to aggregate a correct verdict.
A filter like: AND(Path("path"), NOT(Path("path/to/other")))
Should filter out 'path/to/other/file', but not 'path/to/my/file'.
The path-limiting feature when testing path/to/my/file, would
result to run test for the following paths:
path
path/to
path/to/my
path/to/my/file
isPathPrefix('path/to/other') will return '0' for the first two
and since there is no way for NOT to distinguish between an exact
match and a match indicating that the tested path is a 'parent',
it will incorrectly return false and thus remove everything below
'path' immediately.
isPathMatch has a distinguished value for 'parent' matches that
will be preserved through the logic operators and should not
cause an over-eager removal of paths.
The functionality of isPathPrefix is required by other parts
and is untouched.
Unit tests are included to ensure that the logical functionality
is correct and can be preserved.
Change-Id: Ice2ca9406f09f1b179569e99b86a0e5d77baa20d
Signed-off-by: Magnus Vigerlöf <magnus.vigerlof@gmail.com>
Allow SHA1 instances to be reused to compute another hash value, and
resume caching them in ObjectInserter and PackParser. This shaves a
small amount of running time off parsing git.git's pack file:
before after
------ ------
25.25s 25.55s
25.48s 25.06s
25.26s 24.94s
Almost noise (small difference), but recycling the instances reduces
some stress on the memory allocator finding two 80 word message block
arrays needed for hashing and collision detection.
Change-Id: I4af88a720e81460293bc5c5d1d3db1a831e7e228
5e8e2179b2 (Update Jetty to
9.4.1.v201470120, 2017-01-26) updated Jetty in the maven build.
Update the buck build to match so buck builds work again.
The buck build will go away soon, but in the meantime (until the bazel
build gets the same level of support) it is convenient as a faster way
of running tests than using maven.
The bazel build doesn't need this change since it doesn't build or run
http tests yet.
Change-Id: Ibbdaf2880e76b32fc9f6b5605a2ff29e3deffda2
Generate names for objects using only the pure Java SHA1
implementation, but continue using MessageDigest in tests.
This opens the possibility of changing the hashing function
to incorporate additional safety measures, such as those
used in sha1dc[1].
Since MessageDigest has higher throughput, continue using
MessageDigest for computing pack, idx and DirCache trailers.
These are less likely to be sensitive to SHAttered[2] types
of attacks, as Git uses them to detect random bit flips
during transfer, and not for content identity.
[1] https://github.com/cr-marcstevens/sha1collisiondetection
[2] https://shattered.it/
Change-Id: If6da98334201f7f20cb916e46f782c45f373784e
This implementation is derived straight from the description written
in RFC 3174. On Mac OS X with Java 1.8.0_91 it offers similar
throughput as MessageDigest SHA-1:
system 239.75 MiB/s
system 244.71 MiB/s
system 245.00 MiB/s
system 244.92 MiB/s
sha1 234.08 MiB/s
sha1 244.50 MiB/s
sha1 242.99 MiB/s
sha1 241.73 MiB/s
This is the fastest implementation I could come up with. Common SHA-1
implementation tricks such as unrolling loops creates a method too
large for the JIT to effectively optimize, resulting in lower overall
hashing throughput. Using a preprocessor to perform the register
renaming of A-E also didn't help, as again the method was too large
for the JIT to effectively optimize.
Fortunately the fastest version is a naive, straight-forward
implementation very close to the description in RFC 3174.
Change-Id: I228b05c4a294ca2ad51386cf0e47978c68e1aa42
Since the introduction of generic type parameter inference in Java 7,
it's not necessary to explicitly specify the type of generic parameters.
Enable the warning in Eclipse, and fix all occurrences.
Change-Id: I9158caf1beca5e4980b6240ac401f3868520aad0
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
FileRepository is in the package org.eclipse.jgit.internal, and is
thus non-API. This causes warnings in Eclipse when FileRepository is
used.
Add a filter to prevent the warnings.
Change-Id: I9a8ae106c085bb0e826031fa183b4c4bdabcc5fc
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
In 0bff481d45 to accurately use the two
limits it was necessary to move the LimitedInputStream out of the
PacketLineIn and further down to the PackParser. Unfortuantely this
didn't survive review, as a buggy test failed and the "fix" was to
drop this part of the code.
The maxPackSizeLimit should apply to the pack stream, not the pkt-line
framing used to send commands to control the ReceivePack instance. The
commands are controlled using a different limit. The failing test allowed
too many bytes in the pack and was only failing because it was including
the command framing. The correct fix for the test was simply to drop the
limit lower, to more closely match the actual pack size.
Change-Id: I47d3885b9d7d527e153df7ac9c62fc2865ceecf4
RevCommit.getCommitTime returns time in seconds since the epoch.
ZipArchiveEntry.setTime expects time in milliseconds.
Add the missing unit conversion to get the correct result.
Correct formatting to be consistent with the rest of the code.
Change-Id: I990b92f1d996ec8538d4857755694d91b142eb53
Set missingOverrideAnnotation=warning in Eclipse compiler preferences
which enables the warning:
The method <method> of type <type> should be tagged with @Override
since it actually overrides a superclass method
Justification for this warning is described in:
http://stackoverflow.com/a/94411/381622
Enabling this causes in excess of 1000 warnings across the entire
code-base. They are very easy to fix automatically with Eclipse's
"Quick Fix" tool.
Fix all of them except 2 which cause compilation failure when the
project is built with mvn; add TODO comments on those for further
investigation.
Change-Id: I5772061041fd361fe93137fd8b0ad356e748a29c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
MappedLoginService is no longer available in Jetty 9.4 therefore base
TestLoginService on AbstractLoginService.
Apparently Jetty now uses slf4j hence adapt RecordingLogger accordingly
so we can log error messages containing slf4j style formatting anchors
"{}".
Change-Id: Ibb36aba8782882936849b6102001a88b699bb65c
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Add a new method setTagOpt which sets the annotated tag behavior during
fetch. Pass the option to the fetch command.
No explicit tests are added; the fetch with tags functionality is already
covered by the tests of the fetch command.
Change-Id: I131e1f68d8fcced178d8fa48abf7ffab17f8e173
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
David Ostrovsky
Jonathan Nieder
David Pursehouse
Matthias Sohn
Luca Milanesio
Andrey Loskutov
Thomas Wolf
Shawn Pearce
Magnus Vigerlöf