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>
David Pursehouse [Mon, 18 Dec 2017 05:34:13 +0000 (14:34 +0900)]
StoredObjectRepresentationNotAvailableException: Add constructor that takes cause
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>
Dave Borowitz [Mon, 18 Dec 2017 20:35:02 +0000 (15:35 -0500)]
Config: Rewrite subsection and value escaping and parsing
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:
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: I3b0fad9c45cdf3a918ddb8ea4a37b8d3403aae90 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Zhen Chen [Fri, 15 Dec 2017 20:58:49 +0000 (12:58 -0800)]
DfsFsck: Skip unborn branches and symrefs to nowhere
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.
Thomas Wolf [Thu, 14 Dec 2017 12:24:00 +0000 (13:24 +0100)]
Write packed-refs directly when cloning
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>
David Pursehouse [Thu, 14 Dec 2017 23:50:28 +0000 (08:50 +0900)]
Config: Remove the include functionality
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.
Shawn Pearce [Thu, 14 Dec 2017 01:35:38 +0000 (17:35 -0800)]
Make Config.readIncludedConfig a noop by default
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.
Shawn Pearce [Thu, 14 Dec 2017 01:10:51 +0000 (17:10 -0800)]
DfsBlockCache.hasBlock0: quickly check for file in cache
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.
InMemoryRepository: Make inner class MemObjDatabase static
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>
Dmitry Pavlenko [Thu, 7 Dec 2017 18:36:35 +0000 (19:36 +0100)]
Fix IllegalThreadStateException if stderr closed without exiting
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.
CloneCommand: Refactor listing of files in deleteChildren
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>
* stable-4.9:
LfsServerText: Fix formatting of invalidPathInfo message
InvalidLongObjectIdException: Fix formatting of message
FileLfsServlet: Do not pretty print json response
Change-Id: Ib59431865a45659fe9d91e2556b9d1ce49e3808a Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Pretty printing the response is useful for human readers, but most
(if not all) of the time, the response will be read by programs.
Remove it to avoid the additional overhead of the formatting and
extra bytes in the response. Adjust the test accordingly.
Note that LfsProtocolServlet already doesn't use pretty printing,
so this change makes FileLfsServlet's behavior consistent. In fact,
both classes now have duplicate Gson handling; this will be cleaned
up in a separate change.
Change-Id: I113a23403f9222f16e2c0ddf39461398b721d064 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
DfsBlockCache#creditSpace: release clockLock in finally block
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>
Pretty printing the response is useful for human readers, but most
(if not all) of the time, the response will be read by programs.
Remove it to avoid the additional overhead of the formatting and
extra bytes in the response. Adjust the test accordingly.
Note that LfsProtocolServlet already doesn't use pretty printing,
so this change makes FileLfsServlet's behavior consistent. In fact,
both classes now have duplicate Gson handling; this will be cleaned
up in a separate change.
Change-Id: I113a23403f9222f16e2c0ddf39461398b721d064 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
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: I245e44678166176de0cfb275e22ddd159f88e0bd Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Marc Strapetz [Sat, 18 Nov 2017 16:50:30 +0000 (17:50 +0100)]
FileBasedConfig: support for relative includes
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>
Marc Strapetz [Mon, 20 Nov 2017 17:21:22 +0000 (18:21 +0100)]
ConfigTest: fix on Windows
Change-Id: I37a2ef611aef97faf1b891a9660c1745435a915d Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
projects / jgit.git / log