From 373794473a8a7cf6f6efe8f07b8363d58a9a44a7 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 3 Feb 2015 09:03:55 -0800 Subject: [PATCH 1/4] RestApiServlet: Flush pending padding as well This issue was found by scan.coverity.com (CID 19911) which is a static code analysis tool, free for open source code. Originally it was classified as a resource leak, but this is a miss classification as the auto closing of the OutputStream will flush any pending padding. Change-Id: I4dc2d1cd9f52740490fda7c37e98b115fa59ec3a --- .../com/google/gerrit/httpd/restapi/RestApiServlet.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index b7528b5de4..1279339215 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -740,10 +740,11 @@ public class RestApiServlet extends HttpServlet { b64 = new BinaryResult() { @Override public void writeTo(OutputStream out) throws IOException { - OutputStream e = BaseEncoding.base64().encodingStream( - new OutputStreamWriter(out, ISO_8859_1)); - src.writeTo(e); - e.flush(); + try (OutputStreamWriter w = new OutputStreamWriter(out, ISO_8859_1); + OutputStream e = BaseEncoding.base64().encodingStream(w)) { + src.writeTo(e); + e.flush(); + } } }; } From a92bcf416c11fa69761bcc91cb5915621e16ce89 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Wed, 4 Feb 2015 17:12:37 -0800 Subject: [PATCH 2/4] RestApiServlet: Leave OutputStream open when flushing base64 padding Some Java servlet containers fail if the response's OutputStream is closed twice by the application. This appears to contradict standard behavior in Java where most streams gracefully ignore extra close. Unfortunately the container is required to power gerrit-review and as such Gerrit needs to try to tolerate its behavior. Wrap the supplied OutputStream delegating all calls except for close(). No-op the close() method so the Java 7 try-with-resources block does not automatically close the servlet OutputStream, leaving this for the caller's finally block. Change-Id: I84bd3c8031580f805d5d4ef5d70f09b89e170450 --- .../google/gerrit/httpd/restapi/RestApiServlet.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index 1279339215..45144d7179 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -103,6 +103,7 @@ import org.slf4j.LoggerFactory; import java.io.BufferedReader; import java.io.BufferedWriter; import java.io.EOFException; +import java.io.FilterOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -740,10 +741,15 @@ public class RestApiServlet extends HttpServlet { b64 = new BinaryResult() { @Override public void writeTo(OutputStream out) throws IOException { - try (OutputStreamWriter w = new OutputStreamWriter(out, ISO_8859_1); + try (OutputStreamWriter w = new OutputStreamWriter( + new FilterOutputStream(out) { + @Override + public void close() { + // Do not close out, but only w and e. + } + }, ISO_8859_1); OutputStream e = BaseEncoding.base64().encodingStream(w)) { src.writeTo(e); - e.flush(); } } }; From 3b6c86cb621c694f6b67075be4ce3453eae6b9a6 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sat, 25 Apr 2015 12:33:28 +0200 Subject: [PATCH 3/4] Hybrid OpenID/OAuth: Check for session validity during logout GitHub-Bug: https://github.com/davido/gerrit-oauth-provider/issues/9 Change-Id: I17aaed508ef61959a3fc5634d76eb5386305f9a0 --- .../httpd/auth/openid/OAuthOverOpenIDLogoutServlet.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OAuthOverOpenIDLogoutServlet.java b/gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OAuthOverOpenIDLogoutServlet.java index 8ca71ff858..8fad0ad3c9 100644 --- a/gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OAuthOverOpenIDLogoutServlet.java +++ b/gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OAuthOverOpenIDLogoutServlet.java @@ -52,6 +52,8 @@ class OAuthOverOpenIDLogoutServlet extends HttpLogoutServlet { protected void doLogout(HttpServletRequest req, HttpServletResponse rsp) throws IOException { super.doLogout(req, rsp); - oauthSession.get().logout(); + if (req.getSession(false) != null) { + oauthSession.get().logout(); + } } } From 18c4ccd2c335691eff1202a4facd5dde3adcf05f Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sun, 26 Apr 2015 18:52:45 +0200 Subject: [PATCH 4/4] Bump JGit version to 3.7.1.201504261725-r This version fixed JGit regression, causing severe (>10x) performance degradation on huge repositories (>2GB) on git push and CPU consumption explosion during replication: [1]. [1] https://bugs.eclipse.org/bugs/show_bug.cgi?id=465509 Bug: Issue 3300 Change-Id: I6b1fa985fa3738801d3fa27d690a1c02c1afc1db --- lib/jgit/BUCK | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/jgit/BUCK b/lib/jgit/BUCK index a658fc36e7..ff7388868e 100644 --- a/lib/jgit/BUCK +++ b/lib/jgit/BUCK @@ -1,13 +1,13 @@ include_defs('//lib/maven.defs') -REPO = GERRIT # Leave here even if set to MAVEN_CENTRAL. -VERS = '3.7.0.201502260915-r.58-g65c379e' +REPO = MAVEN_CENTRAL # Leave here even if set to MAVEN_CENTRAL. +VERS = '3.7.1.201504261725-r' maven_jar( name = 'jgit', id = 'org.eclipse.jgit:org.eclipse.jgit:' + VERS, - bin_sha1 = '8fc9620ec499169facad3355f7417eb6a8aff511', - src_sha1 = '40bd9ae8af8e0b03eb4e43f44f5feda8b7325221', + bin_sha1 = '28bae05826c1a34381826fb1d9ea5fd0ec47cc67', + src_sha1 = 'ef46620a8981d49eef2b27090ada4ca80a2b9766', license = 'jgit', repository = REPO, unsign = True, @@ -22,7 +22,7 @@ maven_jar( maven_jar( name = 'jgit-servlet', id = 'org.eclipse.jgit:org.eclipse.jgit.http.server:' + VERS, - sha1 = 'cecc2b9c0b94455348c3a0c63eb83f72cc595757', + sha1 = '874137fe9417c23cd0225ee6b86a13366b58b16e', license = 'jgit', repository = REPO, deps = [':jgit'], @@ -36,7 +36,7 @@ maven_jar( maven_jar( name = 'jgit-archive', id = 'org.eclipse.jgit:org.eclipse.jgit.archive:' + VERS, - sha1 = '7ccc7c78bf47566045ea7a3c08508ba18e4684ca', + sha1 = '8aab2d5d8deeaa8345d6df456de215b3d973c4b2', license = 'jgit', repository = REPO, deps = [':jgit', @@ -53,7 +53,7 @@ maven_jar( maven_jar( name = 'junit', id = 'org.eclipse.jgit:org.eclipse.jgit.junit:' + VERS, - sha1 = '87d64d722447dc3971ace30d2a72593c72a4d05f', + sha1 = '7ab2c3f5f40d07c84e7305b0fd60a8006257639c', license = 'DO_NOT_DISTRIBUTE', repository = REPO, unsign = True,