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>
BaseReceive/UploadPack: Stop using deprecated RefAdvertiser.send(Map)
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>
UploadPack: Do not retain commit body when checking wants
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>
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>
UploadPack: Filter refs used for deepen-not resolution
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>
UploadPack: Filter refs used for want-ref resolution
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>
UploadPack: Defer want-ref resolution to after parsing
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>
UploadPack: Rewrite setAdvertiseRefsHook to use ternary operator
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>
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>
Throw error when deepen-since excludes all commits
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>
UploadPack v0: Extract "agent" client capability at parse time
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: Return correct peer user agent on v2 requests
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>
Accept protocol v2 server options on fetch and ls-refs requests
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>
UploadPack.FirstLine: Revert renaming of getOptions into getCapabilities
This is public facing, stable API.
Fortunately, this class is deprecated and will be removed in the next
major version bump.
Change-Id: I91193964732e9d1943e9dc613256196e9c9d1274
Signed-off-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
UploadPack: Use request instead of field for depth
One more step in removing state from UploadPack, using the request
object instead.
Unfortunately, hooks get from UploadPack information about the current
request. Changing the hooks to receive the request is a public API
change, so at the moment lets keep a reference to the current request.
This kills half the benefit of using a request object vs fields, but
at least we still get better modularity.
Change-Id: I86803d876a621b727c66ee73f2880c93190629e9
Signed-off-by: Ivan Frade <ifrade@google.com>
UploadPack: Use request in computeShallowUnshallow
All data required in this function is available in the request object.
Use that object instead of class members. This reduces class state and
is more readable.
Make the function use a request object and remove the now unnecessary
field "deepenNotRefs".
Change-Id: If861e44c2860a78cf19f456d1b3feb7ddc314cce
Signed-off-by: Ivan Frade <ifrade@google.com>
UploadPack: Use request instead of field for filterBlobLimt
filterBlobLimit is not part of the UploadPack state, and as field
of the class is difficult to see where it is set or accessed.
Use the request object instead of a field. This reduces
UploadPack state and makes clearer how the value is used.
Change-Id: I96a04a5a8b31bf2243de701e1fd7ebb4080b49e2
Signed-off-by: Ivan Frade <ifrade@google.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>
Move protocol v0/v1 parsing to its own class and request objects
Protocol v0/v1 parsing code doesn't have any real dependency on UploadPack.
Move it to its class and use a request object to read the data in
UploadPack.
This makes the code easier to test, keeps similar structure than protocol v2,
reduces the line count of UploadPack and paves the way to remove the
members as implicit parameters in it.
Change-Id: I8188da8bd77e90230a7e37c02d800ea18463694f
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>
Move first line parsing for v0/v1 pack negotiation out of UploadPack
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>
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>
Fetch code has been moved to a ProtocolV2Parser, but ls-refs code is
still in UploadPack.
Moving it to the parser makes it easier to test, keeps the parsing
together and makes the two commands follow similar structure.
Change-Id: I573ce543e804ddeb9f83303b4af250b7cddc8cad
Signed-off-by: Ivan Frade <ifrade@google.com>
UploadPack.processShallow is doing too many things and offering a
confusing API. It is filtering or splitting commit ids depending
if a parameter is null and writing them out (or not) depending on another
flag.
Iterate the list and announce to Consumers what object ids need
to be marked as (un)shallow. They decide what to do with them.
As java consumers don't allow to propagate exceptions, define
our own functional interface for it.
Change-Id: I619cf2eed9b1e0338151120b8ef87a463fbe8827
Signed-off-by: Ivan Frade <ifrade@google.com>
UploadPack.fetchV2: Make shallow commits list non-nullable
shallowCommits variable is nullable only to signal later
if code has gone through the processShallow code.
Use a flag to indicate that condition and make shallowCommits
non-nullable. This makes code clearer and paves the way to untangle
processShallow in a follow-up commit.
Change-Id: I898e6aaf6f860bb6afafbac05653ba116c9b2da6
Signed-off-by: Ivan Frade <ifrade@google.com>
Extract protocol v2 fetch request parsing to its own class
The parsing code for protocol v2 fetch doesn't have any dependency on
the rest of UploadPack.
Move it to its own class. This makes testing easier (no need to
instantiate the full UploadPack), simplifies the code in UploadPack and
increases modularity.
At the moment, the parser needs to know about the reference database to
validate incoming references. This dependency could be easily removed
moving the validation later in the flow, after the parsing, where other
validations are already happening. Postponing that to keep this patch
about moving unmodified code around.
Change-Id: I7ad29a6b99caa7c12c06f5a7f30ab6a5f6e44dc7
Signed-off-by: Ivan Frade <ifrade@google.com>
FetchV2Request: Rename shallowSince to deepenSince
This fetch parameter is called deepen-since in the protocol. Call it
the same thing in the request object to make the code easier to reason
about.
This doesn't touch UploadPack#shallowSince, which is likely to be
eliminated altogether in a later patch anyway.
Change-Id: I8ef34bc7ad12fae3a9057ae951367cc024e1a1cb
Suggested-by: Ivan Frade <ifrade@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>
Make "doneReceived" a member of the fetch request. It indicates if the
"done" line has been received (so it makes sense there) and makes all
the code after the parsing depend only on the request.
Rename "shallowExcludeRefs" to "deepenNot". Those refs come in
"deepen-not" lines in the protocol, and this name makes clearer the
intention.
Change-Id: I7bec65de04930277266491d278de7c3af7d8cbe6
Signed-off-by: Ivan Frade <ifrade@google.com>
When this exception is thrown, the `depth` member variable isn't set
yet, resulting in a confusing error message: "Invalid depth: 0".
Change-Id: I8a2bd5e1d9bec00acb0b8857bbf6821e95bf1369
Signed-off-by: Ivan Frade <ifrade@google.com>
UploadPack: avoid conflating shallow commit lists in protocol v2
At the moment there are two copies of the client shallow commit list:
one in the request and another in the clientShallowCommits member of
the class.
The verifyShallowCommit function was removing missing object ids
from the member but not the request list, and code afterwards was
using the request's version.
In practice, this didn't cause trouble because these shallow commits
are used as endpoint for a walk, and missing ids are just never reached.
Change-Id: I70a8f1fd46de135da09f16e5d954693c8438ffcb
Signed-off-by: Ivan Frade <ifrade@google.com>
This makes it symmetrical with ls-refs operation and gives the
instantiator of UploadPack the chance to run some code after parsing
the protocol and before any actual work for the fetch starts.
Request and Builder methods keep the naming in the original code to
make this change just about request encapsulation and hook invocation.
They are package-private for now to allow further improvements.
Change-Id: I5ad585c914d3a5f23b11c8251803faa224beffb4
Signed-off-by: Ivan Frade <ifrade@google.com>
UploadPack: shallowExcludeRefs doesn't need to be nullable
Code can check size instead of null, and that makes the initialization
trivial.
Change-Id: Icbe655816429a7a680926b0e871d96f3b2f1f7ba
Signed-off-by: Ivan Frade <ifrade@google.com>
In Git protocol v2, UploadPack and ReceivePack have the same
capabilities and can process any protocol v2 request. For example, a
client can sent a "fetch" command to the "/git-receive-pack" endpoint.
This makes it difficult for existing hook interfaces. For example,
PreUploadHook takes UploadPack, but a "fetch" command may be received by
ReceivePack.
To resolve this skew, this change introduce a different hook interface
for the protocol v2. The hook takes a request that is independent to the
handlers (UploadPack, ReceivePack). Also this makes it clear what
parameters the hook is counting on, instead of keep track of the hook
using getters from UploadPack / ReceivePack.
Bug: 534847
Change-Id: I71f3266584483db1e2b2edfc1a72d0bdf1bb6041
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
When fetching with protocol v2, git expects the shallow-info section to
appear before wanted-refs if both appear in the response. Teach
UploadPack to do this.
Change-Id: Ie26a91edcce5d27a1d727d7fba5c30e1144e118b
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Matthew DeVore
Masaya Suzuki
Ivan Frade
Jonathan Nieder
Terry Parker
David Pursehouse
Matthias Sohn
Michael Keppler
Jonathan Tan
Minh Thai