As reported by Sonar Lint:

Array designators should always be located on the type for better code
readability. Otherwise, developers must look both at the type and the
variable name to know whether or not a variable is an array.

Change-Id: If6b41fed3483d0992d402d8680552ab4bef89ffb
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>

Nested enum types are implicitly static.

Change-Id: Id3d7886087494fb67bc0d080b4a3491fb4baac19
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>

The ref points to an ObjectId that then is translated into a RevCommit.
This translation can be costly and with the incremental reachability
check is probably not needed for most of the elements.

Delay the translation from ObjectId to RevCommit to when it is needed.
Use Streams, that have the laziness built-in, all the way from Ref to
RevCommit.

This should reduce the latency for reachability checks over big sets of
references.

Change-Id: I28693087321b2beff3eaa1f3d2e7840ab0eedc6d
Signed-off-by: Ivan Frade <ifrade@google.com>

Preparatory change. Converting ObjectIds to RevCommits is potentially
expensive and in the incremental reachability check, it is probably not
required for all elements in the collection.

Pass a Stream to the reachability checker. In the follow up we make
the conversion from ObjectId to RevCommit in the stream (i.e. on
demand). This should reduce the latency of reachability checks over big
sets of references.

Change-Id: I9f310e331de5b0bf8de34143bd7dcd34316d2fba
Signed-off-by: Ivan Frade <ifrade@google.com>

UploadPack needs to check if object ids that weren't advertised before
are reachable from the references visible to the user. In the
bitmap-based reachability check, this is done incrementally: checking
against one reference, if anything remaining adding a second and so on.
It is more efficient to check first more common references (e.g. refs/heads/*)

Sort the references for the reachability checker. This should solve the
connectivity earlier and require less bitmap creation and less memory.

Change-Id: I48ac10d71e29fab2d346479802401eaea4aacb5c
Signed-off-by: Ivan Frade <ifrade@google.com>

The flag enabling sideband-all is used in two places: in UploadPack
for advertisement and in the protocol parser to read it from the
request.

This leds to problems in distributed deployments where the two requests of
a fetch can go to different servers with different configurations.

Use the existing allowsidebandall to accept the sideband-all request
(and respond to it) and introduce a new "advertisesidebandall" to toggle
the advertising of the feature.

Change-Id: I892d541bc3f321606c89bad1d333b079dce6b5fa
Signed-off-by: Ivan Frade <ifrade@google.com>

This was missed in b8d9734c

Change-Id: I4b9c1cf058eff16af725375047d743944e7f5db4
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

Exception handling can be isolated from UploadPack. This makes it
possible to make the exception handler pluggable.

Change-Id: Ieebbd6711963c7f2e47a98783b4ad815793721c7
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>

This consolidates the sideband stream creation code and the error
handling code for the sideband-allowed part in the Git protocol to one
place.

Change-Id: I0e3e94564f50d1be32006f9d8bcd1ef1ce6bf07e
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>

ErrorWriter writes an error message to the user. The implementation is
swapped once it detects that the client supports sideband. By default it
uses the protocol level ERR packet, which was introduced recently.

In total the error output is done in two different places;
UploadPack#upload and UploadPack#sendPack. These will be consolidated in
the next change.

Change-Id: Ia8d72e31170bbeafc8ffa8ddb92702196af8a587
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>

These warnings were missed to address in a0048208 which introduced them.

Change-Id: Ia2d15fdce72c10378d020682b80fe7fc548c0d4c
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

Teach UploadPack to take a provider of URIs corresponding to cached
packs. When fetching, if the client supports the packfile-uri feature,
and if such a cached pack were to be streamed, instead send the
corresponding URI.

This packfile-uri feature is implemented in the jt/fetch-cdn-offload
branch of Git. There is interest in this feature [1], but it is not yet
merged.

[1] https://public-inbox.org/git/cover.1552073690.git.jonathantanmy@google.com/

Change-Id: I9a32dae131c9c56ad2ff4a8a9638ae3b5e44dc15
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>

In a subsequent patch, in some cases, PackWriter#writePack will be
responsible for both the "packfile-uris" and "packfile" sections,
meaning that (in these cases) it must write the "packfile" section
header itself.

In preparation for that patch, move the writing of the "packfile"
section header closer to the invocation of PackWriter#writePack when the
entire fetch response is configured to use the sideband. This means that
"packfile" is written *after* objects are counted (and progress messages
sent to the client in sideband 2) when the "sideband-all" feature is
used (whether "packfile-uris" is used or not), and written *before*
objects are counted otherwise.

Having code to write "packfile" in two places is unfortunate but
necessary. When "sideband-all" is not used, object counting has to
happen after "packfile" is written, because "packfile" activates the
sideband that allows counting progress to be transmitted. When
"packfile-uris" is used, object counting has to happen before "packfile"
is written, because object counting determines whether to send
"packfile-uris" or "packfile". When "sideband-all" is used but
"packfile-uris" is not used, either way works; this commit uses
"packfile-uris" behavior in this case.

Also make the naming of the sideband-activating methods in PacketLineOut
more consistent.

Change-Id: Ifbfd26cc26af10c41b77758168833702d6983df1
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>

A caller cannot install a second hook in the UploadPack without
overwriting whatever is already there.

Offer a method to get the current protocol v2 hook, so it can be chained
with new hooks.

Change-Id: Icb06f94ec52b8c8714f509b5b8622d6db42960fb
Signed-off-by: Ivan Frade <ifrade@google.com>

Allow the client to specify "sideband-all" in a fetch v2 request,
indicating that the whole response is to be multiplexed (with a sideband
indicator on every non-flush and non-delim pkt) instead of only the
packfile being multiplexed. This allows, for example, progress messages
to be sent at any point in the response.

This implements the "sideband-all" feature documented in
Documentation/technical/protocol-v2.txt in Git.

Change-Id: I3e7f21c88ff0982b1b7ebb09c9ad6c742c4483c8
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>

It is difficult to track what's happening with the pckOut instance
field, so replace it with a local variable in #upload instead.

Change-Id: Ibd9225b28334b7133eccdc6d82b26fc96cbde299
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>

All other exceptions are handled in a wrapped sendPack method.
Consolidate the error handling code.

Change-Id: Ieac0ce64960534d009d1e6b025130b021b744794
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>

By doing this, exceptions thrown by sendPack are also covered by the
same code.

Change-Id: I3509f2d832af1410f307e931577e4d07e32b014e
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>

When another exception is thrown while handling another exception, that
exception can be attached to the original exception since Java 7
(Throwable#getSuppressed). Attach the secondary exception to the
original exception instead of throwing it away.

Change-Id: Ia093b8207714f2638e0343bc45a83d4342947505
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>

If a client clones with "--filter=blob:none", the checkout that "git
clone" automatically does causes the client to fetch all blobs at HEAD.
When fetching from a non-bitmapped repository, this will fail if an
object walk is ever needed, because JGit currently rejects such requests
- see the commit message of d3021788d2 ("Use bitmaps for non-commit
reachability checks", 2017-11-10) for more information.

Rejecting such requests in the absence of bitmaps is probably
overzealous: it is true that the server would prefer to have bitmaps in
this case, but there might be a small proportion of repos (for example,
very small repos or newly created ones) that do not have bitmaps, yet
the server would still like to have partial clones for them.

So, allow such requests, performing the object walk reachability check
if necessary. Limit this to servers with "uploadpack.allowFilter"
configured, so that servers wanting to support partial clone have this
functionality, and servers that do not support partial clone do not have
to pay the object walk reachability check cost.

Change-Id: I51964bafec68696a799625d627615b4f45ddbbbf
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>

When cloning repository with --single-branch option, tag chains are not
packed and pack file is broken in some cases.

Typical test-case:
git tag -a test_tag <commit-id>
git tag -a test_prev_tag test_tag
git tag -d test_tag

git clone --single-branch <repository>
fatal: did not receive expected object <test_tag_id>

The reason for that is missing object for original test_tag reference,
which was deleted.

Problem description:
When pack-objects is given --include-tag, it peels each tag reference
down to a commit. If the commit is prepared to be packed, we we have to
include such tag too. The problem is when the tag points to through some
chain of other tag to commit. Then, the inner tags are not added leading
to broken pack.

Fix:
When going to commit, we have to check and add any of the tags on the
way (if they were not selected, which may happen with --single-branch
option).

Change-Id: I1682d4a2c52d674f90a1b021e0f6c3524c5ce5bc
Signed-off-by: Pavel Flaška <Pavel.Flaska@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

Replace reference comparisons of PacketLineIn's DELIM and END strings
with usage of the helper methods isDelimiter() and isEnd().

Change-Id: I52dcfc4ee9097f1bd6970601c716701847d9eebd
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>

Every caller would need to check if bitmaps are available in the repo to
instantiate a reachability checker.

Offer a method to create the reachability checker in the walk: the
caller has already a walk over the repo, and the walk has all the
information required.

This allows us to make the implementation classes package-private.

Change-Id: I355e47486fcd9d55baa7cb5700ec08dcc230eea5
Signed-off-by: Ivan Frade <ifrade@google.com>

In 7b96bd812e ("UploadPack: Use reachability checker to validate
non-advertised wants", 2019-05-16), a "walk.setRetainBody(false);"
statement was inadvertently deleted. (An earlier version of this commit
had this line in another part of the code and a review comment suggested
to move it back here; the line was then deleted from the other part of
the code but not readded.) Restore this line.

Change-Id: I96ff6106ba9e4eef429388c83e898b3363295f69
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>

In "Reachable commit" request validators, we need to check that a "want"
in the request, that hasn't been advertised, is reachable from the refs
visible to the user.

Current code has intermixed the translation of ObjectIds to RevCommits
(and its error handling) with the actual walk, with the delegation to
bitmaps in restricted circunstances.

Refactor the code to make it "flatter" and more readable. Move ObjectIds
to RevCommits translation to its own functions. Use the reachability
checker instead of a newly defined walk.

Before the non-advertised wants were validated with bitmaps only if any
"want" refered to an non-commit. Now they will be validated with bitmaps
also if the "wants" refer all to commits.

Change-Id: Ib925a48cde89672b07a88bba4e24d0457546baff
Signed-off-by: Ivan Frade <ifrade@google.com>

This is used when fetching, and in particular to populate a partial
clone or a virtual file system cache as the user navigates. With this,
a client can pre-fetch a few directories deeper than only the current
directory.

depth:0 will omit all trees, and is useful if you only want to fetch
the commits of a repository, or fetch just a single tree or blob object.
depth:1 will fetch only the root tree of all commits fetched. depth:2
will fetch the root tree and all blobs and tree objects directly
referenced from it. depth:3 gets one more level, and so on. depth:#
will not filter a blob or tree that is explicitly marked wanted.

Bitmaps are disabled when this filter is used.

This implementation is quite slow because it iterates over all omitted
objects rather than skipping them. This will be addressed in follow-up
commits.

Change-Id: Ic312fee22d60e32cfcad59da56980e90ae2cae6a
Signed-off-by: Matthew DeVore <matvore@gmail.com>

Change-Id: I1a9112e6a4f938638c599b489cb0858eca27ab91
Signed-off-by: Carsten Hammer <carsten.hammer@t-online.de>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

Use the FilterSpec object so that less code has to know about the make-up of
FilterSpecs. When fields are added to FilterSpec, these pieces of code won't
need updating again.

Change-Id: I2b9e59a9926ff112faf62a3fa2d33c961a1779e5
Signed-off-by: Matthew DeVore <matvore@gmail.com>

This increases type-safety and is ground work for support of the
"tree:<depth>" filter.

Change-Id: Id19eacdcdaddb9132064c642f6d554b1060efe9f
Signed-off-by: Matthew DeVore <matvore@gmail.com>

HashMap<String, Ref> has a memory overhead for refs. Use RefMap.

Change-Id: I3fb4616135dacf687cc3bc2b473effc66ccef5e6
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>

RefAdvertiser.send(Map<String, Ref>) is deprecated in favour of
RefAdvertiser.send(Collection<Ref>). Subclasses that need to override
the "send" method need to override also the deprecated version, because
it is still invoked by BaseReceivePack and UploadPack.

Remove the last usages of the deprecated method.

Change-Id: I7eba426970251f78801ddf96b87a65d1baaebdcf
Signed-off-by: Ivan Frade <ifrade@google.com>

The commit body contains the commit message, which is not needed for
reachability checks.

Change-Id: Ie209c3b3f022579942f05b8b5d0625ce26400a5d
Signed-off-by: Jonathan Nieder <jrn@google.com>

Similar to UploadPack.getDepth() to know the shallow clone depth, expose
the user-specified filter blob limit for partial clones.

Change-Id: I04bde06862a1cf8a9862d950c15023c49d16a2a6
Signed-off-by: Terry Parker <tparker@google.com>

Change-Id: Ida09cabb11740e6752a6f079d0528d38021fa2dd
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>

This allows scanning through refs once instead of once per ref, which
should make the lookup less expensive for some RefDatabase
implementations.

Change-Id: I1434f834186cc9a6b4e52659e692b1000c926995
Signed-off-by: Jonathan Nieder <jrn@google.com>

Change-Id: I22a244114ef973b3801cf3b639ff782f21c02f98
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

Clients can use --shallow-exclude to obtain information about what
commits are reachable from refs they are not supposed to be able to
see.  Plug the hole by allowing the AdvertiseRefsHook and RefFilter to
take effect here, too.

Change-Id: If2b8e95344fa49e10a6a202144318b60d002490e
Signed-off-by: Jonathan Nieder <jrn@google.com>

The AdvertiseRefsHook can be called twice if the following conditions
hold:

 1. This AdvertiseRefsHook doesn't set this.refs.
 2. getAdvertisedOrDefaultRefs is called after getFilteredRefs.

For example, this can happen when fetchV2 is called after lsRefsV2
when using a stateful bidirectional transport.

The second call does not accomplish anything useful.  Guard it with
'if (!advertiseRefsHookCalled)' to avoid wasted work.

Reported-by: Jonathan Tan <jonathantanmy@google.com>
Change-Id: Ib746582e4ef645b767a5b3fb969596df99ac2ab5
Signed-off-by: Jonathan Nieder <jrn@google.com>

In the longer term, we can add support for this to the
RequestValidator interface.  In the short term, this is a minimal
band-aid to ensure any refs the client requests are visible to the
client.

Change-Id: I0683c7a00e707cf97eef6c6bb782671d0a550ffe
Reported-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

ProtocolV2Parser explains:

  // TODO(ifrade): This validation should be done after the
  // protocol parsing. It is not a protocol problem asking for an
  // unexisting ref and we wouldn't need the ref database here.

Do so.  This way all ref database accesses are in one place, in the
UploadPack class.

No user-visible change intended --- this is just to make the code
easier to manipulate.

Change-Id: I68e87dff7b9a63ccc169bd0836e8e8baaf5d1048
Signed-off-by: Jonathan Nieder <jrn@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

AdvertiseRefsHook is used to limit the visibility of the refs in Gerrit.
If this hook is not called, then all refs are treated as visible.

In protocol v2, the hook is not called, causing the server to advertise
all refs.  This bug was introduced in v5.0.0.201805221745-rc1~1^2~9
(Execute AdvertiseRefsHook only for protocol v0 and v1, 2018-05-14).

Even before then, the hook was not called in requests after the
capability advertisement, so in transports like HTTP that do not retain
state between round-trips, the server would advertise all refs in
response to an ls-refs (ls-remote) request.

Fix both cases by using getAdvertisedOrDefaultRefs to retrieve the
advertised refs in lsRefs, ensuring the hook is called in all cases that
use its result.

[jn: backported to stable-5.0; split out from a larger patch that also
 fixes protocol v0; avoided filtering this.refs by ref prefix]

Change-Id: I64bce0e72d15b90baccc235c067e57b6af21b55f
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

AdvertiseRefsHook is used to limit the visibility of the refs in Gerrit.
If this hook is not called, then all refs are treated as visible,
causing the server to serve commits reachable from branches the client
should not be able to access, if asked to via a request naming a guessed
object id.

This bug was introduced in v2.0.0.201206130900-r~123 (Modify refs in
UploadPack/ReceivePack using a hook interface, 2012-02-08).  Stateful
bidirectional transports are not affected.

Fix it by moving the AdvertiseRefsHook call to
getAdvertisedOrDefaultRefs, ensuring the hook is called in all cases.

[jn: backported to stable-4.5 by splitting out tests and the protocol v2
 specific parts]

Change-Id: I159f396216354f2eda3968d17802e166d8c8ec2d
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

This makes the implementation consistent with the other similar
methods in this class.

Change-Id: I007876aad883615d696c8eabc886818ae00b10ee
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>

The setProtocolV2Hook sets the protocolV2Hook to whatever value is
passed, which could be null, but the invocations of protocolV2Hook's
methods are not guarded by null-checks.

Annotate the parameter as @Nullable and set ProtocolV2Hook.DEFAULT
when null is passed. This makes the implementation consistent with
other similar methods that set a hook or filter with possible null
value.

Change-Id: I70919a3248d4c2658783941a37c47e437cff0baa
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>

The class has several methods where passing a null parameter is
valid. Annotate those parameters as @Nullable.

Change-Id: Ie08893ee3ab34c1ffb2db875b4ab049ad065c697
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>

s/occured/occurred/
s/stablished/established/

Change-Id: Ib052bef60bd18043001a30ed43754ee1a5182016
Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.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>

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>