Merge tag 'v2.4.1'

gerrit 2.4.1

* tag 'v2.4.1':
  Gerrit 2.4.1 ReleaseNotes
  Make logging events discardable, prevent NPE in AsyncAppender thread.
  Display proper error if file diff fails because content is too large
  Make async logging buffer size configurable.
  Make sure the "Object too large..." error is printed to the client

Conflicts:
	gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptFactory.java

Change-Id: Ifd6b7ba9d3b099576fab97d7d739beef7949d045
Signed-off-by: Sasa Zivkov <sasa.zivkov@sap.com>

Documentation/config-gerrit.txt

@@ -854,6 +854,15 @@ unused mapped spaces fast enough.
 Default on JGit is false. Although potentially slower, it yields
 much more predictable behavior.
 
+[[core.asyncLoggingBufferSize]]core.asyncLoggingBufferSize::
++
+Size of the buffer to store logging events for asynchronous logging.
+Putting a larger value can protect threads from stalling when the
+AsyncAppender threads are not fast enough to consume the logging events
+from the buffer. It also protects from loosing log entries in this case.
++
+Default is 64 entries.
+
 [[database]]Section database
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

ReleaseNotes/ReleaseNotes-2.4.1.txt

@@ -0,0 +1,55 @@
+Release notes for Gerrit 2.4.1
+==============================
+
+Gerrit 2.4.1 is now available:
+
+link:http://code.google.com/p/gerrit/downloads/detail?name=gerrit-2.4.1.war[http://code.google.com/p/gerrit/downloads/detail?name=gerrit-2.4.1.war]
+
+
+There are no schema changes from 2.4.  However, if upgrading from
+anything but 2.4, follow the upgrade procedure in the 2.4
+link:ReleaseNotes-2.4.html[ReleaseNotes].
+
+
+Bug Fixes
+---------
+* Catch all exceptions when async emailing
++
+This fixes email notification issues reported
+link:https://groups.google.com/group/repo-discuss/browse_thread/thread/dd157ebc55b962ef/652822d6fbe61e71[here].
+
+* Fixed cleanup of propagated SshScopes
++
+This improves error reporting in case of email notification errors.
+
+* issue 1394 Fix lookup of the 'Commit Message' file in patch set
++
+There is an assumption that the commit message is always first in the list of
+files of a patch set. However, there was another place in Gerrit code, which
+did binary search through the list of the files, without taking this assumption
+into account. In case when a patch set contained a file which lexicographically
+sorted before '/COMMIT_MSG' (like '.gitignore' for example) it could have
+happened that the commit message was not found and, as a side effect, it wasn't
+possible to review it.
+
+* issue 1162 Fix deadlock on destroy of CommandFactoryProvider
+
+* Honor the sendmail.smtpUser from gerrit.config on upgrade
++
+If sendmail.smtpUser was not present in the gerrit.config then don't set it in
+site upgrade.
+
+* issue 1420 Forge committer bypassed
++
+It was possible to forge committer even without having permission for that.
+This was a regression from 2.3.
+
+* Make sure the "Object too large..." error message is printed when an object
+larger than receive.maxObjectSizeLimit is rejected by Gerrit
+
+* Display proper error if file diff fails because content is too large
+
+* Get around a log4j bug that causes AsyncAppender-Dispatcher thread to die and
+block other threads
+** Make async logging buffer size configurable
+** Make logging events discardable, prevent NPE in AsyncAppender-Dispatcher thread

ReleaseNotes/index.txt

@@ -4,6 +4,7 @@ Gerrit Code Review - Release Notes
 [[2_4]]
 Version 2.4.x
 -------------
+* link:ReleaseNotes-2.4.1.html[2.4.1]
 * link:ReleaseNotes-2.4.html[2.4]
 
 [[2_3]]

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptFactory.java

@@ -31,6 +31,7 @@ import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountInfoCacheFactory;
 import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.LargeObjectException;
 import com.google.gerrit.server.patch.PatchList;
 import com.google.gerrit.server.patch.PatchListCache;
 import com.google.gerrit.server.patch.PatchListEntry;
@@ -121,7 +122,8 @@ class PatchScriptFactory extends Handler<PatchScript> {
   }
 
   @Override
-  public PatchScript call() throws OrmException, NoSuchChangeException {
+  public PatchScript call() throws OrmException, NoSuchChangeException,
+      LargeObjectException {
     validatePatchSetId(psa);
     validatePatchSetId(psb);
 
@@ -161,6 +163,8 @@ class PatchScriptFactory extends Handler<PatchScript> {
     } catch (IOException e) {
       log.error("File content unavailable", e);
       throw new NoSuchChangeException(changeId, e);
+    } catch (org.eclipse.jgit.errors.LargeObjectException err) {
+      throw new LargeObjectException("File content is too large", err);
     } finally {
       git.close();
     }

gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/HttpLog.java

@@ -30,6 +30,7 @@ import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.RequestLog;
 import org.eclipse.jetty.server.Response;
 import org.eclipse.jetty.util.component.AbstractLifeCycle;
+import org.eclipse.jgit.lib.Config;
 
 import java.io.File;
 import java.io.IOException;
@@ -54,7 +55,7 @@ class HttpLog extends AbstractLifeCycle implements RequestLog {
 
   private final AsyncAppender async;
 
-  HttpLog(final SitePaths site) {
+  HttpLog(final SitePaths site, final Config config) {
     final DailyRollingFileAppender dst = new DailyRollingFileAppender();
     dst.setName(LOG_NAME);
     dst.setLayout(new MyLayout());
@@ -69,7 +70,7 @@ class HttpLog extends AbstractLifeCycle implements RequestLog {
 
     async = new AsyncAppender();
     async.setBlocking(true);
-    async.setBufferSize(64);
+    async.setBufferSize(config.getInt("core", "asyncLoggingBufferSize", 64));
     async.setLocationInfo(false);
     async.addAppender(dst);
     async.activateOptions();
@@ -93,7 +94,7 @@ class HttpLog extends AbstractLifeCycle implements RequestLog {
   private void doLog(Request req, Response rsp, CurrentUser user) {
     final LoggingEvent event = new LoggingEvent( //
         Logger.class.getName(), // fqnOfCategoryClass
-        null, // logger (optional)
+        log, // logger
         System.currentTimeMillis(), // when
         Level.INFO, // level
         "", // message text

gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/JettyServer.java

@@ -117,7 +117,7 @@ public class JettyServer {
     Handler app = makeContext(env, cfg);
     if (cfg.getBoolean("httpd", "requestlog", !reverseProxy)) {
       RequestLogHandler handler = new RequestLogHandler();
-      handler.setRequestLog(new HttpLog(site));
+      handler.setRequestLog(new HttpLog(site, cfg));
       handler.setHandler(app);
       app = handler;
     }

gerrit-server/src/main/java/com/google/gerrit/server/git/LargeObjectException.java

@@ -0,0 +1,33 @@
+// Copyright (C) 2012 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.git;
+
+/**
+ * Wrapper for {@link org.eclipse.jgit.errors.LargeObjectException}. Since
+ * org.eclipse.jgit.errors.LargeObjectException is a {@link RuntimeException}
+ * the GerritJsonServlet would treat it as internal failure and as result the
+ * web ui would just show 'Internal Server Error'. Wrapping
+ * org.eclipse.jgit.errors.LargeObjectException into a normal {@link Exception}
+ * allows to display a proper error message.
+ */
+public class LargeObjectException extends Exception {
+
+  private static final long serialVersionUID = 1L;
+
+  public LargeObjectException(final String message,
+      final org.eclipse.jgit.errors.LargeObjectException cause) {
+    super(message, cause);
+  }
+}

gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshLog.java

@@ -18,6 +18,7 @@ import com.google.gerrit.extensions.events.LifecycleListener;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.PeerDaemonUser;
+import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.util.IdGenerator;
 import com.google.gerrit.sshd.SshScope.Context;
@@ -33,6 +34,7 @@ import org.apache.log4j.Level;
 import org.apache.log4j.Logger;
 import org.apache.log4j.spi.ErrorHandler;
 import org.apache.log4j.spi.LoggingEvent;
+import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.util.QuotedString;
 
 import java.io.File;
@@ -59,7 +61,7 @@ class SshLog implements LifecycleListener {
 
   @Inject
   SshLog(final Provider<SshSession> session, final Provider<Context> context,
-      final SitePaths site) {
+      final SitePaths site, @GerritServerConfig Config config) {
     this.session = session;
     this.context = context;
 
@@ -77,7 +79,7 @@ class SshLog implements LifecycleListener {
 
     async = new AsyncAppender();
     async.setBlocking(true);
-    async.setBufferSize(64);
+    async.setBufferSize(config.getInt("core", "asyncLoggingBufferSize", 64));
     async.setLocationInfo(false);
     async.addAppender(dst);
     async.activateOptions();
@@ -99,7 +101,7 @@ class SshLog implements LifecycleListener {
   void onAuthFail(final SshSession sd) {
     final LoggingEvent event = new LoggingEvent( //
         Logger.class.getName(), // fqnOfCategoryClass
-        null, // logger (optional)
+        log, // logger
         System.currentTimeMillis(), // when
         Level.INFO, // level
         "AUTH FAILURE FROM " + sd.getRemoteAddressAsString(), // message text
@@ -168,7 +170,7 @@ class SshLog implements LifecycleListener {
 
     final LoggingEvent event = new LoggingEvent( //
         Logger.class.getName(), // fqnOfCategoryClass
-        null, // logger (optional)
+        log, // logger
         System.currentTimeMillis(), // when
         Level.INFO, // level
         msg, // message text

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Receive.java

@@ -24,6 +24,7 @@ import com.google.gerrit.server.git.VisibleRefFilter;
 import com.google.gerrit.sshd.AbstractGitCommand;
 import com.google.inject.Inject;
 
+import org.eclipse.jgit.errors.TooLargeObjectInPackException;
 import org.eclipse.jgit.errors.UnpackException;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.transport.AdvertiseRefsHook;
@@ -31,6 +32,7 @@ import org.eclipse.jgit.transport.ReceivePack;
 import org.kohsuke.args4j.Option;
 
 import java.io.IOException;
+import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
@@ -92,6 +94,15 @@ final class Receive extends AbstractGitCommand {
       receive.advertiseHistory();
       rp.receive(in, out, err);
     } catch (UnpackException badStream) {
+      // In case this was caused by the user pushing an object whose size
+      // is larger than the receive.maxObjectSizeLimit gerrit.config parameter
+      // we want to present this error to the user
+      if (badStream.getCause() instanceof TooLargeObjectInPackException) {
+        PrintWriter p = toPrintWriter(err);
+        p.print("error: " + badStream.getCause().getMessage() + "\n");
+        p.flush();
+      }
+
       // This may have been triggered by branch level access controls.
       // Log what the heck is going on, as detailed as we can.
       //