Cache user global and system-wide git configurations
So far the git configuration and the system wide git configuration were
always reloaded when jgit accessed these global configuration files to
access global configuration options which are not in the context of a
single git repository. Cache these configurations in SystemReader and
only reload them if their file metadata observed using FileSnapshot
indicates a modification.
Change-Id: I092fe11a5d95f1c5799273cacfc7a415d0b7786c
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Try to give as much information as possible. The connection's
response message might contain additional hints as to why the
connection could not be established.
Bug: 536541
Change-Id: I7230e4e0be9417be8cedeb8aaab35186fcbf00a5
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
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>
TransportHttp: retry on IOException with another mechanism
When a 401 occurs on POST and the server advertises Negotiate, we
may get an exception from GSSAPI if the client isn't configured
at all for Kerberos.
Add exception logic similar to the GET case: keep trying other
authentication mechanisms if this occurs.
Bug: 501167
Change-Id: Ic3a3368378d4b3408a35aec93e78ef425d54b3e4
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
SystemReader.openUserConfig() does not load the config yet; an
explicit StoredConfig.load() is needed.
Bug: 374703
Change-Id: I1c397e2fb1a07ac4d9de3675d996417734ff90e9
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
When a https connection could not be established because the SSL
handshake was unsuccessful, TransportHttp would unconditionally
throw a TransportException.
Other https clients like web browsers or also some SVN clients
handle this more gracefully. If there's a problem with the server
certificate, they inform the user and give him a possibility to
connect to the server all the same.
In git, this would correspond to dynamically setting http.sslVerify
to false for the server.
Implement this using the CredentialsProvider to inform and ask the
user. We offer three choices:
1. skip SSL verification for the current git operation, or
2. skip SSL verification for the server always from now on for
requests originating from the current repository, or
3. always skip SSL verification for the server from now on.
For (1), we just suppress SSL verification for the current instance of
TransportHttp.
For (2), we store a http.<uri>.sslVerify = false setting for the
original URI in the repo config.
For (3), we store the http.<uri>.sslVerify setting in the git user
config.
Adapt the SmartClientSmartServerSslTest such that it uses this
mechanism instead of setting http.sslVerify up front.
Improve SimpleHttpServer to enable setting it up also with HTTPS
support in anticipation of an EGit SWTbot UI test verifying that
cloning via HTTPS from a server that has a certificate that doesn't
validate pops up the correct dialog, and that cloning subsequently
proceeds successfully if the user decides to skip SSL verification.
Bug: 374703
Change-Id: Ie1abada9a3d389ad4d8d52c2d5265d2764e3fb0e
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Git has a rather elaborate mechanism to specify HTTP configuration
options per URL, based on pattern matching the URL against "http"
subsection names.[1] The URLs used for this matching are always the
original URLs; redirected URLs do not participate.
* Scheme and host must match exactly case-insensitively.
* An optional user name must match exactly.
* Ports must match exactly after default ports have been filled in.
* The path of a subsection, if any, must match a segment prefix of
the path of the URL.
* Matches with user name take precedence over equal-length path
matches without, but longer path matches are preferred over
shorter matches with user name.
Implement this for JGit. Factor out the HttpConfig from TransportHttp
and implement the matching and override mechanism.
The set of supported settings is still the same; JGit currently
supports only followRedirects, postBuffer, and sslVerify, plus the
JGit-specific maxRedirects key.
Add tests for path normalization and prefix matching only on segment
separators, and use the new mechanism in SmartClientSmartServerSslTest
to disable sslVerify selectively for only the test server URLs.
Compare also bug 374703 and bug 465492. With this commit it would be
possible to set sslVerify to false for only the git server using a
self-signed certificate instead of having to switch it off globally
via http.sslVerify.
[1] https://git-scm.com/docs/git-config
Change-Id: I42a3c2399cb937cd7884116a2a32fcaa7a418fcb
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Cleanup: message reporting for HTTP redirect handling
The addition of "tooManyRedirects" in commit 7ac1bfc ("Do
authentication re-tries on HTTP POST") was an error I didn't
catch after rebasing that change. That message had been renamed
in the earlier commit e17bfc9 ("Add support to follow HTTP
redirects") to "redirectLimitExceeded".
Also make sure we always use the TransportException(URIish, ...)
constructor; it'll prefix the message given with the sanitized URI.
Change messages to remove the explicit mention of that URI inside the
message. Adapt tests that check the expected exception message text.
For the info logging of redirects, remove a potentially present
password component in the URI to avoid leaking it into the log.
Change-Id: I517112404757a9a947e92aaace743c6541dce6aa
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
There is at least one git server out there (GOGS) that does
not require authentication on the initial GET for
info/refs?service=git-receive-pack but that _does_ require
authentication for the subsequent POST to actually do the push.
This occurs on GOGS with public repositories; for private
repositories it wants authentication up front.
Handle this behavior by adding 401 handling to our POST request.
Note that this is suboptimal; we'll re-send the push data at
least twice if an authentication failure on POST occurs. It
would be much better if the server required authentication
up-front in the GET request.
Added authentication unit tests (using BASIC auth) to the
SmartClientSmartServerTest:
- clone with authentication
- clone with authentication but lacking CredentialsProvider
- clone with authentication and wrong password
- clone with authentication after redirect
- clone with authentication only on POST, but not on GET
Also tested manually in the wild using repositories at try.gogs.io.
That server offers only BASIC auth, so the other paths
(DIGEST, NEGOTIATE, fall back from DIGEST to BASIC) are untested
and I have no way to test them.
* public repository: GET unauthenticated, POST authenticated
Also tested after clearing the credentials and then entering a
wrong password: correctly asks three times during the HTTP
POST for user name and password, then gives up.
* private repository: authentication already on GET; then gets
applied correctly initially to the POST request, which succeeds.
Also fix the authentication to use the credentials for the redirected
URI if redirects had occurred. We must not present the credentials
for the original URI in that case. Consider a malicious redirect A->B:
this would allow server B to harvest the user credentials for server
A. The unit test for authentication after a redirect also tests for
this.
Bug: 513043
Change-Id: I97ee5058569efa1545a6c6f6edfd2b357c40592a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
git-core follows HTTP redirects so JGit should also provide this.
Implement config setting http.followRedirects with possible values
"false" (= never), "true" (= always), and "initial" (only on GET, but
not on POST).[1]
We must do our own redirect handling and cannot rely on the support
that the underlying real connection may offer. At least the JDK's
HttpURLConnection has two features that get in the way:
* it does not allow cross-protocol redirects and thus fails on
http->https redirects (for instance, on Github).
* it translates a redirect after a POST to a GET unless the system
property "http.strictPostRedirect" is set to true. We don't want
to manipulate that system setting nor require it.
Additionally, git has its own rules about what redirects it accepts;[2]
for instance, it does not allow a redirect that adds query arguments.
We handle response codes 301, 302, 303, and 307 as per RFC 2616.[3]
On POST we do not handle 303, and we follow redirects only if
http.followRedirects == true.
Redirects are followed only a certain number of times. There are two
ways to control that limit:
* by default, the limit is given by the http.maxRedirects system
property that is also used by the JDK. If the system property is
not set, the default is 5. (This is much lower than the JDK default
of 20, but I don't see the value of following so many redirects.)
* this can be overwritten by a http.maxRedirects git config setting.
The JGit http.* git config settings are currently all global; JGit has
no support yet for URI-specific settings "http.<pattern>.name". Adding
support for that is well beyond the scope of this change.
Like git-core, we log every redirect attempt (LOG.info) so that users
may know about the redirection having occurred.
Extends the test framework to configure an AppServer with HTTPS support
so that we can test cloning via HTTPS and redirections involving HTTPS.
[1] https://git-scm.com/docs/git-config
[2] 6628eb41db
[3] https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html
CQ: 13987
Bug: 465167
Change-Id: I86518cb76842f7d326b51f8715e3bbf8ada89859
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
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
Enable and fix warnings about redundant specification of type arguments
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>
Enable and fix 'Should be tagged with @Override' warning
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>
Decide whether to "Accept-Encoding: gzip" on a request-by-request basis
When the reply is already compressed (e.g. a packfile fetched using dumb
HTTP), "Content-Encoding: gzip" wastes bandwidth relative to sending the
content raw. So don't "Accept-Encoding: gzip" for such requests.
Change-Id: Id25702c0b0ed2895df8e9790052c3417d713572c
Signed-off-by: Zhen Chen <czhen@google.com>
dump HTTP: Avoid being confused by Content-Length of a gzipped stream
TransportHttp sets 'Accept-Encoding: gzip' to allow the server to
compress HTTP responses. When fetching a loose object over HTTP, it
uses the following code to read the response:
InputStream in = openInputStream(c);
int len = c.getContentLength();
return new FileStream(in, len);
If the content is gzipped, openInputStream decompresses it and produces
the correct content for the object. Unfortunately the Content-Length
header contains the length of the compressed stream instead of the
actual content length. Use a length of -1 instead since we don't know
the actual length.
Loose objects are already compressed, so the gzip encoding typically
produces a longer compressed payload. The value from the Content-Length
is too high, producing EOFException: Short read of block.
Change-Id: I8d5284dad608e3abd8217823da2b365e8cd998b0
Signed-off-by: Zhen Chen <czhen@google.com>
Helped-by: Jonathan Nieder <jrn@google.com>
http transport does not use authentication fallback
Git servers supporting HTTP transport can send multiple WWW-Authenticate
challenges [1] for different authentication schemes the server supports.
If authentication fails now retry all authentication types proposed by
the server.
[1] https://tools.ietf.org/html/rfc2617#page-3
Bug: 492057
Change-Id: I01d438a5896f9b1008bd6b751ad9c7cbf780af1a
Signed-off-by: Christian Pontesegger <christian.pontesegger@web.de>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Allow to reuse disableSslVerify method, move it to HttpSupport
The disableSslVerify method will be used in the follow up change.
Change-Id: Ie00b5e14244a9a036cbdef94768007f1c25aa8d3
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
[performance] Remove synthetic access$ methods in transport package
Java compiler must generate synthetic access methods for private methods
and fields of the enclosing class if they are accessed from inner
classes and vice versa.
While invisible in the code, those synthetic access methods exist in the
bytecode and seem to produce some extra execution overhead at runtime
(compared with the direct access to this fields or methods), see
https://git.eclipse.org/r/58948/.
By removing the "private" access modifier from affected methods and
fields we help compiler to avoid generation of synthetic access methods
and hope to improve execution performance.
To validate changes, one can either use javap or use Bytecode Outline
plugin in Eclipse. In both cases one should look for "synthetic
access$<number>" methods at the end of the class and inner class files
in question - there should be none.
NB: don't mix this "synthetic access$" methods up with "public synthetic
bridge" methods generated to allow generic method override return types.
Change-Id: I0ebaeb2bc454cd8051b901addb102c1a6688688b
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
Since git-core ff5effd (v1.7.12.1) the native wire protocol transmits
the server and client implementation and version strings using
capability "agent=git/1.7.12.1" or similar.
Support this in JGit and hang the implementation data off UploadPack
and ReceivePack. On HTTP transports default to the User-Agent HTTP
header until the client overrides this with the optional capability
string in the first line.
Extract the user agent string into a UserAgent class under transport
where it can be specified to a different value if the application's
build process has broken the Implementation-Version header in the
JGit package.
Change-Id: Icfc6524d84a787386d1786310b421b2f92ae9e65
Do not use .netrc implicitly if no CredentialsProvider was set
Do not silently set the NetRCCredentialsProvider if no
CredentialsProvider was set explicitly since applications may want to
have full control which provider should be used.
Bug: 444338
Change-Id: Ie096983bc1caa90443a504d302bfea8f2d26ab9e
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Prevent NPE if no CredentialsProvider is registered
If the git server requires authentication and no CredentialsProvider is
registered TransportHttp.connect() would throw an NPE since it tries to
reset the credentials provider. Instead throw a TransportException
explaining the problem.
Change-Id: Ib274e7d9c43bba301089975423de6a05ca5169f6
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Creates HttpAuthMethod type enum to support auth ordering
Refactors HttpAuthMethod to support more authentication methods,
still sorted by priority orders.
Bug: 428836
Change-Id: I049c1742e7afbc51f3f6033fa4d471b344813cfa
Signed-off-by: Laurent Goujon <lgoujon@twitter.com>
Signed-off-by: Chris Aniszczyk <caniszczyk@gmail.com>
Detects background authentication and force use of jgit authentication
Sun HttpURLConnection is able to handle authentication like SPNEGO without
caller intervention. However, there are some restrictions:
- do not need user direct input (user,password for example)
- it doesn't work when request body is chunked/streamed (because it cannot be
replayed)
Unfortunately there is no real way to leverage HttpURLConnection authentication
work as the authentication header is stripped off the request before returning
to the caller. There's also no way to explicitly disable authentication in
HttpURLConnection (SPNEGO auth will always be attempted if a valid token can be
created by GSSAPI).
This is an issue for jgit since it is expected that the first request will be
used to detect authentication method, and reuse for the subsequent requests.
This patch modifies TransportHTTP to detect authentication done in the background
by HttpURLConnection and sets the jgit authentication method accordingly so it will
always work for future requests (assuming that the authentication method used by
HttpURLConnection is also supported by jgit).
Bug: 428836
Change-Id: I79f3b70ca2b8377e20da8e6a01914e43e96595ce
Signed-off-by: Laurent Goujon <lgoujon@twitter.com>
Signed-off-by: Chris Aniszczyk <caniszczyk@gmail.com>
Ensure that stored credentials aren't reset too early
Some commands are started without showing a dialog allowing to enter
credentials if needed. Hence we need to tolerate one failing HTTP
authentication to trigger loading credentials from the secure store.
Hence we should not immediately reset the stored credentials if the
first attempt to authenticate fails.
Bug: 431209
Change-Id: I1b9fa34c3d70be226bb1c59c9ebe995998d29bc8
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Retry to call credentials provider if http authentication failed
If the user provided wrong credentials or credentials changed we
shouldn't give up immediately but retry to get valid credentials from
the credentials provider. Reset the credentials provider if
authentication failed to avoid it reuses wrong credentials in
case it stored them in a persistent store.
Bug: 338048
Bug: 342592
Bug: 427735
Change-Id: Ibd62ef3da17be6454991c43f524c8bbc7ca3c37e
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Previously all HTTP communication was done with the help of
java.net.HttpUrlConnection. In order to make JGit usable in environments
where the direct usage of such connections is not allowed but where the
environment provides other means to get network connections an
abstraction for connections is introduced. The idea is that new
implementations of this interface will be introduced which will not use
java.net.HttpUrlConnection but use e.g.
org.apache.client.http.HttpClient to provide network connections.
One example: certain cloud infrastructures don't allow that components
in the cloud communicate directly with HttpUrlConnection. Instead they
provide services where a component can ask for a connection (given a
symbolic name for the destination) and where the infrastructure returns
a preconfigured org.apache.http.client.HttpClient. In order to allow
JGit to be running in such environments we need the abstraction
introduced in this commit.
Change-Id: I3b06629f90a118bd284e55bb3f6465fe7d10463d
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This allows subclasses to configure the HTTP connection (for example,
to add headers to the request).
Bug: 400724
Change-Id: I6f9d699e158a7b9d813c8fa8d273992a28994e41
Signed-off-by: Michael Nelson <michael.nelson@tasktop.com>
Signed-off-by: Chris Aniszczyk <caniszczyk@gmail.com>
Allow users to provide their OutputStream (via Transport#
push(monitor, refUpdates, out)) so that server messages can be written
to it (in SideBandInputStream) while they're coming in.
CQ: 7065
Bug: 398404
Change-Id: I670782784b38702d52bca98203909aca0496d1c0
Signed-off-by: Andre Dietisheim <andre.dietisheim@gmail.com>
Signed-off-by: Chris Aniszczyk <zx@twitter.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
JGit 3.0: move internal classes into an internal subpackage
This breaks all existing callers once. Applications are not supposed
to build against the internal storage API unless they can accept API
churn and make necessary updates as versions change.
Change-Id: I2ab1327c202ef2003565e1b0770a583970e432e9
A few classes such as Constanrs are marked with @SuppressWarnings, as are
toString() methods with many liternal, but otherwise $NLS-n$ is used for
string containing text that should not be translated. A few literals may
fall into the gray zone, but mostly I've tried to only tag the obvious
ones.
Change-Id: I22e50a77e2bf9e0b842a66bdf674e8fa1692f590
The value of -1 is the default value used by the underlying http
transports provided by the jre. On some versions an attempt to
set the timeout explicitly to -1 triggers a check condition,
disallowing negative numbers.
Bug: 389003
Change-Id: I74a22f8edc6c8e15843ad07c96a137739d9dcad1
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Split Service into MultiRequestService (fetch, push) and
LongPollService (upcoming publish-subscribe).
Change-Id: Ice373d3dee63c395490d2707473ccf20a022e5cf
Add Transport URI constructor without a repository
Let a Transport instance be opened with only a URI, for use in the
upcoming publish-subscribe feature.
Change-Id: I391c60c10d034b5c1c0ef19b1f24a9ba76b17bb5
Use Integer, Character, and Long valueOf methods when
passing parameters to MessageFormat and other places
that expect objects instead of primitives
Change-Id: I5942fbdbca6a378136c00d951ce61167f2366ca4
Enable smart HTTP transport to place EOF at end of pack
When fetching over smart HTTP the InputStream that gets fed into
a PackParser doesn't really support EOF at the end of the pack. It
instead tries to make a new HTTP request, which fails because there
is no request body currently buffered by the client.
Make EOF work correctly on the end of an HTTP derived InputStream
for the pack by denoting no more requests are expected as the higher
level code is now consuming the pack (or side-band embedded pack).
Smart HTTP support doesn't automatically enqueue execute support onto
the end of the UnionInputStream, which allows the UnionInputStream
to correctly reflect EOF when the HTTP response is consumed.
Change-Id: I975f1ab1c81ab1c1af925716970088bc7b8d6b1a
The client's use of UnionInputStream was broken when combined with a
8192 byte buffer used by PackParser. A smart HTTP client connection
always pushes in the execute stateless RPC input stream after the
data stream has ended from the remote peer. At the end of the pack,
PackParser asked to fill a 8192 byte buffer, but if only e.g. 1000
bytes remained UnionInputStream went to the next stream and asked
it for input, which triggered a new RPC, and failed because there
was nothing pending in the request buffer.
Change UnionInputStream to only return what it consumed from a
single InputStream without invoking the next InputStream, just in
case that second InputStream happens to be one of these magical
ones that generates an RPC invocation.
Change-Id: I0e51a8e6fea1647e4d2e08ac9cfc69c2945ce4cb
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
TransportProtocol: Allow null Repository in canHandle()
This allows callers to determine if a URI is supported, before
worrying about the local repository.
Suggested-by: Dariusz Luksza <dariusz@luksza.org>
Change-Id: Ifc76a4ba841f2e2e7354bd51306b87b3b9d7f6ab
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Chris Aniszczyk <caniszczyk@gmail.com>
Matthias Sohn
David Pursehouse
Thomas Wolf
Han-Wen Nienhuys
Dave Borowitz
Bo Zhang
Zhen Chen
Christian Pontesegger
Saša Živkov
Andrey Loskutov
Shawn Pearce
Robin Rosenberg
Alexey Kuznetsov
Laurent Goujon
Bogdan Gheorghe
Christian Halstrick
Michael Nelson
André Dietisheim
Jason Pyeron
Ian Wetherbee
Kevin Sawicki