From f8e514c74a039209488ef56948c784c00a1d87b3 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sat, 15 Sep 2018 21:09:51 +0200 Subject: ObjectDownloadListener: Return from onWritePossible when data is written MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../src/org/eclipse/jgit/lfs/server/fs/ObjectDownloadListener.java | 1 + 1 file changed, 1 insertion(+) (limited to 'org.eclipse.jgit.lfs.server') diff --git a/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/ObjectDownloadListener.java b/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/ObjectDownloadListener.java index cc4350090f..0b96426420 100644 --- a/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/ObjectDownloadListener.java +++ b/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/ObjectDownloadListener.java @@ -127,6 +127,7 @@ public class ObjectDownloadListener implements WriteListener { outChannel.write(buffer); } else { context.complete(); + return; } } } -- cgit v1.2.3 From 5c134f4d42e12e9192f2beca8370c1b8bd58c08d Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sat, 15 Sep 2018 21:09:51 +0200 Subject: 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 Signed-off-by: Matthias Sohn --- .../jgit/lfs/server/fs/ObjectDownloadListener.java | 32 +++++++++++++--------- 1 file changed, 19 insertions(+), 13 deletions(-) (limited to 'org.eclipse.jgit.lfs.server') diff --git a/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/ObjectDownloadListener.java b/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/ObjectDownloadListener.java index 0b96426420..60258602f1 100644 --- a/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/ObjectDownloadListener.java +++ b/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/ObjectDownloadListener.java @@ -80,7 +80,7 @@ public class ObjectDownloadListener implements WriteListener { private final WritableByteChannel outChannel; - private final ByteBuffer buffer = ByteBuffer.allocateDirect(8192); + private ByteBuffer buffer = ByteBuffer.allocateDirect(8192); /** * @param repository @@ -115,20 +115,26 @@ public class ObjectDownloadListener implements WriteListener { @Override public void onWritePossible() throws IOException { while (out.isReady()) { - if (in.read(buffer) != -1) { - buffer.flip(); - outChannel.write(buffer); - buffer.compact(); - } else { - in.close(); - buffer.flip(); - while (out.isReady()) { - if (buffer.hasRemaining()) { - outChannel.write(buffer); - } else { + try { + buffer.clear(); + if (in.read(buffer) < 0) { + buffer = null; + } else { + buffer.flip(); + } + } catch(Throwable t) { + LOG.log(Level.SEVERE, t.getMessage(), t); + buffer = null; + } finally { + if (buffer != null) { + outChannel.write(buffer); + } else { + try { + out.close(); + } finally { context.complete(); - return; } + return; } } } -- cgit v1.2.3 From c18c768678094dba36e4d394de7a673d1a8764c4 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Mon, 17 Sep 2018 00:35:33 +0200 Subject: Fix error handling in FileLfsServlet 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=9667 https://bugs.chromium.org/p/gerrit/issues/detail?id=9721 https://github.com/eclipse/jetty.project/issues/2911 Change-Id: Ie35563c2e0ac1c5e918185a746622589a880dc7f Signed-off-by: Matthias Sohn --- .../src/org/eclipse/jgit/lfs/server/fs/FileLfsServlet.java | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'org.eclipse.jgit.lfs.server') diff --git a/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/FileLfsServlet.java b/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/FileLfsServlet.java index a8e3c11e27..15c4448da8 100644 --- a/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/FileLfsServlet.java +++ b/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/FileLfsServlet.java @@ -202,6 +202,11 @@ public class FileLfsServlet extends HttpServlet { */ protected static void sendError(HttpServletResponse rsp, int status, String message) throws IOException { + if (rsp.isCommitted()) { + rsp.getOutputStream().close(); + return; + } + rsp.reset(); rsp.setStatus(status); PrintWriter writer = rsp.getWriter(); gson.toJson(new Error(message), writer); -- cgit v1.2.3 From 1a4e12a451217075310458b94a39bfc132abb276 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Tue, 18 Sep 2018 01:14:34 +0200 Subject: Fix ObjectUploadListener#close Do not try to set response status if response is already committed. Change-Id: I9a7c2871c86eb53416b905324775f3ed961c8ae6 Signed-off-by: Matthias Sohn --- .../src/org/eclipse/jgit/lfs/server/fs/ObjectUploadListener.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'org.eclipse.jgit.lfs.server') diff --git a/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/ObjectUploadListener.java b/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/ObjectUploadListener.java index 84e4e6f1c6..da86880472 100644 --- a/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/ObjectUploadListener.java +++ b/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/ObjectUploadListener.java @@ -150,7 +150,9 @@ public class ObjectUploadListener implements ReadListener { channel.close(); // TODO check if status 200 is ok for PUT request, HTTP foresees 204 // for successful PUT without response body - response.setStatus(HttpServletResponse.SC_OK); + if (!response.isCommitted()) { + response.setStatus(HttpServletResponse.SC_OK); + } } finally { context.complete(); } -- cgit v1.2.3