All that's really required to run a merge operation is a single
ObjectInserter, from which we can construct a RevWalk, plus a Config
that declares a diff algorithm. Provide some factory methods that don't
take Repository.
Change-Id: Ib884dce2528424b5bcbbbbfc043baec1886b9bbd
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>
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>
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
Place a configurable upper bound on the amount of command data
received from clients during `git push`. The limit is applied to the
encoded wire protocol format, not the JGit in-memory representation.
This allows clients to flexibly use the limit; shorter reference names
allow for more commands, longer reference names permit fewer commands
per batch.
Based on data gathered from many repositories at $DAY_JOB, the average
reference name is well under 200 bytes when encoded in UTF-8 (the wire
encoding). The new 3 MiB default receive.maxCommandBytes allows about
11,155 references in a single `git push` invocation. A Gerrit Code
Review system with six-digit change numbers could still encode 29,399
references in the 3 MiB maxCommandBytes limit.
Change-Id: I84317d396d25ab1b46820e43ae2b73943646032c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Logging the repository name makes it easier to track down what is
incorrectly closing a repository.
Change-Id: I42a8bdf766c0e67f100adbf76d9616584e367ac2
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
ObjectDirectory.getShallowCommits should throw an IOException
instead of an InvalidArgumentException if invalid SHAs are present
in .git/shallow (as this file is usually edited by a human).
Change-Id: Ia3a39d38f7aec4282109c7698438f0795fbec905
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
MonotonicClock can be implemented to provide more certainity about
time than the standard System.currentTimeMillis() can provide. This
can be used by classes such as PersonIdent and Ketch to rely on
more certainity about time moving in a strictly ascending order.
Gerrit Code Review can also leverage this interface through its
embedding of JGit and use MonotonicClock and ProposedTimestamp to
provide stronger assurance that NoteDb time is moving forward.
Change-Id: I1a3cbd49a39b150a0d49b36d572da113ca83a786
This method pair allows the caller to read and modify the description
file that is traditionally used by gitweb and cgit when rendering a
repository on the web.
Gerrit Code Review has offered this feature for years as part of
its GitRepositoryManager interface, but its fundamentally a feature
of JGit and its Repository abstraction.
git-core typically initializes a repository with a default value
inside the description file. During getDescription() this string
is converted to null as it is never a useful description.
Change-Id: I0a457026c74e9c73ea27e6f070d5fbaca3439be5
Work around issues with JSch not handling interrupts by
isolating the JSch interactions onto another thread.
Run write and flush on a single threaded Executor using
simple Callable operations wrapping the method calls,
waiting on the future to determine the outcome before
allowing the caller to continue.
If any operation was interrupted the state of the stream
becomes fuzzy at close time. The implementation tries to
interrupt the pending write or flush, but this is very
likely to corrupt the stream object, so exceptions are
ignored during such a dirty close.
Change-Id: I42e3ba3d8c35a2e40aad340580037ebefbb99b53
In case a value is used which isn’t a power of 2 there will be a high
chance of java.lang.ArrayIndexOutBoundsException and
org.eclipse.jgit.errors.CorruptObjectException due to a mismatching
assumption for the DfsBlockCache#blockSizeShift parameter.
Change-Id: Ib348b3704edf10b5f93a3ffab4fa6f09cbbae231
Signed-off-by: Philipp Marx <smigfu@googlemail.com>
This SIOOBE happens reproducibly when trying to access
a repository containing Cygwin symlinks
Change-Id: I25f103fcc723bac7bfaaeee333a86f11627a92c7
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Git barfs on these (and they don't make any sense), so we certainly
shouldn't write them.
Change-Id: I3faf8554a05f0fd147be2e63fbe55987d3f88099
Signed-off-by: David Turner <dturner@twosigma.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
JGit supports smudge filters defined in repository configuration. The
filters are implemented as external programs filtering content by
accepting the original content (as seen in git's object database) on
stdin and which emit the filtered content on stdout. This content is
then written to the file in the working tree. To run such a filter JGit
has to start an external process and pump data into/from this process.
This commit adds support for built-in smudge filters which are
implemented in Java and which are executed by jgit's main thread. When a
filter is defined in the configuration as
"jgit://builtin/<filterDriverName>/smudge" then JGit will lookup in a
static map whether a builtin filter is registered under this name. If
found such a filter is called to do the filtering.
The functionality in this commit requires that a program using JGit
explicitly calls the JGit API to register built-in implementations for
specific smudge filters. In follow-up commits configuration parameters
will be added which trigger such registrations.
Change-Id: Ia743aa0dbed795e71e5792f35ae55660e0eb3c24
Change-Id: I6691b454404dd4db3c690ecfc7515de765bc2ef7
Signed-off-by: Martin Goellnitz <m.goellnitz@outlook.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
- enhance FS.readPipe to throw an exception if the external command
fails to enable the caller to handle the command failure
- reduce log level to warning if system git config does not exist
- improve log message
Bug: 476639
Change-Id: I94ae3caec22150dde81f1ea8e1e665df55290d42
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
cgit changed the --depth parameter to mean the total depth of history
rather than the depth of ancestors to be returned [1]. JGit still uses
the latter meaning, so update it to match cgit.
depth=0 still means a non-shallow clone. depth=1 now means only the
wants rather than the wants and their direct parents.
This is accomplished by changing the semantic meaning of "depth" in
UploadPack and PackWriter to mean the total depth of history desired,
while keeping "depth" in DepthWalk.{RevWalk,ObjectWalk} to mean
the depth of traversal. Thus UploadPack and PackWriter always
initialize their DepthWalks with "depth-1".
[1] upload-pack: fix off-by-one depth calculation in shallow clone
https://code.googlesource.com/git/+/682c7d2f1a2d1a5443777237450505738af2ff1a
Change-Id: I87ed3c0f56c37e3491e367a41f5e555c4207ff44
Signed-off-by: Terry Parker <tparker@google.com>
When fetching from a shallow clone, the client sends "have" lines
to tell the server about objects it already has and "shallow" lines
to tell where its local history terminates. In some circumstances,
the server fails to honor the shallow lines and fails to return
objects that the client needs.
UploadPack passes the "have" lines to PackWriter so PackWriter can
omit them from the generated pack. UploadPack processes "shallow"
lines by calling RevWalk.assumeShallow() with the set of shallow
commits. RevWalk creates and caches RevCommits for these shallow
commits, clearing out their parents. That way, walks correctly
terminate at the shallow commits instead of assuming the client has
history going back behind them. UploadPack converts its RevWalk to an
ObjectWalk, maintaining the cached RevCommits, and passes it to
PackWriter.
Unfortunately, to support shallow fetches the PackWriter does the
following:
if (shallowPack && !(walk instanceof DepthWalk.ObjectWalk))
walk = new DepthWalk.ObjectWalk(reader, depth);
That is, when the client sends a "deepen" line (fetch --depth=<n>)
and the caller has not passed in a DepthWalk.ObjectWalk, PackWriter
throws away the RevWalk that was passed in and makes a new one. The
cleared parent lists prepared by RevWalk.assumeShallow() are lost.
Fortunately UploadPack intends to pass in a DepthWalk.ObjectWalk.
It tries to create it by calling toObjectWalkWithSameObjects() on
a DepthWalk.RevWalk. But it doesn't work: because DepthWalk.RevWalk
does not override the standard RevWalk#toObjectWalkWithSameObjects
implementation, the result is a plain ObjectWalk instead of an
instance of DepthWalk.ObjectWalk.
The result is that the "shallow" information is thrown away and
objects reachable from the shallow commits can be omitted from the
pack sent when fetching with --depth from a shallow clone.
Multiple factors collude to limit the circumstances under which this
bug can be observed:
1. Commits with depth != 0 don't enter DepthGenerator's pending queue.
That means a "have" cannot have any effect on DepthGenerator unless
it is also a "want".
2. DepthGenerator#next() doesn't call carryFlagsImpl(), so the
uninteresting flag is not propagated to ancestors there even if a
"have" is also a "want".
3. JGit treats a depth of 1 as "1 past the wants".
Because of (2), the only place the UNINTERESTING flag can leak to a
shallow commit's parents is in the carryFlags() call from
markUninteresting(). carryFlags() only traverses commits that have
already been parsed: commits yet to be parsed are supposed to inherit
correct flags from their parent in PendingGenerator#next (which
doesn't happen here --- that is (2)). So the list of commits that have
already been parsed becomes relevant.
When we hit the markUninteresting() call, all "want"s, "have"s, and
commits to be unshallowed have been parsed. carryFlags() only
affects the parsed commits. If the "want" is a direct parent of a
"have", then it carryFlags() marks it as uninteresting. If the "have"
was also a "shallow", then its parent pointer should have been null
and the "want" shouldn't have been marked, so we see the bug. If the
"want" is a more distant ancestor then (2) keeps the uninteresting
state from propagating to the "want" and we don't see the bug. If the
"shallow" is not also a "have" then the shallow commit isn't parsed
so (2) keeps the uninteresting state from propagating to the "want
so we don't see the bug.
Here is a reproduction case (time flowing left to right, arrows
pointing to parents). "C" must be a commit that the client
reports as a "have" during negotiation. That can only happen if the
server reports it as an existing branch or tag in the first round of
negotiation:
A <-- B <-- C <-- D
First do
git clone --depth 1 <repo>
which yields D as a "have" and C as a "shallow" commit. Then try
git fetch --depth 1 <repo> B:refs/heads/B
Negotiation sets up: have D, shallow C, have C, want B.
But due to this bug B is marked as uninteresting and is not sent.
Change-Id: I6e14b57b2f85e52d28cdcf356df647870f475440
Signed-off-by: Terry Parker <tparker@google.com>
Gerrit's superproject subscription feature uses RefSpecs to formalize
the ACLs of when the superproject subscription feature is allowed.
As this is a slightly different use case than describing a local/remote
pair of refs, we need to be more permissive. Specifically we want to allow:
refs/heads/*
refs/heads/*:refs/heads/master
refs/heads/master:refs/heads/*
Introduce a new constructor, that allows constructing these RefSpecs.
Change-Id: I46c0bea9d876e61eb2c8d50f404b905792bc72b3
Signed-off-by: Stefan Beller <sbeller@google.com>
Example usage:
$ ./jgit push \
--push-option "Reviewer=j.doe@example.org" \
--push-option "<arbitrary string>" \
origin HEAD:refs/for/master
Stefan Beller has also made an equivalent change to CGit:
http://thread.gmane.org/gmane.comp.version-control.git/299872
Change-Id: I6797e50681054dce3bd179e80b731aef5e200d77
Signed-off-by: Dan Wang <dwwang@google.com>
We observe in Gerrit 2.12 that useCnt can become negative in rare cases.
Log this to help finding the bug.
Change-Id: Ie91c7f9d190a5d7cf4733d4bf84124d119ca20f7
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
1f86350 added initial support for include.path. Relative path and path
with tilde are not yet supported but config load was failing if one of
those 2 unsupported options was encountered. Another problem was that
config load was failing if the include.path file did not exist.
Change the behavior to be consistent with native git. Ignore unsupported
or nonexistent include.path.
Bug: 495505
Bug: 496732
Change-Id: I7285d0e7abb6389ba6983e9c46021bea4344af68
Signed-off-by: Hugo Arès <hugo.ares@ericsson.com>
File.listFiles() returns null if the File is not a directory, improve
validation of directory and gitDir to fix this.
Change-Id: I763d08835faf96a0beb8e706992df0908526bd2c
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
As per [1], but limited to absolute paths indeed. No support yet for
tilde or $HOME expansion. Support for the --[no-]includes options
([1]) is not part of this commit scope either, but those options'
defaults are in effect as described in [1].
[1] https://git-scm.com/docs/git-config
Included path can be a config file that includes other path-s in turn.
An exception is thrown if too many recursions (circular includes)
happen because of ill-specified config files.
Change-Id: I700bd7b7e1625eb7de0180f220c707d8e7b0930b
Signed-off-by: Marco Miller <marco.miller@ericsson.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Repurpose RefDatabase#performsAtomicTransactions() slightly, to
indicate that the backend _supports_ atomic transactions, rather than
the current definition, which is that the backend always _uses_ atomic
transactions regardless of whether or not the caller actually wants
them. Allow BatchRefUpdate callers to turn off atomic transactions by
calling setAtomic(false). Defaulting to true means this is backwards
compatible.
Change-Id: I6df78d7df65ab147b4cce7764bd3101db985491c
Experimental flag to turn on the KetchLeader within this daemon JVM.
This is a manually elected leader process, set from the command line.
Remote followers for each repository are configured per-repository
using remote sections with ketch-type = FULL. For example:
Manually elected leader's $GIT_DIR/config:
[ketch]
name = A
[remote "A"]
ketch-type = FULL
[remote "B"]
url = git://127.0.0.1:9421/sample.git
ketch-type = FULL
[remote "C"]
url = git://127.0.0.1:9422/sample.git
ketch-type = FULL
Replica B and C daemons:
git daemon \
--export-all \
--enable=receive-pack \
--listen=127.0.0.1 --port=9421 \
--base-path=$HOME/ketch_test/follower_one \
$HOME/ketch_test/follower_one &
git daemon \
--export-all \
--enable=receive-pack \
--listen=127.0.0.1 --port=9422 \
--base-path=$HOME/ketch_test/follower_two \
$HOME/ketch_test/follower_two &
Change-Id: I165f85970a77e16b5263115290d685d8a00566f5
Git Ketch is a multi-master Git repository management system. Writes
are successful only if a majority of participant servers agree. Acked
writes are durable against server failures as a majority of the
participants store all required objects.
Git Ketch is modeled on the Raft Consensus Algorithm[1]. A ketch
sailing vessel is faster and more nimble than a raft. It can also
carry more source codes.
Git Ketch front-loads replication costs, which vaguely resembles a
ketch sailing vessel's distinguishing feature of the main mast on the
front of the ship.
[1] https://raft.github.io/
Change-Id: Ib378dab068961fc7de624cd96030266660b64fb4
Accept some of the same section keys that fsck does in git-core,
allowing repositories to skip over specific kinds of acceptable
broken objects, e.g.:
[fsck]
duplicateEntries = ignore
zeroPaddedFilemode = ignore
The zeroPaddedFilemode = ignore is a synonym for the JGit specific
allowLeadingZeroFileMode = true. Only accept the JGit key if git-core
key was not specified.
Change-Id: Idaed9310e2a5ce5511670ead1aaea2b30aac903c
This change fixes all compiler errors in JGit and replaces possible
NPE's with either appropriate exceptions, avoiding multiple "Nullable
return" method calls or early returning from the method.
Change-Id: I24c8a600ec962d61d5f40abf73eac4203e115240
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
This should mirror the behavior of `git push --atomic` where the
client asks the server to apply all-or-nothing. Some JGit servers
already support this based on a custom DFS backend. InMemoryRepository
is extended to support atomic push for unit testing purposes.
Local disk server side support inside of JGit is a more complex animal
due to the excessive amount of file locking required to protect every
reference as a loose reference.
Change-Id: I15083fbe48447678e034afeffb4639572a32f50c
When filters are defined for certain paths in gitattributes make
sure that clean filters are processed when adding new content to the
object database.
Change-Id: Iffd72914cec5b434ba4d0de232e285b7492db868
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
org.eclipse.jgit.lib.Repository class is an example of the API which
should be written with Java 8 java.util.Optional<T> type. Unfortunately
this API is already released and widely used. The good clients are
currently doing their best with checking return values for null and bad
clients do not know how bad their code is.
I've tried not to change any logic and to be as less intrusive as
possible. Most of the JGit code was well prepared to this, only few
classes needed some smaller fixes.
This change fixes all compiler errors in JGit and replaces possible
NPE's with either appropriate exceptions, avoiding multiple "Nullable
return" method calls or early returning from the method.
Because annotating getDirectory() and getFS() as Nullable would cause
lot of additional changes in JGit and EGit they are postponed.
Change-Id: Ie8369d2c9c5fac5ce83b3b1b9bc217d7b55502a3
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
File, FileInputStream and friends may throw FileNotFoundException even
if the file is existing e.g. when file permissions don't allow to access
the file content. In most cases this is a severe error we should not
suppress hence rethrow the FileNotFoundException in this case.
This may also fix bug 451508.
Bug: 451508
Change-Id: If4a94217fb5b7cfd4c04d881902f3e86193c7008
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
If the index file exists but can't be read for example because of wrong
filesystem permissions we should throw a specific exception. This allows
EGit to handle this error situation.
Bug: 482607
Change-Id: I50bfcb719c45caac3cb5550a8b16307c2ea9def4
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
See previous attempt: https://git.eclipse.org/r/#/c/16674/
Here we preserve as much of JetS3t mode as possible
while allowing to use new Java 8+ PBE algorithms
such as PBEWithHmacSHA512AndAES_256
Summary of changes:
* change pom.xml to control long tests
* add WalkEncryptionTest.launch to run long tests
* add AmazonS3.Keys to to normalize use of constants
* change WalkEncryption to support AES in JetS3t mode
* add WalkEncryptionTest to test remote encryption pipeline
* add support for CI configuration for live Amazon S3 testing
* add log4j based logging for tests in both Eclipse and Maven build
To test locally, check out the review branch, then:
* create amazon test configuration file
* located your home dir: ${user.home}
* named jgit-s3-config.properties
* file format follows AmazonS3 connection settings file:
accesskey = your-amazon-access-key
secretkey = your-amazon-secret-key
test.bucket = your-bucket-for-testing
* finally:
* run in Eclipse: WalkEncryptionTest.launch
* or
* run in Shell: mvn test --define test=WalkEncryptionTest
Change-Id: I6f455fd9fb4eac261ca73d0bec6a4e7dae9f2e91
Signed-off-by: Andrei Pozolotin <andrei.pozolotin@gmail.com>
Since 4.0 we require Java 7 so there is no longer a need to override the
following methods in FS_POSIX, FS_Win32, FS_Win32_Cygwin
- lastModified()
- setLastModified()
- length()
- isSymlink()
- exists()
- isDirectory()
- isFile()
- isHidden()
Hence implement these methods in FS and remove overrides in subclasses.
Change-Id: I5dbde6ec806c66c86ac542978918361461021294
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
If a client mistakenly tries to send a tag object as a shallow line
JGit blindly assumes this is a commit and tries to parse the tag
buffer using the commit parser. This can cause an obtuse error like:
InvalidObjectIdException: Invalid id: t c0ff331234...
The "t" comes from the "object c0ff331234..." line of the tag tring
to be parsed as though it where the "tree" line of a commit.
Run any client supplied shallow lines through the RevWalk to lookup
the object types. Fail fast with a protocol exception if any of them
are non-commit.
Skip objects not known to this repository. This matches behavior
with git-core's upload-pack, which sliently skips over any shallow
line object named by the client but not known by the server.
Change-Id: Ic6c57a90a42813164ce65c2244705fc42e84d700
On a local filesystem the packed-refs file will be orphaned if it is
replaced by another client while the current client is reading the old
one. However, since NFS servers do not keep track of open files, instead
of orphaning the old packed-refs file, such a replacement will cause the
old file to be garbage collected instead. A stale file handle exception
will be raised on NFS servers 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 reopen the new replacement file). So retrying the read is a
viable strategy to deal with stale file handles on the packed-refs file,
implement such a strategy.
Since it is possible that the packed-refs file could be replaced again
while rereading it (multiple consecutive updates can easily occur with
ref deletions), loop on stale file handle exceptions, up to 5 extra
times, trying to read the packed-refs 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: I085c472bafa6e2f32f610a33ddc8368bb4ab1814
Signed-off-by: Martin Fick<mfick@codeaurora.org>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Catch unexpected PatternSyntaxException and convert it to
InvalidPatternException. Log such errors, do not silently ignore them.
Bug: 463581
Change-Id: Id0936d9816769ec0cfae1898beda0f7a3c146e67
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Inspired by a proposal from gitolite[1], where we store a file in
a tree for each ref name, and the contents of the file is the latest
push cert to affect that ref.
The main modification from that proposal (other than lacking the
out-of-git batching) is to append "@{cert}" to filenames, which allows
storing certificates for both refs/foo and refs/foo/bar. Those
refnames cannot coexist at the same time in a repository, but we do
not want to discard the push certificate responsible for deleting the
ref, which we would have to do if refs/foo in the push cert tree
changed from a tree to a blob.
The "@{cert}" syntax is at least somewhat consistent with
gitrevisions(7) wherein @{...} describe operators on ref names.
As we cannot (currently) atomically update the push cert ref with the
refs that were updated, this operation is inherently racy. Kick the can
down the road by pushing this burden on callers.
[1] cf062b8bb6/contrib/hooks/repo-specific/save-push-signatures
Change-Id: Id3eb32416f969fba4b5e4d9c4b47053c564b0ccd
In most texts we use "cannot" hence instead of escaping the apostroph in
"can't" use "cannot".
Bug: 471796
Change-Id: Icda5b4db38076789d06498428909306aef3cb68b
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
The invalidId message in JGitText and the asAscii bad id both contain a
colon, so the resulting message would say
Invalid id: : a78987c98798ufa
Fix it by keeping the colon in the translated message and not adding
another colon programmatically.
Noticed by code inspection.
Change-Id: I13972eebde27a4128828e6c64517666f0ba6288b
Signed-off-by: Jonathan Nieder <jrn@google.com>
- Consistently return structured data, such as actual ReceiveCommands,
which is more useful for callers that are doing things other than
verifying the signature, e.g. recording the set of commands.
- Store the certificate version field, as this is required to be part
of the signed payload.
- Add a toText() method to recreate the actual payload for signature
verification. This requires keeping track of the un-chomped command
strings from the original protocol stream.
- Separate the parser from the certificate itself, so the actual
PushCertificate object can be immutable. Make a fair attempt at deep
immutability, but this is not possible with the current mutable
ReceiveCommand structure.
- Use more detailed error messages that don't involve NON-NLS strings.
- Document null return values more thoroughly. Instead of having the
undocumented behavior of throwing NPE from certain methods if they
are not first guarded by enabled(), eliminate enabled() and return
null from those methods.
- Add tests for parsing a push cert from a section of pkt-line stream
using a real live stream captured with Wireshark (which, it should
be noted, uncovered several simply incorrect statements in C git's
Documentation/technical/pack-protocol.txt).
This is a slightly breaking API change to classes that were
technically public and technically released in 4.0. However, it is
highly unlikely that people were actually depending on public
behavior, since there were no public methods to create
PushCertificates with anything other than null field values, or a
PushCertificateParser that did anything other than infinite loop or
throw exceptions when reading.
Change-Id: I5382193347a8eb1811032d9b32af9651871372d0
Dave Borowitz
Matthias Sohn
David Ostrovsky
Luca Milanesio
Shawn Pearce
David Pursehouse
Marc Strapetz
Philipp Marx
David Turner
Christian Halstrick
Martin Goellnitz
Terry Parker
Stefan Beller
Dan Wang
Hugo Arès
Michael Keppler
Marco Miller
Andrey Loskutov
Andrei Pozolotin
Martin Fick
Jonathan Nieder