opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.14' into stable-2.15

Browse Source 
* stable-2.14:
  HttpPluginServlet: Don't trim leading whitespace from about.md content
  ProjectConfig: Don't use JGit's StringUtils to convert to lower case
  Do not abort indexing if < 50% projects failed
  Revert "AllChangesIndexer: Don't abort when failing to open repository"
  VersionedAccountDestinations: Remove unused createSink(String) method
  ProjectBasicAuthFilter: Add comment why cause is not logged
  BazelBuild: Fix exception message when command was interrupted
  GitwebServlet: Write only one log entry for CGI errors
  GitwebServlet: Log unexpected errors on error level
  PostGpgKeys: Remove unneeded use of Joiner
  Remove some logs for errors that are rethrown
  DropWizardMetricMaker: Improve error messages for invalid arguments
  DropWizardMetricMaker: Improve error message when metric name is invalid
  AllChangesIndexer: Don't abort when failing to open repository

Change-Id: I6febb890b7717731fcb5f0653360982668469069
This commit is contained in:
David Pursehouse
2018-05-17 15:39:34 +09:00
parent 6ae012e010 fcbdca1e30
commit b5e620a6a9
10 changed files with 29 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java
View File

@@ -261,10 +261,10 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
if (accountStates.size() > 1) {
StringBuilder msg = new StringBuilder();
msg.append("GPG key ").append(extIdKey.get()).append(" associated with multiple accounts: ");
Joiner.on(", ")
.appendTo(msg, Lists.transform(accountStates, AccountState.ACCOUNT_ID_FUNCTION));
log.error(msg.toString());
msg.append("GPG key ")
.append(extIdKey.get())
.append(" associated with multiple accounts: ")
.append(Lists.transform(accountStates, AccountState.ACCOUNT_ID_FUNCTION));
throw new IllegalStateException(msg.toString());
}

1
gerrit-httpd/BUILD
View File

@@ -42,6 +42,7 @@ java_library(
"//lib:soy",
"//lib/auto:auto-value",
"//lib/commons:codec",
"//lib/commons:lang",
"//lib/guice",
"//lib/guice:guice-assistedinject",
"//lib/guice:guice-servlet",

2
gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
View File

@@ -164,6 +164,8 @@ class ProjectBasicAuthFilter implements Filter {
rsp.sendError(SC_UNAUTHORIZED);
return false;
} catch (AuthenticationFailedException e) {
// This exception is thrown if the user provided wrong credentials, we don't need to log a
// stacktrace for it.
log.warn("Authentication failed for " + username + ": " + e.getMessage());
rsp.sendError(SC_UNAUTHORIZED);
return false;

11
gerrit-httpd/src/main/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java
View File

@@ -643,7 +643,7 @@ class GitwebServlet extends HttpServlet {
dst.close();
}
} catch (IOException e) {
log.debug("Unexpected error copying input to CGI", e);
log.error("Unexpected error copying input to CGI", e);
}
},
"Gitweb-InputFeeder")
@@ -653,14 +653,19 @@ class GitwebServlet extends HttpServlet {
private void copyStderrToLog(InputStream in) {
new Thread(
() -> {
StringBuilder b = new StringBuilder();
try (BufferedReader br =
new BufferedReader(new InputStreamReader(in, ISO_8859_1.name()))) {
String line;
while ((line = br.readLine()) != null) {
log.error("CGI: " + line);
if (b.length() > 0) {
b.append('\n');
}
b.append("CGI: ").append(line);
}
log.error(b.toString());
} catch (IOException e) {
log.debug("Unexpected error copying stderr from CGI", e);
log.error("Unexpected error copying stderr from CGI", e);
}
},
"Gitweb-ErrorLogger")

3
gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java
View File

@@ -88,6 +88,7 @@ import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.commons.lang.StringUtils;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.util.IO;
import org.eclipse.jgit.util.RawParseUtils;
@@ -478,7 +479,7 @@ class HttpPluginServlet extends HttpServlet implements StartPluginListener, Relo
try (BufferedReader reader = new BufferedReader(isr)) {
String line;
while ((line = reader.readLine()) != null) {
line = line.trim();
line = StringUtils.stripEnd(line, null);
if (line.isEmpty()) {
aboutContent.append("\n");
} else {

4
gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/BazelBuild.java
View File

@@ -17,6 +17,7 @@ package com.google.gerrit.httpd.raw;
import static com.google.common.base.MoreObjects.firstNonNull;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.Joiner;
import com.google.common.escape.Escaper;
import com.google.common.html.HtmlEscapers;
import com.google.common.io.ByteStreams;
@@ -62,7 +63,8 @@ public class BazelBuild {
try {
status = rebuild.waitFor();
} catch (InterruptedException e) {
throw new InterruptedIOException("interrupted waiting for " + proc.toString());
throw new InterruptedIOException(
"interrupted waiting for: " + Joiner.on(' ').join(proc.command()));
}
if (status != 0) {
log.warn("build failed: " + new String(out, UTF_8));

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

@@ -69,13 +69,9 @@ import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.eclipse.jetty.util.thread.ThreadPool;
import org.eclipse.jgit.lib.Config;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@Singleton
public class JettyServer {
private static final Logger log = LoggerFactory.getLogger(JettyServer.class);
static class Lifecycle implements LifecycleListener {
private final JettyServer server;
private final Config cfg;
@@ -425,9 +421,8 @@ public class JettyServer {
"/*",
EnumSet.of(DispatcherType.REQUEST, DispatcherType.ASYNC));
} catch (Throwable e) {
String errorMessage = "Unable to instantiate front-end HTTP Filter " + filterClassName;
log.error(errorMessage, e);
throw new IllegalArgumentException(errorMessage, e);
throw new IllegalArgumentException(
"Unable to instantiate front-end HTTP Filter " + filterClassName, e);
}
}

9
gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java
View File

@@ -151,8 +151,8 @@ public class DropWizardMetricMaker extends MetricMaker {
private static void checkCounterDescription(String name, Description desc) {
checkMetricName(name);
checkArgument(!desc.isConstant(), "counters must not be constant");
checkArgument(!desc.isGauge(), "counters must not be gauge");
checkArgument(!desc.isConstant(), "counter must not be constant");
checkArgument(!desc.isGauge(), "counter must not be gauge");
}
CounterImpl newCounterImpl(String name, boolean isRate) {
@@ -326,7 +326,7 @@ public class DropWizardMetricMaker extends MetricMaker {
if (!desc.getAnnotations()
.get(Description.DESCRIPTION)
.equals(annotations.get(Description.DESCRIPTION))) {
throw new IllegalStateException(String.format("metric %s already defined", name));
throw new IllegalStateException(String.format("metric '%s' already defined", name));
}
} else {
descriptions.put(name, desc.getAnnotations());
@@ -339,7 +339,8 @@ public class DropWizardMetricMaker extends MetricMaker {
private static void checkMetricName(String name) {
checkArgument(
METRIC_NAME_PATTERN.matcher(name).matches(),
"metric name must match %s",
"invalid metric name '%s': must match pattern '%s'",
name,
METRIC_NAME_PATTERN.pattern());
}

4
gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAccountDestinations.java
View File

@@ -69,10 +69,6 @@ public class VersionedAccountDestinations extends VersionedMetaData {
}
}
public ValidationError.Sink createSink(String file) {
return ValidationError.createLoggerSink(file, log);
}
@Override
protected boolean onSave(CommitBuilder commit) throws IOException, ConfigInvalidException {
throw new UnsupportedOperationException("Cannot yet save destinations");

4
gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
View File

@@ -64,6 +64,7 @@ import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Optional;
@@ -75,7 +76,6 @@ import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.transport.RefSpec;
import org.eclipse.jgit.util.StringUtils;
public class ProjectConfig extends VersionedMetaData implements ValidationError.Sink {
public static final String COMMENTLINK = "commentlink";
@@ -1250,7 +1250,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
List<String> types = Lists.newArrayListWithCapacity(4);
for (NotifyType t : NotifyType.values()) {
if (nc.isNotify(t)) {
types.add(StringUtils.toLowerCase(t.name()));
types.add(t.name().toLowerCase(Locale.US));
}
}
rc.setStringList(NOTIFY, nc.getName(), KEY_TYPE, types);