Fix file handle leak in ObjectDownloadListener.onWritePossible
5c134f4d removed closing the input stream when we reached end of the
stream. This caused file handle leaks.
Bug: 540049
Change-Id: I48082b537077c7471fc160f59aa04deb99687d9b
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
ObjectDownloadListener#onWritePossible: Add comment on return statement
It is not obvious why this return statement is needed. Clarify with a
comment that otherwise endless loop may show up when recent versions
of Jetty are used.
Change-Id: I8e5d4de51869fb1179bf599bfb81bcd7d745874b
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
Do not try to set response status if response is already committed.
Change-Id: I9a7c2871c86eb53416b905324775f3ed961c8ae6
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Check in #sendError method if the response was committed already.
If yes we cannot set response status or send an error message, last
resort is to close the outputstream.
If the response wasn't yet committed first reset the response before
using writer to send the error message to the client since mixing STREAM
and WRITE mode (mixing asynchronous and blocking I/O) is illegal in
servlet 3.1.
see the following bugs in the gerrit and jetty issue trackers
https://bugs.chromium.org/p/gerrit/issues/detail?id=9667https://bugs.chromium.org/p/gerrit/issues/detail?id=9721https://github.com/eclipse/jetty.project/issues/2911
Change-Id: Ie35563c2e0ac1c5e918185a746622589a880dc7f
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
ObjectDownloadListener#onWritePossible: Make code spec compatible
Current code violates the ServletOutputStream contract. For every
out.isReady() == true either write or close of that ServletOutputStream
should be called.
See also this issue upstream for more context: [1].
[1] https://github.com/eclipse/jetty.project/issues/2911
Change-Id: Ied575f3603a6be0d2dafc6c3329d685fc212c7a3
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
ObjectDownloadListener: Return from onWritePossible when data is written
When buffer was written not only call AsyncContext#complete() but also
return from the ObjectDownloadListener#onWritePossible(). This avoids
endless loop after upgrading from Jetty 9.3.x to 9.4.x lines.
In Jetty example implementation:[1] the return statemnt is also used:
// If we are at EOF then complete
if (len < 0)
{
async.complete();
return;
}
See also this issue upstream: [2].
[1] https://webtide.com/servlet-3-1-async-io-and-jetty
[2] https://github.com/eclipse/jetty.project/issues/2911
Change-Id: Iac73fb25e67d40228a378a8e34103f1d28b72a76
Signed-off-by: David Ostrovsky <david@ostrovsky.org>
Transfer data in chunks of 8k Transferring data byte per byte is slow,
running checkout with CleanFilter on a 2.9MB file takes 20 seconds.
Using a buffer of 8k shrinks this time to 70ms.
Also register the filter commands in a way that the native GIT LFS can
be used alongside with JGit.
Implements auto-discovery of LFS server URL when cloning from a Gerrit
LFS server.
Change-Id: I452a5aa177dcb346d92af08b27c2e35200f246fd
Also-by: Christian Halstrick <christian.halstrick@sap.com>
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>
Revert usage of TYPE_USE in Nullable and NonNull annotations
Using TYPE_USE causes compilation errors in Eclipse Neon.3 (JDT 3.12.3)
and Eclipse Oxygen.2 (JDT 3.13.2).
This reverts commit 8e217517e2.
This reverts commit 55eba8d0f5.
Reported-by: Thomas Wolf <thomas.wolf@paranor.ch>
Change-Id: I96869f80dd11ee238911706581b224bca4fb12cd
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Since JGit now requires Java 8, we can switch to TYPE_USE instead
of explicitly specifying the target type.
Some of the existing uses of Nullable need to be reworked slightly
as described in [1] to prevent the compilation error:
scoping construct cannot be annotated with type-use annotation
[1] https://stackoverflow.com/a/21385939/381622
Change-Id: Idba48f67a09353b5237685996ce828c8ca398168
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Pretty printing the response is useful for human readers, but most
(if not all) of the time, the response will be read by programs.
Remove it to avoid the additional overhead of the formatting and
extra bytes in the response. Adjust the test accordingly.
Note that LfsProtocolServlet already doesn't use pretty printing,
so this change makes FileLfsServlet's behavior consistent. In fact,
both classes now have duplicate Gson handling; this will be cleaned
up in a separate change.
Change-Id: I113a23403f9222f16e2c0ddf39461398b721d064
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Use constants from StandardCharsets instead of hard-coded strings
Instead of hard-coding the charset strings "US-ASCII", "UTF-8", and
"ISO-8859-1", use the corresponding constants from StandardCharsets.
UnsupportedEncodingException is not thrown when the StandardCharset
constants are used, so remove the now redundant handling.
Because the encoding names are no longer hard-coded strings, also
remove redundant $NON-NLS warning suppressions.
Also replace existing usages of the constants with static imports.
Change-Id: I0a4510d3d992db5e277f009a41434276f95bda4e
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
LfsProtocolServlet and FileLfsServlet both implement the same
setup of the Gson object.
Factor it out to a common class and reuse it.
Change-Id: I5696404fad140cbff1b712ebb04a7e8bba60e4b4
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Pretty printing the response is useful for human readers, but most
(if not all) of the time, the response will be read by programs.
Remove it to avoid the additional overhead of the formatting and
extra bytes in the response. Adjust the test accordingly.
Note that LfsProtocolServlet already doesn't use pretty printing,
so this change makes FileLfsServlet's behavior consistent. In fact,
both classes now have duplicate Gson handling; this will be cleaned
up in a separate change.
Change-Id: I113a23403f9222f16e2c0ddf39461398b721d064
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
LfsProtocolServlet: Pass HTTP Authorization header to getLargeFileRepository
This allows implementations to reject operations that do not
include proper authentication.
Change-Id: If301476d8fb56a0899e424be3789c7576097d185
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
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>
Don't rely on default locale when using toUpperCase() and toLowerCase()
Otherwise these methods may produce unexpected results if used for
strings that are intended to be interpreted locale independently.
Examples are programming language identifiers, protocol keys, and HTML
tags. For instance, "TITLE".toLowerCase() in a Turkish locale returns
"t\u0131tle", where '\u0131' is the LATIN SMALL LETTER DOTLESS I
character.
See
https://docs.oracle.com/javase/8/docs/api/java/lang/String.html#toLowerCase--http://blog.thetaphi.de/2012/07/default-locales-default-charsets-and.html
Bug: 511238
Change-Id: Id8d8f37d84d62239c918b81f8d883ed798d87656
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
The same was already done for ObjectUploadListener in I5b0fb1220.
Change-Id: Ie8a79f715fe69bed9331962fb478f7e35acf8680
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
LfsProtocolServlet#LfsRequest: Add operation type helper methods
Server implementations may need to check what type of operation is
being performed. Add helper methods to make it a bit simpler.
Change-Id: Ia33ed6699ebcb9000ef514ce79bcddbebf94e1c5
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
So that they can be used either by third party LFS server implementations
or from within other parts of JGit's internal LFS implementation.
Change-Id: I58da6230247204588e017c010ce5b14e4615448d
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
LfsProtocolServlet: Improve error on getLargeFileRepository failure
Externalize the error message and make it more specific. Also emit
an error log.
Change-Id: Ie7b1c90c54673bfb8e133fb0aa19a117d4ca6587
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Add support for refusing LFS request due to invalid authorization
Add a new exception type that server implementations can throw when a
client attempts to make an unauthorized LFS operation, which will result
in HTTP 401 Unauthorized being returned to the client.
An example of this is a Gerrit server that rejects a request to perform
an LFS operation on a ref that is not visible to the caller.
As defined in the LFS spec [1] the request may include authentication,
and per RFC 2616 [2], "401 response indicates that authorization has been
refused for those credentials".
[1] https://github.com/git-lfs/git-lfs/blob/master/docs/api/batch.md
[2] https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html
Change-Id: I2aa22e2144df5fb7972df0e3285b77b08ecc63f2
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Expose getObjectToTransfer method of FileLfsServlet
Providing own implementation to doGet/doPut methods is troublesome when
this method is private.
Change-Id: I098cdc5cb90410eaaebc56c88c2d9e168584dd6d
Signed-off-by: Jacek Centkowski <geminica.programs@gmail.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Use the same variable to check and extract LFS object id
It is easier to maintain when the same variable is used for both check
and extraction of LFS object id.
Change-Id: I5406f9bc4a085aa164c4565a9667ad2925105190
Signed-off-by: Jacek Centkowski <geminica.programs@gmail.com>
The class AtomicObjectOutputStream should be available to all lfs
related classes, not only to the server side. Move the class from
org.eclipse.jgit.lfs.server.fs to org.eclipse.jgit.lfs.internal to
achieve that.
Change-Id: I028e1c9ec7c21f316340b21d558b9a6b77e2060d
Improve JavaDoc for LfsProtocolServlet.getLargeFileRepository
Guide implementors which exception to throw in case of errors.
Change-Id: I74fb76cdf6b7cdef513f3fe8c144572e869cc533
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Instead of returning null, LfsProtocolServlet#getLargeFileRepository
should throw LfsUnavailable.
If null is returned, throw a generic LfsException.
Handle LfsException as an internal server error and return HTTP 500.
Change-Id: I33e2a19fcc0fde8aaf0f703860c8fa8ce2de2db5
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
LfsProtocolServlet: Add support for insufficient storage error
Since [1], the git-lfs specification allows the server to return
HTTP 507 if there is insufficient storage for the uploaded object(s).
Add a new exception class, which implementations may throw from the
getRepository() method, causing HTTP 507 to be returned to the client.
[1] https://github.com/github/git-lfs/pull/1473
Change-Id: If5bc0a35fcf870d4216af6ca2f7c8924689ef9c5
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
LfsProtocolServlet: Add support for rate limit and bandwidth limit errors
The git-lfs specification [1] describes the following optional status codes
that may be returned:
429 - The user has hit a rate limit with the server. Though the API does
not specify any rate limits, implementors are encouraged to set some
for availability reasons.
509 - Returned if the bandwidth limit for the user or repository has been
exceeded. The API does not specify any bandwidth limit, but implementors
may track usage.
Add two new exception classes to represent these cases. Implementations may
throw these from #getLargeFileRepository(), causing the corresponding HTTP
status codes to be returned to the client.
[1] https://github.com/github/git-lfs/blob/master/docs/api/v1/http-v1-batch.md
Change-Id: I7b93f3cf90f7344c90b1587e07927fdeb167097e
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
LfsProtocolServlet: Always include message in error response
If the message is not sent, the client shows:
Unable to parse HTTP response for POST http://admin@localhost:8080/test-project/info/lfs/objects/batch
Change-Id: I8b72d1aded2bcd41b7389676e2373034625a1379
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
LfsProtocolServlet: Always set the Content-Type header on response
If the Content-Type is not set on error responses, the git-lfs client
does not read the body which contains the error message, and instead
just displays a generic error message.
Also set the charset on the Content-Type header.
Change-Id: I88e6f07f20b622a670e7c5063145dffb8b630aee
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
LfsProtocolServlet: Don't set pretty printing on Gson
Pretty printing is only used for outputting json content, which is
interpreted by the client and does not need to be pretty printed.
Change-Id: I48e0280241b6b0f5706300ae0f4c9bc461a89110
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
LfsProtocolServlet: Reuse existing Writer when sending error response
Trying to open a new writer on the response causes an illegal state
exception and the response is not sent.
Change-Id: Ic718d23cfb3e74f5691cc2aea7283003af7df207
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
LfsProtocolServlet: Allow access to objects in request
Classes implementing the LFS servlet should be able to inspect the
objects given in the request.
Add a getObjects method. Make the LfsObject class public, and add
accessor methods.
Change-Id: I27961679f620eb3a89dc8521aadd4ea2f936c60e
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
LfsProtocolServlet: Allow getLargeFileRepository to raise exceptions
According to the specification [1] the server may return the following
HTTP error responses:
- 403: The user has read, but not write access.
- 404: The repository does not exist for the user.
- 422: Validation error with one or more of the objects in the request.
In the current implementation, however, getLargeFileRepository can only
return null to indicate an error. This results in the error code:
- 503: Service Unavailable
being returned to the client regardless of what the actual reason was.
Add exception classes to cover these cases, derived from a common base
exception, and change the specification of getLargeFileRepository to throw
the base exception.
In LfsProtocolServlet#post, handle the new exceptions and send back the
appropriate HTTP responses as mentioned above.
The specification also mentions several other optional response codes (406,
429, 501, and 509) but these are not implemented in this commit. It should
be trivial to implement them in follow-up commits.
[1] https://github.com/github/git-lfs/blob/master/docs/api/v1/http-v1-batch.md#response-errors
Change-Id: I91be6165bcaf856d0cefc533882330962e2fc9b2
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
LfsProtocolServlet: Pass request and path to getLargeFileRepository
Passing the request and path to the method will allow implementations
to have more control over determination of the backend, for example:
- return different backends for different requests
- accept or refuse requests based on request characteristics
- etc
Change-Id: I1ec6ec54c91a5f0601b620ed18846eb4a3f46783
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
According to the specification [1], the error response status code
should be 422 when there is a validation error with one or more of
the objects in the request
[1] https://github.com/github/git-lfs/blob/master/docs/api/v1/http-v1-batch.md#response-errors
Change-Id: Id03fe00a2109b896d9a154228a14a33bce5accc3
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Matthias Sohn
David Ostrovsky
Han-Wen Nienhuys
David Pursehouse
Markus Duft
Jacek Centkowski
Christian Halstrick