ProtocolV2Parser has unit tests but protocol v0/v1 is not covered.
Change-Id: I96022e8f8eb60d4da748d1042474fd1efd67e882
Signed-off-by: Ivan Frade <ifrade@google.com>
Parsed requests represent object ids (SHA1) in ObjectId instances but tests
use strings for those ids because they are easier to define.
Create a custom matcher that hides the conversion from string to
ObjectId. Note that this reverses the existing code conversion (it was
transforming ObjectIds into string).
This produces more readable code, consistent with the other hamcrest
assertions.
Change-Id: I47ba1d25557d791fe74fb93c740ff7de9923cc00
Signed-off-by: Ivan Frade <ifrade@google.com>
This allows clients to use the --shallow-exclude parameter (producing a
"deepen-not <ref>" line when communicating with the server) in their fetch
commands when fetching against a JGit server using protocol v2.
Note that the implementation in this commit is somewhat inefficient, as
described in the TODO comment in DepthGenerator.
Change-Id: I9fad3ed9276b624d8f668356ffd99a067dc67ef7
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
In C Git, when a client fetches with "git fetch --shallow-since=<date>
origin <ref>", and all commits reachable from <ref> are older than
<date>, the server dies with a message "no commits selected for shallow
requests". That is, (1) the --shallow-since filter applies to the commit
pointed to by the ref itself, and (2) there is a check that at least one
commit is not filtered out. (The pack-protocol.txt documentation does
not describe this, but the C implementation does this.)
The implementation in commit 1bb430dc21 ("UploadPack: support
deepen-since in protocol v2", 2018-09-27) does neither (1) nor (2), so
do both of these.
Change-Id: I9946327a71627626ecce34ca2d017d2add8867fc
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
The request receives a list of capabilities and takes out the "agent" to
offer it on its own setter (getAgent).
Do this at parse time: when reading the line if the capability is
"agent" set it directly in the builder.
This makes the treatment of "agent" consistent in v0/v1 and v2.
Change-Id: Ie4f9f2cad8639adeeaef4921df49a30a8ce5b42f
Signed-off-by: Ivan Frade <ifrade@google.com>
All of the input lines passed to pre-push hook scripts must be properly
terminated by '\n', so that normal shell scripts like the git-supplied
pre-push.sample work properly, even when pushing just a single branch.
With the old code, hook scripts that use the following pattern didn't
process the last line, because 'read' has a non-zero exit status when
EOF is encountered:
while read local_ref local_sha remote_ref remote_sha; do ... done
Change-Id: Id899662ed3fedef6c314fc4b2ddf91a6dcb98cbb
Signed-off-by: Markus Keller <markus.kell.r@gmail.com>
Before this commit, a force checkout would fail if there
were any conflicting files. After this commit, a force
checkout will overwrite the conflicting files, as expected.
Making this work required fixing a bug in DirCacheCheckout.
Before this commit, when DirCacheCheckout had
failOnConflict=false, it would delete all conflicting files
from the working copy and just leave them missing. After
this commit, DirCacheCheckout overwrites conflicting files
with the merge tree.
This change in DirCacheCheckout causes "reset --hard" and
"revert --abort" to behave as expected (previously they
would simply delete conflicting files, now they will be
overwritten from the merge tree).
Change-Id: If7e328ee792ef6511ab7d9c26d8d77c39210ec9f
Signed-off-by: Ned Twigg <ned.twigg@diffplug.com>
UploadPack.getPeerUserAgent() doesn't produce the expected results for
protocol v2 requests. In v2, the agent reported in the request (in an
"agent=" line) is not in the clientCapabilities but in a field on its
own. This makes getPeerUserAgent default to the transport user agent.
Making "agent" a shared property between protocol v0/v1 and v2 fixes the
problem, simplifies the function and harmonizes the implementation
between protocol versions.
In a follow up commit the "agent" will be identified on parsing time,
instead of taking it from the client capabilities.
Change-Id: Idf9825ec4e0b81a1458c8e3701f3e28aafd8a32a
Signed-off-by: Ivan Frade <ifrade@google.com>
In protocol v2, a command request can be followed by server options
(lines like "agent=<>" and "server-option=<>"), but current code
doesn't accept those lines.
Advertise the "server-option" capability, parse the lines and add
them to the request objects.
Other code in JGit can see this options and act accordingly via the
protocol v2 hooks.
This should not require any change in the client side.
Change-Id: If3946390f9cc02d29644b6ca52534b6f757bda9f
Signed-off-by: Ivan Frade <ifrade@google.com>
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>
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>
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>
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>
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>
The main concern are submodule urls starting with '-' that could pass as
options to an unguarded tool.
Pass through the parser the ids of blobs identified as .gitmodules
files in the ObjectChecker. Load the blobs and parse/validate them
in SubmoduleValidator.
Change-Id: Ia0cc32ce020d288f995bf7bc68041fda36be1963
Signed-off-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
In order to validate .gitmodules files, we first need to find them
in the incoming pack.
Do it in the ObjectChecker stage. Check in the tree objects if they
point to a .gitmodules file and report the tree id and the .gitmodules
blob id.
This can be used later to check if the file is in the root of the
project and if the contents are good.
While we're here, make isMacHFSGit more accurate by detecting variants
of filenames that vary in case.
[jn: tweaked NTFS and HFS+ checking; added more tests]
Change-Id: I70802e7d2c1374116149de4f89836b9498f39582
Signed-off-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
In C git versions before 2.19.1, the submodule is fetched by running
"git clone <uri> <path>". A URI starting with "-" would be interpreted
as an option, causing security problems. See CVE-2018-17456.
Refuse to add submodules with URIs, names or paths starting with "-",
that could be confused with command line arguments.
[jn: backported to JGit 4.7.y, bringing portions of Masaya Suzuki's
dotdot check code in v5.1.0.201808281540-m3~57 (Add API to specify
the submodule name, 2018-07-12) along for the ride]
Change-Id: I2607c3acc480b75ab2b13386fe2cac435839f017
Signed-off-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
EolStreamTypeUtil didn't handle these correctly on Windows.
Add three new tests to verify that the crlf attribute is handled as
described at [1], and that core.eol=native produces the expected
line endings on check-out.
[1] https://git-scm.com/docs/gitattributes
Bug: 497290
Change-Id: Idd9b435e3256c1e3251cc7b966f2f0460e787f07
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This probably doesn't provide any benefit for the tests, but gets
rid of a warning from Error Prone.
See https://errorprone.info/bugpattern/InputStreamSlowMultibyteRead
Change-Id: I584d2e0d18475fad38747b688af6301c423f54a7
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Deprecate constructors and methods taking a character set name as
a String, in favor of new variants taking a Charset.
Change-Id: I616c601daf232fa17610dba1087fd902030d46ea
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Error Prone reports:
Not calling fail() when expecting an exception masks bugs
See https://errorprone.info/bugpattern/MissingFail
Change-Id: I58ad45a87dcf7d646dce056d20776d62faafbfef
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Error Prone reports:
Not calling fail() when expecting an exception masks bugs
See https://errorprone.info/bugpattern/MissingFail
Change-Id: I518b524de7cd3802f03b80450cad02ab3f79d57b
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Error Prone reports:
Not calling fail() when expecting an exception masks bugs
See https://errorprone.info/bugpattern/MissingFail
Change-Id: Ic89f9daef3c31bfbfd1f3c003cb90a373cd74847
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Error Prone reports:
Not calling fail() when expecting an exception masks bugs
See https://errorprone.info/bugpattern/MissingFail
Change-Id: I463510342bb6e6b99b31a0fe264d953340784393
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
The tests were set up to expect an IllegalArgumentException when
the Config.getInt method was called with a section.key that has
not been set, or explicitly set to an empty string.
However, the IllegalArgumentException never gets thrown because
the getInt method returns the provided default ("1"), and because
there was no call to "fail" after getInt, the incorrect behavior
of the test was not noticed.
Remove the try/catch around getInt, and instead assert that the
expected default value is returned.
Found by Error Prone, which reported:
Not calling fail() when expecting an exception masks bugs
See https://errorprone.info/bugpattern/MissingFail
Change-Id: Ie8e692aba9fb8523241fb8f298d57493923d9f78
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Support the deepen-since parameter when requested by a client using
protocol v2. This is done by:
- adding a DepthWalk.RevWalk#setDeepenSince method
- updating DepthGenerator to recognize when deepen-since is set
- recording in DepthWalk.Commit whether a commit is a boundary commit
Existing users of DepthWalk such as UploadPack previously recognized
boundary commits by comparing their depths against the threshold, not
tracking whether any parents were truly excluded. This behavior is
preserved - UploadPack considers a commit as boundary if its depth is
equal to the threshold *or* a parent was excluded (whether by depth or
by deepen-since).
Change-Id: I852bba6b1279f9cc8aee38282e9339d62b8dcddc
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
The BufferedWriter is opened in a try-with-resource and thus will be
automatically closed.
Presumably the close was added to make sure it is closed before the
subsequent test statements are executed. Instead of explicitly closing
it, let the try-with-resource automatically close it, and move the
subsequent statements out of the try-block.
Change-Id: If5fada2f580ef9cbaad3a0b9216b5200b917781a
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Replace explicit construction of FileWriter with calls to the
utility method Files.newBufferedWriter, which allows to specify
the charset.
Also convert to try-with-resource.
Change-Id: I9fa3f612b9b2fc5ac12cd79d6e61ca181120dbf5
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Ivan Frade
Jonathan Tan
Markus Keller
Ned Twigg
Matthias Sohn
Michael Keppler
David Pursehouse
Thomas Wolf