The private fetchSubmodules method in the FetchCommand class creates a
Repository instance for each submodule being fetched, but never calls
closes on it.
This leads to the leaking of file handles.
Bug: 526494
Change-Id: I7070388b8b62063d9d5cd31afae3015a8388044f
Signed-off-by: Tim Hosey <timhoseydev@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
The ObjectId of an unborn branch is null, skip those in UploadPack.
Change-Id: I7cbf66b05dff98c4fe9f33e20a647ba6acf364b2
Signed-off-by: Zhen Chen <czhen@google.com>
This is necessary to make sure that the FS set to e.g. the
CloneCommand will be passed on and used by the new repository
Change-Id: I9f81f65df784099b07e548b91482e7ace3f5a17e
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
Change-Id: Iaaefc2cbafbf083d6ab158b1c378ec69cc76d282
Signed-off-by: David Turner <dturner@twosigma.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
When a submodule is moved, the "name" field remains the same, while
the "path" field changes. Git uses the "name" field in .git/config
when a submodule is initialized, so this patch makes JGit do so too.
Change-Id: I48d8e89f706447b860c0162822a8e68170aae42b
Signed-off-by: David Turner <dturner@twosigma.com>
* stable-4.9:
Strings#convertGlob: fix escaping of patterns like [\[].
Change-Id: I18d55537002b3153db35f8a6b60f2f5317d17248
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Originally the patterns were escaped twice leading
to wrong matching results.
Bug: 528886
Change-Id: I26e201b4b0ef51cac08f940b76f381260fa925ca
Signed-off-by: Dmitry Pavlenko <pavlenko@tmatesoft.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* changes:
ConfigTest: Add some additional comment parsing tests
Config: Drop backslash in invalid escape sequences in subsections
Config: Match C git behavior more closely in escaping values
The intent with the setCompressionLevel and checkExisting methods (which
are already public) is for callers to be able to call them, but they
can't do that if the class itself is not public.
Change-Id: I014044fec3bfa1d33775500345efd60eb5d45bde
These are ignored by C git when parsing:
$ git config -f - --list <<EOF
[foo "x\0y"]
bar = baz
[foo "x\qy"]
bar = baz
[foo "x\by"]
bar = baz
[foo "x\ny"]
bar = baz
[foo "x\ty"]
bar = baz
EOF
foo.x0y.bar=baz
foo.xqy.bar=baz
foo.xby.bar=baz
foo.xny.bar=baz
foo.xty.bar=baz
This behavior is different from value parsing, where an invalid escape
sequence is an error (which JGit already does as well):
$ git config -f - --list <<EOF
[foo]
bar = x\qy
EOF
fatal: bad config line 2 in standard input
Change-Id: Ifd40129b37d9a62df3d886d8d7e22f766f54e9d1
* stable-4.9:
PackInserter: Ensure objects are written at the end of the pack
ObjectInserter: Add warning about mixing read-back with writes
Change-Id: I308e7c1c6b72e8d4d9b5d0f4f51e9815fc92d7d7
When interleaving reads and writes from an unflushed pack, we forgot to
reset the file pointer back to the end of the file before writing more
new objects. This had at least two unfortunate effects:
* The pack data was potentially corrupt, since we could overwrite
previous portions of the file willy-nilly.
* The CountingOutputStream would report more bytes read than the size
of the file, which stored the wrong PackedObjectInfo, which would
cause EOFs during reading.
We already had a test in PackInserterTest which was supposed to catch
bugs like this, by interleaving reads and writes. Unfortunately, it
didn't catch the bug, since as an implementation detail we always read a
full buffer's worth of data from the file when inflating during
readback. If the size of the file was less than the offset of the object
we were reading back plus one buffer (8192 bytes), we would completely
accidentally end up back in the right place in the file.
So, add another test for this case where we read back a small object
positioned before a large object. Before the fix, this test exhibited
exactly the "Unexpected EOF" error reported at crbug.com/gerrit/7668.
Change-Id: I74f08f3d5d9046781d59e5bd7c84916ff8225c3b
Change-Id: I2150889b5ed04e8739e2367fc9023b750b516398
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Change-Id: I35370c66e54d93d9b0aa3995e300706956ec0923
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Where the exception being thrown has a constructor that takes a
Throwable, use that instead of instantiating the exception and then
explicitly calling initCause.
Change-Id: I06a0df407ba751a7af8c1c4a46f9e2714f13dbe3
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
CorruptObjectException has a constructor that takes Throwable and
calls initCause with it. Use that instead of instantiating the
exception and explicitly calling initCause.
Change-Id: I1f2747d6c4cc5249e93401b9787eb4ceb50cb995
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
In 5e7eed4 a new StoredObjectRepresentationNotAvailableException
constructor was added, that takes a Throwable to initialize the
exception cause.
Update more call sites to use this constructor instead of first
instantiating it and explicitly calling initCause().
All callers now use the new constructor, so annotate the other one as
deprecated.
Change-Id: I6d2a7e289a95f0360ddebf904cfd8b6c18fef10c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
If the cause can be passed into the constructor, callers don't need to
instantiate it and then explicitly call initCause.
Note that the constructors in this class cause "non-API parameter type"
warnings because ObjectToPack is internal, however it's probably OK
since the only non-internal reference to it is in the pgm.debug package.
Change-Id: Ia4eab24e79f9afe6214ea8160137d941d4048319
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Previously, Config was using the same method for both escaping and
parsing subsection names and config values. The goal was presumably code
savings, but unfortunately, these two pieces of the git config format
are simply different.
In git v2.15.1, Documentation/config.txt says the following about
subsection names:
"Subsection names are case sensitive and can contain any characters
except newline (doublequote `"` and backslash can be included by
escaping them as `\"` and `\\`, respectively). Section headers cannot
span multiple lines. Variables may belong directly to a section or to
a given subsection."
And, later in the same documentation section, about values:
"A line that defines a value can be continued to the next line by
ending it with a `\`; the backquote and the end-of-line are stripped.
Leading whitespaces after 'name =', the remainder of the line after
the first comment character '#' or ';', and trailing whitespaces of
the line are discarded unless they are enclosed in double quotes.
Internal whitespaces within the value are retained verbatim.
Inside double quotes, double quote `"` and backslash `\` characters
must be escaped: use `\"` for `"` and `\\` for `\`.
The following escape sequences (beside `\"` and `\\`) are recognized:
`\n` for newline character (NL), `\t` for horizontal tabulation (HT,
TAB) and `\b` for backspace (BS). Other char escape sequences
(including octal escape sequences) are invalid."
The main important differences are that subsection names have a limited
set of supported escape sequences, and do not support newlines at all,
either escaped or unescaped. Arguably, it would be easy to support
escaped newlines, but C git simply does not:
$ git config -f foo.config $'foo.bar\nbaz.quux' value
error: invalid key (newline): foo.bar
baz.quux
I468106ac was an attempt to fix one bug in escapeValue, around leading
whitespace, without having to rewrite the whole escaping/parsing code.
Unfortunately, because escapeValue was used for escaping subsection
names as well, this made it possible to write invalid config files, any
time Config#toText is called with a subsection name with trailing
whitespace, like {foo }.
Rather than pile hacks on top of hacks, fix it for real by largely
rewriting the escaping and parsing code.
In addition to fixing escape sequences, fix (and write tests for) a few
more issues in the old implementation:
* Now that we can properly parse it, always emit newlines as "\n" from
escapeValue, rather than the weird (but still supported) syntax with a
non-quoted trailing literal "\n\" before the newline. In addition to
producing more readable output and matching the behavior of C git,
this makes the escaping code much simpler.
* Disallow '\0' entirely within both subsection names and values, since
due to Unix command line argument conventions it is impossible to pass
such values to "git config".
* Properly preserve intra-value whitespace when parsing, rather than
collapsing it all to a single space.
Change-Id: I304f626b9d0ad1592c4e4e449a11b136c0f8b3e3
The map returned by getAllRefs includes all refs, including symrefs like
HEAD that may not point to any object yet. That is a valid state (e.g.,
in a new repository that has just been created by "git init"), so skip
such refs.
Change-Id: Ieff8a1aa738b8d09a2990d075eb20601156b70d3
Signed-off-by: Zhen Chen <czhen@google.com>
When we are cloning we have no refs at all yet, and there cannot
(or at least should not) be any other thread doing something with
refs yet.
Locking loose refs is thus not needed, since there are no loose
refs yet and nothing should be trying to create them concurrently.
Let's skip the whole loose ref locking when we are cloning a repository.
As a result, JGit will write the refs directly to the packed-refs
file, and will not create the refs/remotes/ directories nor the
lock files underneath when cloning and packed refs are used. Since
no lock files are created, any problems on case-insensitive file
systems with tag or branch names that differ only in case are avoided
during cloning.
Detect if we are cloning based on the following heuristics:
* HEAD is a dangling symref
* There is no loose ref
* There is no packed-refs file
Note, however, that there may still be problems with such tag or
branch names later on. This is primarily a five-minutes-past-twelve
stop-gap measure to resolve the referenced bug, which affects the
Oxygen.2 release.
Bug: 528497
Change-Id: I57860c29c210568165276a123b855e462b6a107a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
The Config class must be safe to run against untrusted input files.
Reading arbitrary local system paths using include.path is risky for
servers, including Gerrit Code Review.
This was fixed on master [1] by making "readIncludedConfig" a noop
by default. This allows only FileBasedConfig, which originated from
local disk, to read local system paths.
However, the "readIncludedConfig" method was only introduced in [2]
which was needed by [3], both of which are only on the master branch.
On the stable branch only Config supports includes. Therefore this
commit simply disables the include functionality.
[1] https://git.eclipse.org/r/#/c/113371/
[2] https://git.eclipse.org/r/#/c/111847/
[3] https://git.eclipse.org/r/#/c/111848/
Bug: 528781
Change-Id: I9a3be3f1d07c4b6772bff535a2556e699a61381c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.9:
InMemoryRepository: Make inner class MemObjDatabase static
Change-Id: I62bb5957de1ae3bc6030ea2181b09efccc48252b
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
The Config class must be safe to run against untrusted input files.
Reading arbitrary local system paths using include.path is risky for
servers, including Gerrit Code Review. Return null by default to
incide the include should be ignored.
Only FileBasedConfig which originated from local disk should be trying
to read local system paths. FileBasedConfig already overrides this
method with its own implementation.
Change-Id: I2ff31753868aa1bbac4a6843a4c23e50bd6f46f3
This can be useful for sophisticated pre-read algorithms to quickly
determine if a file is likely already in cache, especially small
reftables which may be smaller than a typical DFS block size.
Change-Id: I7756948063b722ff650c9ba82060ff9ad554b0ba
* stable-4.9:
TransportCommand#setTimeout: Specify units for timeout in Javadoc
Fix typo in key of a JGitText externalized string
Change-Id: Icb60537d2e99cb6e928d9fe07f66695ed69081b5
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
FindBugs reports:
This class is an inner class, but does not use its embedded reference
to the object which created it. This reference makes the instances
of the class larger, and may keep the reference to the creator object
alive longer than necessary. If possible, the class should be made
static.
Change-Id: I9f49de32b4cd81b7ef1239b390353689263bf66e
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
If some process executed by FS#readPipe lived for a while after
closing stderr, FS#GobblerThread#run failed with an
IllegalThreadStateException exception when accessing p.exitValue()
for the process which is still alive.
Add Process#waitFor calls to wait for the process completion.
Bug: 528335
Change-Id: I87e0b6f9ad0b995dbce46ddfb877e33eaf3ae5a6
Signed-off-by: Dmitry Pavlenko <pavlenko@tmatesoft.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
File.listFiles() returns null if the given File does not represent a
directory, so we can just test for null instead of making a separate
call to FS.DETECTED.isDirectory()
This also avoids a false-positive error from SpotBugs which claims
that there is a potential null-pointer exception on dereferencing the
result of Files.listFiles().
Change-Id: I18e09e391011db997470f5a09d8e38bb604c0213
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Enclose the call to getStat in a `try`, and release the previously
acquired lock in the `finally`. This prevents that the lock is left
unreleased in the case of an exception being raised in getStat.
Change-Id: I17b4cd134dae887e23a1165253be0ac2d4fd452c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Boolean is being abused to represent three possible states of atomic
file creation support (true/enabled, false/disabled, null/undefined).
Replace this with an enum of the three explicit states.
Change-Id: I2cd7fa6422311dc427823304b082ce8da50d2fbe
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Instead of hard-coding the charset strings "US-ASCII", "UTF-8", and
"ISO-8859-1", use the corresponding constants from StandardCharsets.
UnsupportedEncodingException is not thrown when the StandardCharset
constants are used, so remove the now redundant handling.
Because the encoding names are no longer hard-coded strings, also
remove redundant $NON-NLS warning suppressions.
Also replace existing usages of the constants with static imports.
Change-Id: I0a4510d3d992db5e277f009a41434276f95bda4e
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-4.9:
LfsStore: Make inner class AppServer static
DirCacheCheckout#processEntry: Fix typo in javadoc
Change-Id: Id8e4a3c4dc741e6e0182522e72ecb4b34ae419eb
When a 401 occurs on POST and the server advertises Negotiate, we
may get an exception from GSSAPI if the client isn't configured
at all for Kerberos.
Add exception logic similar to the GET case: keep trying other
authentication mechanisms if this occurs.
Bug: 501167
Change-Id: Ic3a3368378d4b3408a35aec93e78ef425d54b3e4
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
ConfigTest#pathToString is not visible to FileBasedConfigTest when
bulding with bazel.
Move it to FileUtils rather than messing about with the bazel build
rules to make it visible.
Change-Id: Idcfd4822699dac9dc4a426088a929a9cd31bf53f
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Relative include.path are now resolved against the config's parent
directory. include.path starting with ~/ are resolved against the
user's home directory
Change-Id: I91911ef404126618b1ddd3589294824a0ad919e6
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
This was silenced before but suppression was unintentionally lost in
merge commit 6858339c1e.
This method was removed in 4.9.0 and reintroduced in 4.9.1 to avoid
breaking EMF compare versions which were built against older versions.
See: abf420302b
Change-Id: I152d58ac885e044bcab682b9423f6cc83b667989
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-4.9:
GC: Delete stale temporary packs and indexes
Change-Id: I49b37845ee8a465404b801a2d8de0205a2e7ba30
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Matthias Sohn