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>

Commit body contains the message that is not needed for reachability checks, and
takes up memory unnecessarily.

Change-Id: I0c7f6da249bf9c4fda9dc9e62e809322c68effce
Signed-off-by: Minh Thai <mthai@google.com>

Its implementation contains

		} catch (IOException e) {
			// Legacy API, assume error means "no"
			return false;
		}

Better to use ObjectDatabase#has, which throws IOException to report
errors.

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

Another step toward merging BaseReceivePack into ReceivePack.

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

Another step toward merging BaseReceivePack into ReceivePack.

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

Another step toward eliminating BaseReceivePack as a separate API.

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

This is a first step toward eliminating the BaseReceivePack API.

Inspired by a larger change by Dan Wang <dwwang@google.com>.

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

This simplifies the BaseReceivePack class and decreases its API
surface, which should make merging with ReceivePack easier.

Inspired by 6aca8899a5 (Move first line
parsing for v0/v1 pack negotiation out of UploadPack, 2018-09-17).

Change-Id: I1fc175d15aa7cb5968c26fc83a95480403af617c

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>

Remove it from

 * package private functions.

 * try blocks

 * for loops

this was done with the following python script:

$ cat f.py
import sys
import re
import os

def replaceFinal(m):
  return m.group(1) + "(" +  m.group(2).replace('final ', '') + ")"

methodDecl = re.compile(r"^([\t ]*[a-zA-Z_ ]+)\(([^)]*)\)")

def subst(fn):
  input = open(fn)
  os.rename(fn, fn + "~")

  dest = open(fn, 'w')
  for l in input:
    l = methodDecl.sub(replaceFinal, l)
    dest.write(l)
  dest.close()


for root, dirs, files in os.walk(".", topdown=False):
  for f in files:
    if not f.endswith('.java'):
      continue

    full = os.path.join(root, f)
    print full
    subst(full)

Change-Id: If533a75a417594fc893e7c669d2c1f0f6caeb7ca
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>

Change-Id: Id924f79c8b2c720297ebc49bf9c5d4ddd6d52547
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>

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

An application can choose to invoke setAdvertisedRefs multiple times,
for example several AdvertiseRefsHook installed in a chain. Each of
these invocations populates the advertisedHaves collection with the
unique set of ObjectIds.

This can lead to a server over-advertising with ".have" lines if the
first hook pushes in a lot of references, and the second hook filters
this to a subset.  ReceivePack will advertise the unique objects from
the first hook using ".have" lines, which may lead to a huge
advertisement sent to the client.

This can also contribute to a very slow connectivity check after the
pack is parsed as ReceivePack calls markUninteresting on every commit
in advertisedHaves.  This may require expanding a lot of subtrees to
mark all trees as uninteresting as well.  On a very big repository
this can lead to a many-second stall.

Clear the advertisedHaves collection any time the refs are updated.
Add a test to verify the correct set of objects was sent.

Change-Id: I97f6998d0597251444a2e846a3ea1f461bae96f9

Happily, most anonymous SectionParser implementations can be replaced
with FooConfig::new, as long as the constructor takes a single Config
arg. Many of these, the non-public ones, can in turn be inlined. A few
remaining SectionParsers can be lambdas.

Change-Id: I3f563e752dfd2007dd3a48d6d313d20e2685943a

Since the introduction of generic type parameter inference in Java 7,
it's not necessary to explicitly specify the type of generic parameters.

Enable the warning in Eclipse, and fix all occurrences.

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

In 0bff481d45 to accurately use the two
limits it was necessary to move the LimitedInputStream out of the
PacketLineIn and further down to the PackParser. Unfortuantely this
didn't survive review, as a buggy test failed and the "fix" was to
drop this part of the code.

The maxPackSizeLimit should apply to the pack stream, not the pkt-line
framing used to send commands to control the ReceivePack instance. The
commands are controlled using a different limit. The failing test allowed
too many bytes in the pack and was only failing because it was including
the command framing. The correct fix for the test was simply to drop the
limit lower, to more closely match the actual pack size.

Change-Id: I47d3885b9d7d527e153df7ac9c62fc2865ceecf4

Set missingOverrideAnnotation=warning in Eclipse compiler preferences
which enables the warning:

  The method <method> of type <type> should be tagged with @Override
  since it actually overrides a superclass method

Justification for this warning is described in:

  http://stackoverflow.com/a/94411/381622

Enabling this causes in excess of 1000 warnings across the entire
code-base. They are very easy to fix automatically with Eclipse's
"Quick Fix" tool.

Fix all of them except 2 which cause compilation failure when the
project is built with mvn; add TODO comments on those for further
investigation.

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

Place a configurable upper bound on the amount of command data
received from clients during `git push`.  The limit is applied to the
encoded wire protocol format, not the JGit in-memory representation.
This allows clients to flexibly use the limit; shorter reference names
allow for more commands, longer reference names permit fewer commands
per batch.

Based on data gathered from many repositories at $DAY_JOB, the average
reference name is well under 200 bytes when encoded in UTF-8 (the wire
encoding).  The new 3 MiB default receive.maxCommandBytes allows about
11,155 references in a single `git push` invocation.  A Gerrit Code
Review system with six-digit change numbers could still encode 29,399
references in the 3 MiB maxCommandBytes limit.

Change-Id: I84317d396d25ab1b46820e43ae2b73943646032c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

1beb3ccd1e was submitted in 4.6.0-SNAPSHOT

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

This is like PackStatistics, but for PackParser.

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

This allows the same try/catch to handle parsing the command list,
push certificate and push options. Any errors will be caught and
handled by the same catch block, as the client is in the same state.

Change-Id: I13a66f9100e2dc8ca8f72cd701a5bd44d093ec84

Refactor all of the push option support code to allocate the list
immediately before parsing the options section off the stream.

Move option support down to ReceivePack instead of BaseReceivePack.
Push options are specific to the ReceivePack protocol and are not
likely to appear in the 4 year old subscription proposal.  These
changes are OK before JGit 4.5 ships as no consumer should be relying
on these new APIs.

Change-Id: Ib07d18c877628aba07da07cd91875f918d509c49

Initialize pushOptions when we decide to use them, instead of when we
advertise them.

In the case of HTTP the advertisement is in a different network
request, hence in a different instance of the BaseReceivePack.

Change-Id: I094c60942e04de82cb6d8433c9cd43a46ffae332
Signed-off-by: Stefan Beller <sbeller@google.com>

Change-Id: I5c6790719991931d615d821c900bfd90a20e540b
Signed-off-by: Dan Wang <dwwang@google.com>

Example usage:
$ ./jgit push \
  --push-option "Reviewer=j.doe@example.org" \
  --push-option "<arbitrary string>" \
  origin HEAD:refs/for/master
Stefan Beller has also made an equivalent change to CGit:
http://thread.gmane.org/gmane.comp.version-control.git/299872

Change-Id: I6797e50681054dce3bd179e80b731aef5e200d77
Signed-off-by: Dan Wang <dwwang@google.com>

If the client sent a well-formed enough request to see it wants to use
side-band-64k for status reporting (meaning its a modern client), but
any other command record was somehow invalid (e.g. corrupt SHA-1)
report the parsing exception using channel 3.  This allows clients to
see the failure and know the server will not be continuing.

git-core and JGit clients send all commands and then start a sideband
demux before sending the pack. By consuming all commands first we get
the client into a state where it can see and respond to the channel 3
server failure.

This behavior is useful on HTTPS connections when the client is buggy
and sent a corrupt command, but still managed to request side-band-64k
in the first line.

Change-Id: If385b91ceb9f024ccae2d1645caf15bc6b206130

The "shallow $id" parsing can also throw InvalidObjectIdException,
just like parseCommand. Move it into its own method with a proper
try-catch block to convert to the checked PackProtocolException.

Change-Id: I6839b95f0bc4fbf4d2c213331ebb9bd24ab2af65

This exception is already handled in the outer catch block.

Change-Id: Ifc04b8e138732a97b9c0ee323af923b8e52a7f8e

Instead of deferring until after command parsing, enable the
capabilities after the first pkt-line has been read from the client.
This allows the server to setup the side-band-64k channel immediately.

Change-Id: I141b7fc92e983a41d3a58da8e1464a6917422b6c

The more specific type InvalidObjectIdException is thrown by
ObjectId.fromString().  Use it here in ReceivePack as the more
generic IAE is never thrown by the body of the try-catch block.

Change-Id: I53fc13c561c7d429a50b5eb82773f1a670431c54

This makes atomic more consistent with other properties like
allowNonFastForwards.

Change-Id: Ie3b41f14ffba1d0b206ffecc697377417daa6973

Change-Id: I0e4e6cef693e8138562f5ca345e0db61a8abef9c

If one or more commands is failing the entire group usually has to
also fail with "transaction aborted". Pull this loop into a helper
so the idiom can be easily reused in several places throughout JGit.

Change-Id: I3b9399b7e26ce2b0dc5f7baa85d585a433b4eaed

This avoids duplication of code between receive-pack and fetch-pack paths.
Separate methods are still required to check use of receive.fsckobjects vs.
fetch.fsckobjects, both of which default to transfer.fsckobjects.

Change-Id: I41193e093e981a79fc2f63914e273aaa44b82162

This change fixes all compiler errors in JGit and replaces possible
NPE's with either appropriate exceptions, avoiding multiple "Nullable
return" method calls or early returning from the method.

Change-Id: I24c8a600ec962d61d5f40abf73eac4203e115240
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>

Change-Id: Id912f45c933c78a85b245fa7b6edebbeb40b009c

Rather than lazily parsing the push in this method, parse it at the
end of recvCommands(), which already contains the necessary try/catch
for handling this error. This allows later callers to avoid having to
handle this condition superfluously.

Change-Id: I5dcaf1a44bf4e321adc281e3381e7e17ac89db06

We have done this since forever with the "wanted old new ref" error,
so let's do it for other such errors thrown in the same block as well.

Change-Id: Ib3b1c7f05e31a5b3e40e85eb07b16736920a033b

Discussion on the git mailing list has concluded[1] that the intended
behavior for all (non-sideband) portions of the receive-pack protocol
is for trailing LFs in pkt-lines to be optional. Go back to using
PacketLineIn#readString() everywhere.

For push certificates specifically, we agreed that the payload signed
by the client is always concatenated with LFs even though the client
MAY omit LFs when framing the certificate for the wire. This is still
reflected in the implementation of PushCertificate#toText().

[1] http://thread.gmane.org/gmane.comp.version-control.git/273175/focus=273412

Change-Id: I817231c4d4defececb8722142fea18ff42e06e44

Change-Id: I25f3a5582a45dd0ec8f78f5daf74c2203797a184

Add test for this case in both the enabled and disabled cases.

Change-Id: If9d12192a2dc9f9dd1eac9844b5a7b0edadc0b34

This may be used by e.g. a custom reflog implementation to record
this information along with the ref update.

Change-Id: I44adbfad704b76f9c1beced6e1ce82eaf71410d2

The default behavior is to read a repository's signed push
configuration from that repo's config file, but this is not very
flexible when it comes to managing groups of repositories (e.g. with
Gerrit). Allow callers to override the configuration using a POJO.

Change-Id: Ib8f33e75daa0b2fbd000a2c4558c01c014ab1ce5

Add a missing continues to prevent falling through to the command
parsing section. The first continue happens when the command list is
empty, so change the condition to see whether we have read the first
line, rather than any commands.

Fix comparison to BEGIN_SIGNATURE to use raw line with newline.

Change-Id: If3d92f5ceade8ba7605847a4b2bc55ff17d119ac

- Consistently return structured data, such as actual ReceiveCommands,
  which is more useful for callers that are doing things other than
  verifying the signature, e.g. recording the set of commands.
- Store the certificate version field, as this is required to be part
  of the signed payload.
- Add a toText() method to recreate the actual payload for signature
  verification. This requires keeping track of the un-chomped command
  strings from the original protocol stream.
- Separate the parser from the certificate itself, so the actual
  PushCertificate object can be immutable. Make a fair attempt at deep
  immutability, but this is not possible with the current mutable
  ReceiveCommand structure.
- Use more detailed error messages that don't involve NON-NLS strings.
- Document null return values more thoroughly. Instead of having the
  undocumented behavior of throwing NPE from certain methods if they
  are not first guarded by enabled(), eliminate enabled() and return
  null from those methods.
- Add tests for parsing a push cert from a section of pkt-line stream
  using a real live stream captured with Wireshark (which, it should
  be noted, uncovered several simply incorrect statements in C git's
  Documentation/technical/pack-protocol.txt).

This is a slightly breaking API change to classes that were
technically public and technically released in 4.0. However, it is
highly unlikely that people were actually depending on public
behavior, since there were no public methods to create
PushCertificates with anything other than null field values, or a
PushCertificateParser that did anything other than infinite loop or
throw exceptions when reading.

Change-Id: I5382193347a8eb1811032d9b32af9651871372d0

C git's receive-pack.c strips trailing newlines in command lists when
present[1], although send-pack.c does not send them, at least in the
case of command lists[2]. Change JGit to match this behavior.
Add tests.

This also fixes parsing of commands in the push cert, which, unlike
commands sent in the non-push case, always have trailing newlines.

[1] 7974889a05/builtin/receive-pack.c (L1380)
where packet_read_line chomps newlines:
7974889a05/pkt-line.c (L202)

[2] 7974889a05/send-pack.c (L470)

Change-Id: I4bca6342a7482a53c9a5815a94b3c181a479d04b

When during a PushOperation the server hits an exception different from
UnpackException the JGit server behaved wrong. That kind of exceptions
are handled so late that the connection is already released and the
information whether to talk sideband to the client is lost. In detail:
ReceivePack.receive() will call release() and that will reset the
capabilities. But later on the stack in ReceivePackServlet.doPost() it
is tried to send a response to client now with reset capabilities (no
sideband!).

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

git-core has supported this for a long time; allowing clients to
avoid progress messages from the server if they are dumping to a
pipe instead of a tty.

Avoid the two progress monitors going on side-band and expose
isQuiet() method to allow hooks to also reduce their output if
this is sensible for them.

Change-Id: I1df7e38d16765446b441366500b017a90b8ff958

Despite being the primary author of RevWalk and ObjectWalk I still
fail to remember to setRetainBody(false) in application code using
an ObjectWalk to examine the graph.

Document the default for RevWalk is setRetainBody(true), where the
application usually wants the commit bodies to display or inspect.

Change the default for ObjectWalk to setRetainBody(false), as nearly
all callers want only the graph shape and do not need the larger text
inside a commit body. This allows some code in JGit to be simplified.

Change-Id: I367e42209e805bd5e1f41b4072aeb2fa98ec9d99