Merge changes Ic502b820,I43630932 * changes: Throw GitilesRequestFailureException Introduce GitilesRequestFailureException and DefaultErrorHandlingFilter
diff --git a/java/com/google/gitiles/ArchiveServlet.java b/java/com/google/gitiles/ArchiveServlet.java index 3f03455..ed9fedd 100644 --- a/java/com/google/gitiles/ArchiveServlet.java +++ b/java/com/google/gitiles/ArchiveServlet.java
@@ -14,10 +14,10 @@ package com.google.gitiles; -import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import static javax.servlet.http.HttpServletResponse.SC_OK; import com.google.common.base.Strings; +import com.google.gitiles.GitilesRequestFailureException.FailureReason; import java.io.IOException; import java.util.Optional; import javax.servlet.ServletException; @@ -50,15 +50,13 @@ ObjectId treeId = getTree(view, repo, rev); if (treeId.equals(ObjectId.zeroId())) { - res.sendError(SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.INCORRECT_OBJECT_TYPE); } Optional<ArchiveFormat> format = ArchiveFormat.byExtension(view.getExtension(), getAccess(req).getConfig()); if (!format.isPresent()) { - res.setStatus(SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT); } String filename = getFilename(view, rev, view.getExtension()); setDownloadHeaders(req, res, filename, format.get().getMimeType());
diff --git a/java/com/google/gitiles/BaseServlet.java b/java/com/google/gitiles/BaseServlet.java index 4b8a139..02a3f1a 100644 --- a/java/com/google/gitiles/BaseServlet.java +++ b/java/com/google/gitiles/BaseServlet.java
@@ -30,6 +30,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import com.google.common.net.HttpHeaders; +import com.google.gitiles.GitilesRequestFailureException.FailureReason; import com.google.gson.FieldNamingPolicy; import com.google.gson.GsonBuilder; import java.io.BufferedWriter; @@ -119,8 +120,7 @@ break; case DEFAULT: default: - res.sendError(SC_BAD_REQUEST); - break; + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT); } } @@ -147,7 +147,7 @@ * @param res in-progress response. */ protected void doGetHtml(HttpServletRequest req, HttpServletResponse res) throws IOException { - res.sendError(SC_BAD_REQUEST); + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT); } /** @@ -157,7 +157,7 @@ * @param res in-progress response. */ protected void doGetText(HttpServletRequest req, HttpServletResponse res) throws IOException { - res.sendError(SC_BAD_REQUEST); + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT); } /** @@ -167,7 +167,7 @@ * @param res in-progress response. */ protected void doGetJson(HttpServletRequest req, HttpServletResponse res) throws IOException { - res.sendError(SC_BAD_REQUEST); + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT); } protected static Map<String, Object> getData(HttpServletRequest req) {
diff --git a/java/com/google/gitiles/DefaultErrorHandlingFilter.java b/java/com/google/gitiles/DefaultErrorHandlingFilter.java new file mode 100644 index 0000000..958b800 --- /dev/null +++ b/java/com/google/gitiles/DefaultErrorHandlingFilter.java
@@ -0,0 +1,63 @@ +// Copyright 2019 Google LLC +// +// 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 +// +// https://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.gitiles; + +import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; +import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; +import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; +import static org.eclipse.jgit.http.server.GitSmartHttpTools.sendError; + +import java.io.IOException; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.eclipse.jgit.errors.AmbiguousObjectException; +import org.eclipse.jgit.errors.RepositoryNotFoundException; +import org.eclipse.jgit.transport.ServiceMayNotContinueException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Convert exceptions into HTTP response. */ +public class DefaultErrorHandlingFilter extends AbstractHttpFilter { + private static final Logger log = LoggerFactory.getLogger(DefaultErrorHandlingFilter.class); + + /** HTTP header that indicates an error detail. */ + public static final String GITILES_ERROR = "X-Gitiles-Error"; + + @Override + public void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain) + throws IOException, ServletException { + try { + chain.doFilter(req, res); + } catch (GitilesRequestFailureException e) { + res.setHeader(GITILES_ERROR, e.getReason().toString()); + String publicMessage = e.getPublicErrorMessage(); + if (publicMessage != null) { + res.sendError(e.getReason().getHttpStatusCode(), publicMessage); + } else { + res.sendError(e.getReason().getHttpStatusCode()); + } + } catch (RepositoryNotFoundException e) { + res.sendError(SC_NOT_FOUND); + } catch (AmbiguousObjectException e) { + res.sendError(SC_BAD_REQUEST); + } catch (ServiceMayNotContinueException e) { + sendError(req, res, e.getStatusCode(), e.getMessage()); + } catch (IOException | ServletException err) { + log.warn("Internal server error", err); + res.sendError(SC_INTERNAL_SERVER_ERROR); + } + } +}
diff --git a/java/com/google/gitiles/DescribeServlet.java b/java/com/google/gitiles/DescribeServlet.java index 785d452..ebd2ea4 100644 --- a/java/com/google/gitiles/DescribeServlet.java +++ b/java/com/google/gitiles/DescribeServlet.java
@@ -14,11 +14,9 @@ package com.google.gitiles; -import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; -import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; - import com.google.common.base.Joiner; import com.google.common.collect.ImmutableMap; +import com.google.gitiles.GitilesRequestFailureException.FailureReason; import com.google.gson.reflect.TypeToken; import java.io.IOException; import java.io.Writer; @@ -84,21 +82,14 @@ try { return repo.resolve(rev); } catch (RevisionSyntaxException e) { - renderTextError( - req, - res, - SC_BAD_REQUEST, - "Invalid revision syntax: " + RefServlet.sanitizeRefForText(rev)); - return null; + throw new GitilesRequestFailureException(FailureReason.INCORECT_PARAMETER) + .withPublicErrorMessage( + "Invalid revision syntax: %s", RefServlet.sanitizeRefForText(rev)); } catch (AmbiguousObjectException e) { - renderTextError( - req, - res, - SC_BAD_REQUEST, - String.format( + throw new GitilesRequestFailureException(FailureReason.AMBIGUOUS_OBJECT) + .withPublicErrorMessage( "Ambiguous short SHA-1 %s (%s)", - e.getAbbreviatedObjectId(), Joiner.on(", ").join(e.getCandidates()))); - return null; + e.getAbbreviatedObjectId(), Joiner.on(", ").join(e.getCandidates())); } } @@ -106,8 +97,7 @@ Repository repo, GitilesView view, HttpServletRequest req, HttpServletResponse res) throws IOException { if (!getBooleanParam(view, CONTAINS_PARAM)) { - res.setStatus(SC_BAD_REQUEST); - return null; + throw new GitilesRequestFailureException(FailureReason.INCORECT_PARAMETER); } ObjectId id = resolve(repo, view, req, res); if (id == null) { @@ -124,8 +114,7 @@ throw new IOException(e); } if (name == null) { - res.setStatus(SC_NOT_FOUND); - return null; + throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND); } return name; } @@ -137,8 +126,8 @@ boolean all = getBooleanParam(view, ALL_PARAM); boolean tags = getBooleanParam(view, TAGS_PARAM); if (all && tags) { - renderTextError(req, res, SC_BAD_REQUEST, "Cannot specify both \"all\" and \"tags\""); - return null; + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_REVISION_NAMES) + .withPublicErrorMessage("Cannot specify both \"all\" and \"tags\""); } if (all) { cmd.addPrefix(Constants.R_REFS);
diff --git a/java/com/google/gitiles/DiffServlet.java b/java/com/google/gitiles/DiffServlet.java index d0e4409..5b1801d 100644 --- a/java/com/google/gitiles/DiffServlet.java +++ b/java/com/google/gitiles/DiffServlet.java
@@ -15,11 +15,11 @@ package com.google.gitiles; import static com.google.common.base.Preconditions.checkNotNull; -import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import com.google.common.io.BaseEncoding; import com.google.gitiles.CommitData.Field; import com.google.gitiles.DateFormatter.Format; +import com.google.gitiles.GitilesRequestFailureException.FailureReason; import java.io.IOException; import java.io.OutputStream; import java.io.Writer; @@ -67,8 +67,7 @@ AbstractTreeIterator newTree; try { if (tw == null && !view.getPathPart().isEmpty()) { - res.setStatus(SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND); } isFile = tw != null && isFile(tw); @@ -77,9 +76,10 @@ showCommit = isParentOf(walk, view.getOldRevision(), view.getRevision()); oldTree = getTreeIterator(walk, view.getOldRevision().getId()); newTree = getTreeIterator(walk, view.getRevision().getId()); - } catch (MissingObjectException | IncorrectObjectTypeException e) { - res.setStatus(SC_NOT_FOUND); - return; + } catch (MissingObjectException e) { + throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND, e); + } catch (IncorrectObjectTypeException e) { + throw new GitilesRequestFailureException(FailureReason.INCORRECT_OBJECT_TYPE, e); } Map<String, Object> data = getData(req); @@ -124,9 +124,10 @@ try { oldTree = getTreeIterator(walk, view.getOldRevision().getId()); newTree = getTreeIterator(walk, view.getRevision().getId()); - } catch (MissingObjectException | IncorrectObjectTypeException e) { - res.setStatus(SC_NOT_FOUND); - return; + } catch (MissingObjectException e) { + throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND, e); + } catch (IncorrectObjectTypeException e) { + throw new GitilesRequestFailureException(FailureReason.INCORRECT_OBJECT_TYPE, e); } try (Writer writer = startRenderText(req, res);
diff --git a/java/com/google/gitiles/GitilesFilter.java b/java/com/google/gitiles/GitilesFilter.java index 59e8acd..2c810bb 100644 --- a/java/com/google/gitiles/GitilesFilter.java +++ b/java/com/google/gitiles/GitilesFilter.java
@@ -37,6 +37,7 @@ import java.util.Iterator; import java.util.Map; import java.util.regex.Pattern; +import javax.annotation.Nullable; import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; @@ -174,20 +175,22 @@ private TimeCache timeCache; private BlameCache blameCache; private GitwebRedirectFilter gitwebRedirect; + private Filter errorHandler; private boolean initialized; GitilesFilter() {} GitilesFilter( Config config, - Renderer renderer, - GitilesUrls urls, - GitilesAccess.Factory accessFactory, - final RepositoryResolver<HttpServletRequest> resolver, - VisibilityCache visibilityCache, - TimeCache timeCache, - BlameCache blameCache, - GitwebRedirectFilter gitwebRedirect) { + @Nullable Renderer renderer, + @Nullable GitilesUrls urls, + @Nullable GitilesAccess.Factory accessFactory, + @Nullable RepositoryResolver<HttpServletRequest> resolver, + @Nullable VisibilityCache visibilityCache, + @Nullable TimeCache timeCache, + @Nullable BlameCache blameCache, + @Nullable GitwebRedirectFilter gitwebRedirect, + @Nullable Filter errorHandler) { this.config = checkNotNull(config, "config"); this.renderer = renderer; this.urls = urls; @@ -199,6 +202,7 @@ if (resolver != null) { this.resolver = resolver; } + this.errorHandler = errorHandler; } @Override @@ -232,6 +236,12 @@ initialized = true; } + @Override + protected ServletBinder register(ServletBinder b) { + b.through(errorHandler); + return b; + } + public synchronized BaseServlet getDefaultHandler(GitilesView.Type view) { checkNotInitialized(); switch (view) { @@ -300,6 +310,7 @@ setDefaultTimeCache(); setDefaultBlameCache(); setDefaultGitwebRedirect(); + setDefaultErrorHandler(); } private void setDefaultConfig(FilterConfig filterConfig) throws ServletException { @@ -407,6 +418,12 @@ } } + private void setDefaultErrorHandler() { + if (errorHandler == null) { + errorHandler = new DefaultErrorHandlingFilter(); + } + } + private static String getBaseGitUrl(Config config) throws ServletException { String baseGitUrl = config.getString("gitiles", null, "baseGitUrl"); if (baseGitUrl == null) {
diff --git a/java/com/google/gitiles/GitilesRequestFailureException.java b/java/com/google/gitiles/GitilesRequestFailureException.java new file mode 100644 index 0000000..91f9cf8 --- /dev/null +++ b/java/com/google/gitiles/GitilesRequestFailureException.java
@@ -0,0 +1,163 @@ +// Copyright 2019 Google LLC +// +// 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 +// +// https://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.gitiles; + +import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; +import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN; +import static javax.servlet.http.HttpServletResponse.SC_GONE; +import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; +import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; +import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED; + +import javax.annotation.Nullable; + +/** + * Indicates the request should be failed. + * + * <p>When an HTTP request should be failed, throw this exception instead of directly setting an + * HTTP status code. The exception is caught by an error handler in {@link GitilesFilter}. By + * default, {@link DefaultErrorHandlingFilter} handles this exception and set an appropriate HTTP + * status code. If you want to customize how the error is surfaced, like changing the error page + * rendering, replace this error handler from {@link GitilesServlet}. + * + * <h2>Extending the error space</h2> + * + * <p>{@link GitilesServlet} lets you customize some parts of Gitiles, and sometimes you would like + * to create a new {@link FailureReason}. For example, a customized {@code RepositoryResolver} might + * check a request quota and reject a request if a client sends too many requests. In that case, you + * can define your own {@link RuntimeException} and an error handler. + * + * <pre><code> + * public final class MyRequestFailureException extends RuntimeException { + * private final FailureReason reason; + * + * public MyRequestFailureException(FailureReason reason) { + * super(); + * this.reason = reason; + * } + * + * public FailureReason getReason() { + * return reason; + * } + * + * enum FailureReason { + * QUOTA_EXCEEDED(429); + * } + * + * private final int httpStatusCode; + * + * FailureReason(int httpStatusCode) { + * this.httpStatusCode = httpStatusCode; + * } + * + * public int getHttpStatusCode() { + * return httpStatusCode; + * } + * } + * + * public class MyErrorHandlingFilter extends AbstractHttpFilter { + * private static final DefaultErrorHandlingFilter delegate = + * new DefaultErrorHandlingFilter(); + * + * {@literal @}Override + * public void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain) + * throws IOException, ServletException { + * try { + * delegate.doFilter(req, res, chain); + * } catch (MyRequestFailureException e) { + * res.setHeader(DefaultErrorHandlingFilter.GITILES_ERROR, e.getReason().toString()); + * res.sendError(e.getReason().getHttpStatusCode()); + * } + * } + * } + * </code></pre> + * + * <p>{@code RepositoryResolver} can throw {@code MyRequestFailureException} and {@code + * MyErrorHandlingFilter} will handle that. You can control how the error should be surfaced. + */ +public final class GitilesRequestFailureException extends RuntimeException { + private final FailureReason reason; + private String publicErrorMessage; + + public GitilesRequestFailureException(FailureReason reason) { + super(); + this.reason = reason; + } + + public GitilesRequestFailureException(FailureReason reason, Throwable cause) { + super(cause); + this.reason = reason; + } + + public GitilesRequestFailureException withPublicErrorMessage(String format, Object... params) { + this.publicErrorMessage = String.format(format, params); + return this; + } + + public FailureReason getReason() { + return reason; + } + + @Nullable + public String getPublicErrorMessage() { + return publicErrorMessage; + } + + /** The request failure reason. */ + public enum FailureReason { + /** The object specified by the URL is ambiguous and Gitiles cannot identify one object. */ + AMBIGUOUS_OBJECT(SC_BAD_REQUEST), + /** There's nothing to show for blame (e.g. the file is empty). */ + BLAME_REGION_NOT_FOUND(SC_NOT_FOUND), + /** Cannot parse URL as a Gitiles URL. */ + CANNOT_PARSE_GITILES_VIEW(SC_NOT_FOUND), + /** URL parameters are not valid. */ + INCORECT_PARAMETER(SC_BAD_REQUEST), + /** + * The object specified by the URL is not suitable for the view (e.g. trying to show a blob as a + * tree). + */ + INCORRECT_OBJECT_TYPE(SC_BAD_REQUEST), + /** Markdown rendering is not enabled. */ + MARKDOWN_NOT_ENABLED(SC_NOT_FOUND), + /** Request is not authorized. */ + NOT_AUTHORIZED(SC_UNAUTHORIZED), + /** Object is not found. */ + OBJECT_NOT_FOUND(SC_NOT_FOUND), + /** Object is too large to show. */ + OBJECT_TOO_LARGE(SC_INTERNAL_SERVER_ERROR), + /** Repository is not found. */ + REPOSITORY_NOT_FOUND(SC_NOT_FOUND), + /** Gitiles is not enabled for the repository. */ + SERVICE_NOT_ENABLED(SC_FORBIDDEN), + /** GitWeb URL cannot be converted to Gitiles URL. */ + UNSUPPORTED_GITWEB_URL(SC_GONE), + /** The specified object's type is not supported. */ + UNSUPPORTED_OBJECT_TYPE(SC_NOT_FOUND), + /** The specified format type is not supported. */ + UNSUPPORTED_RESPONSE_FORMAT(SC_BAD_REQUEST), + /** The specified revision names are not supported. */ + UNSUPPORTED_REVISION_NAMES(SC_BAD_REQUEST); + + private final int httpStatusCode; + + FailureReason(int httpStatusCode) { + this.httpStatusCode = httpStatusCode; + } + + public int getHttpStatusCode() { + return httpStatusCode; + } + } +}
diff --git a/java/com/google/gitiles/GitilesServlet.java b/java/com/google/gitiles/GitilesServlet.java index 56f4c61..6d10c0c 100644 --- a/java/com/google/gitiles/GitilesServlet.java +++ b/java/com/google/gitiles/GitilesServlet.java
@@ -52,6 +52,30 @@ @Nullable TimeCache timeCache, @Nullable BlameCache blameCache, @Nullable GitwebRedirectFilter gitwebRedirect) { + this( + config, + renderer, + urls, + accessFactory, + resolver, + visibilityCache, + timeCache, + blameCache, + gitwebRedirect, + null); + } + + public GitilesServlet( + Config config, + @Nullable Renderer renderer, + @Nullable GitilesUrls urls, + @Nullable GitilesAccess.Factory accessFactory, + @Nullable RepositoryResolver<HttpServletRequest> resolver, + @Nullable VisibilityCache visibilityCache, + @Nullable TimeCache timeCache, + @Nullable BlameCache blameCache, + @Nullable GitwebRedirectFilter gitwebRedirect, + @Nullable Filter errorHandler) { super( new GitilesFilter( config, @@ -62,7 +86,8 @@ visibilityCache, timeCache, blameCache, - gitwebRedirect)); + gitwebRedirect, + errorHandler)); } public GitilesServlet() {
diff --git a/java/com/google/gitiles/GitwebRedirectFilter.java b/java/com/google/gitiles/GitwebRedirectFilter.java index f1e01bc..324af72 100644 --- a/java/com/google/gitiles/GitwebRedirectFilter.java +++ b/java/com/google/gitiles/GitwebRedirectFilter.java
@@ -18,7 +18,6 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.net.HttpHeaders.LOCATION; import static java.nio.charset.StandardCharsets.UTF_8; -import static javax.servlet.http.HttpServletResponse.SC_GONE; import static javax.servlet.http.HttpServletResponse.SC_MOVED_PERMANENTLY; import com.google.common.base.Splitter; @@ -27,6 +26,7 @@ import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.ListMultimap; import com.google.gitiles.GitilesView.InvalidViewException; +import com.google.gitiles.GitilesRequestFailureException.FailureReason; import java.io.IOException; import java.io.UnsupportedEncodingException; import java.net.URLDecoder; @@ -117,8 +117,7 @@ } else { // Gitiles does not provide an RSS feed (a=rss,atom,opml) // Any other URL is out of date and not valid anymore. - res.sendError(SC_GONE); - return; + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_GITWEB_URL); } if (!Strings.isNullOrEmpty(project)) { @@ -132,8 +131,7 @@ .setServletPath(gitwebView.getServletPath()) .toUrl(); } catch (InvalidViewException e) { - res.setStatus(SC_GONE); - return; + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_GITWEB_URL); } res.setStatus(SC_MOVED_PERMANENTLY); res.setHeader(LOCATION, url);
diff --git a/java/com/google/gitiles/HostIndexServlet.java b/java/com/google/gitiles/HostIndexServlet.java index d23aa3c..ce2ec6b 100644 --- a/java/com/google/gitiles/HostIndexServlet.java +++ b/java/com/google/gitiles/HostIndexServlet.java
@@ -15,16 +15,11 @@ package com.google.gitiles; import static com.google.common.base.Preconditions.checkNotNull; -import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; -import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN; -import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; -import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE; -import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED; -import static org.eclipse.jgit.http.server.GitSmartHttpTools.sendError; import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Sets; +import com.google.gitiles.GitilesRequestFailureException.FailureReason; import com.google.gson.reflect.TypeToken; import com.google.template.soy.data.SoyListData; import com.google.template.soy.data.SoyMapData; @@ -40,8 +35,6 @@ import javax.annotation.Nullable; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.eclipse.jgit.errors.RepositoryNotFoundException; -import org.eclipse.jgit.transport.ServiceMayNotContinueException; import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; import org.slf4j.Logger; @@ -66,27 +59,13 @@ Map<String, RepositoryDescription> descs; try { descs = getAccess(req).listRepositories(prefix, branches); - } catch (RepositoryNotFoundException e) { - res.sendError(SC_NOT_FOUND); - return null; } catch (ServiceNotEnabledException e) { - res.sendError(SC_FORBIDDEN); - return null; + throw new GitilesRequestFailureException(FailureReason.SERVICE_NOT_ENABLED, e); } catch (ServiceNotAuthorizedException e) { - res.sendError(SC_UNAUTHORIZED); - return null; - } catch (ServiceMayNotContinueException e) { - sendError(req, res, e.getStatusCode(), e.getMessage()); - return null; - } catch (IOException err) { - String name = urls.getHostName(req); - log.warn("Cannot scan repositories" + (name != null ? " for " + name : ""), err); - res.sendError(SC_SERVICE_UNAVAILABLE); - return null; + throw new GitilesRequestFailureException(FailureReason.NOT_AUTHORIZED, e); } if (prefix != null && descs.isEmpty()) { - res.sendError(SC_NOT_FOUND); - return null; + throw new GitilesRequestFailureException(FailureReason.REPOSITORY_NOT_FOUND); } return descs; } @@ -103,15 +82,14 @@ protected void doHead(HttpServletRequest req, HttpServletResponse res) throws IOException { Optional<FormatType> format = getFormat(req); if (!format.isPresent()) { - res.sendError(SC_BAD_REQUEST); - return; + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT); } GitilesView view = ViewFilter.getView(req); String prefix = view.getRepositoryPrefix(); if (prefix != null) { Map<String, RepositoryDescription> descs = - list(req, res, prefix, Collections.<String>emptySet()); + list(req, res, prefix, Collections.emptySet()); if (descs == null) { return; } @@ -125,8 +103,7 @@ break; case DEFAULT: default: - res.sendError(SC_BAD_REQUEST); - break; + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT); } }
diff --git a/java/com/google/gitiles/LogServlet.java b/java/com/google/gitiles/LogServlet.java index f1bdd95..3523846 100644 --- a/java/com/google/gitiles/LogServlet.java +++ b/java/com/google/gitiles/LogServlet.java
@@ -15,8 +15,6 @@ package com.google.gitiles; import static com.google.common.base.Preconditions.checkNotNull; -import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; -import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import com.google.common.base.Strings; import com.google.common.collect.Iterables; @@ -27,6 +25,7 @@ import com.google.common.primitives.Longs; import com.google.gitiles.CommitData.Field; import com.google.gitiles.DateFormatter.Format; +import com.google.gitiles.GitilesRequestFailureException.FailureReason; import com.google.gson.reflect.TypeToken; import java.io.IOException; import java.io.OutputStream; @@ -42,7 +41,6 @@ import org.eclipse.jgit.diff.DiffConfig; import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.MissingObjectException; -import org.eclipse.jgit.errors.RevWalkException; import org.eclipse.jgit.http.server.ServletUtils; import org.eclipse.jgit.lib.AbbreviatedObjectId; import org.eclipse.jgit.lib.Constants; @@ -97,8 +95,7 @@ GitilesAccess access = getAccess(req); paginator = newPaginator(repo, view, access); if (paginator == null) { - res.setStatus(SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND); } DateFormatter df = new DateFormatter(access, Format.DEFAULT); @@ -133,10 +130,6 @@ .renderStreaming(paginator, null, renderer, w, df, LogSoyData.FooterBehavior.NEXT); w.flush(); } - } catch (RevWalkException e) { - log.warn("Error in rev walk", e); - res.setStatus(SC_INTERNAL_SERVER_ERROR); - return; } finally { if (paginator != null) { paginator.getWalk().close(); @@ -160,8 +153,7 @@ GitilesAccess access = getAccess(req); paginator = newPaginator(repo, view, access); if (paginator == null) { - res.setStatus(SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND); } DateFormatter df = new DateFormatter(access, Format.DEFAULT); CommitJsonData.Log result = new CommitJsonData.Log();
diff --git a/java/com/google/gitiles/PathServlet.java b/java/com/google/gitiles/PathServlet.java index 0ecb875..dac38d3 100644 --- a/java/com/google/gitiles/PathServlet.java +++ b/java/com/google/gitiles/PathServlet.java
@@ -16,8 +16,6 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.gitiles.TreeSoyData.resolveTargetUrl; -import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; -import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; import static org.eclipse.jgit.lib.Constants.OBJ_COMMIT; import static org.eclipse.jgit.lib.Constants.OBJ_TREE; @@ -29,6 +27,7 @@ import com.google.common.collect.Maps; import com.google.common.io.BaseEncoding; import com.google.common.primitives.Bytes; +import com.google.gitiles.GitilesRequestFailureException.FailureReason; import java.io.IOException; import java.io.OutputStream; import java.io.Writer; @@ -131,8 +130,7 @@ try (RevWalk rw = new RevWalk(repo); WalkResult wr = WalkResult.forPath(rw, view, false)) { if (wr == null) { - res.setStatus(SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND); } switch (wr.type) { case TREE: @@ -149,12 +147,10 @@ showGitlink(req, res, wr); break; default: - log.error("Bad file type: {}", wr.type); - res.setStatus(SC_NOT_FOUND); - break; + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_OBJECT_TYPE); } } catch (LargeObjectException e) { - res.setStatus(SC_INTERNAL_SERVER_ERROR); + throw new GitilesRequestFailureException(FailureReason.OBJECT_TOO_LARGE, e); } } @@ -166,8 +162,7 @@ try (RevWalk rw = new RevWalk(repo); WalkResult wr = WalkResult.forPath(rw, view, false)) { if (wr == null) { - res.setStatus(SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND); } // Write base64 as plain text without modifying any other headers, under @@ -185,11 +180,10 @@ break; case GITLINK: default: - renderTextError(req, res, SC_NOT_FOUND, "Not a file"); - break; + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_OBJECT_TYPE); } } catch (LargeObjectException e) { - res.setStatus(SC_INTERNAL_SERVER_ERROR); + throw new GitilesRequestFailureException(FailureReason.OBJECT_TOO_LARGE, e); } } @@ -252,8 +246,7 @@ try (RevWalk rw = new RevWalk(repo); WalkResult wr = WalkResult.forPath(rw, view, recursive)) { if (wr == null) { - res.setStatus(SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND); } switch (wr.type) { case REGULAR_FILE: @@ -275,11 +268,10 @@ case GITLINK: case SYMLINK: default: - res.setStatus(SC_NOT_FOUND); - break; + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_OBJECT_TYPE); } } catch (LargeObjectException e) { - res.setStatus(SC_INTERNAL_SERVER_ERROR); + throw new GitilesRequestFailureException(FailureReason.OBJECT_TOO_LARGE, e); } }
diff --git a/java/com/google/gitiles/RefServlet.java b/java/com/google/gitiles/RefServlet.java index 2b16ef5..fd93126 100644 --- a/java/com/google/gitiles/RefServlet.java +++ b/java/com/google/gitiles/RefServlet.java
@@ -16,7 +16,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -25,6 +24,7 @@ import com.google.common.collect.Ordering; import com.google.common.primitives.Ints; import com.google.common.util.concurrent.UncheckedExecutionException; +import com.google.gitiles.GitilesRequestFailureException.FailureReason; import com.google.gson.reflect.TypeToken; import java.io.IOException; import java.io.Writer; @@ -59,8 +59,7 @@ @Override protected void doGetHtml(HttpServletRequest req, HttpServletResponse res) throws IOException { if (!ViewFilter.getView(req).getPathPart().isEmpty()) { - res.setStatus(SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.INCORECT_PARAMETER); } List<Map<String, Object>> tags; try (RevWalk walk = new RevWalk(ServletUtils.getRepository(req))) {
diff --git a/java/com/google/gitiles/RepositoryFilter.java b/java/com/google/gitiles/RepositoryFilter.java index fa9d601..a25a8de 100644 --- a/java/com/google/gitiles/RepositoryFilter.java +++ b/java/com/google/gitiles/RepositoryFilter.java
@@ -16,11 +16,9 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.gitiles.ViewFilter.getRegexGroup; -import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN; -import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED; -import static org.eclipse.jgit.http.server.GitSmartHttpTools.sendError; import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_REPOSITORY; +import com.google.gitiles.GitilesRequestFailureException.FailureReason; import java.io.IOException; import javax.servlet.FilterChain; import javax.servlet.ServletException; @@ -28,12 +26,12 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.transport.ServiceMayNotContinueException; import org.eclipse.jgit.transport.resolver.RepositoryResolver; import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; class RepositoryFilter extends AbstractHttpFilter { + private final RepositoryResolver<HttpServletRequest> resolver; RepositoryFilter(RepositoryResolver<HttpServletRequest> resolver) { @@ -53,15 +51,13 @@ // to HostIndexServlet which will attempt to list repositories // or send SC_NOT_FOUND there. chain.doFilter(req, res); - } catch (ServiceMayNotContinueException e) { - sendError(req, res, e.getStatusCode(), e.getMessage()); } finally { req.removeAttribute(ATTRIBUTE_REPOSITORY); } } catch (ServiceNotEnabledException e) { - sendError(req, res, SC_FORBIDDEN); + throw new GitilesRequestFailureException(FailureReason.SERVICE_NOT_ENABLED, e); } catch (ServiceNotAuthorizedException e) { - res.sendError(SC_UNAUTHORIZED); + throw new GitilesRequestFailureException(FailureReason.NOT_AUTHORIZED, e); } } }
diff --git a/java/com/google/gitiles/RepositoryIndexServlet.java b/java/com/google/gitiles/RepositoryIndexServlet.java index 8e13155..d057512 100644 --- a/java/com/google/gitiles/RepositoryIndexServlet.java +++ b/java/com/google/gitiles/RepositoryIndexServlet.java
@@ -15,7 +15,6 @@ package com.google.gitiles; import static com.google.common.base.Preconditions.checkNotNull; -import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; @@ -23,6 +22,7 @@ import com.google.common.collect.Maps; import com.google.common.html.types.SafeHtml; import com.google.gitiles.DateFormatter.Format; +import com.google.gitiles.GitilesRequestFailureException.FailureReason; import com.google.gitiles.doc.MarkdownConfig; import com.google.gson.reflect.TypeToken; import java.io.IOException; @@ -65,8 +65,7 @@ // If the repository didn't exist a prior filter would have 404 replied. Optional<FormatType> format = getFormat(req); if (!format.isPresent()) { - res.sendError(SC_BAD_REQUEST); - return; + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT); } switch (format.get()) { case HTML: @@ -77,8 +76,7 @@ case TEXT: case DEFAULT: default: - res.sendError(SC_BAD_REQUEST); - break; + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT); } }
diff --git a/java/com/google/gitiles/RevisionServlet.java b/java/com/google/gitiles/RevisionServlet.java index 597c053..8bf661f 100644 --- a/java/com/google/gitiles/RevisionServlet.java +++ b/java/com/google/gitiles/RevisionServlet.java
@@ -15,7 +15,6 @@ package com.google.gitiles; import static com.google.common.base.Preconditions.checkNotNull; -import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; import static org.eclipse.jgit.lib.Constants.OBJ_COMMIT; import static org.eclipse.jgit.lib.Constants.OBJ_TAG; @@ -28,6 +27,7 @@ import com.google.gitiles.CommitData.Field; import com.google.gitiles.CommitJsonData.Commit; import com.google.gitiles.DateFormatter.Format; +import com.google.gitiles.GitilesRequestFailureException.FailureReason; import java.io.IOException; import java.io.OutputStream; import java.io.Writer; @@ -125,18 +125,12 @@ new TagSoyData(linkifier, req).toSoyData(walk, (RevTag) obj, df))); break; default: - log.warn("Bad object type for {}: {}", ObjectId.toString(obj.getId()), obj.getType()); - res.setStatus(SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_OBJECT_TYPE); } } catch (MissingObjectException e) { - log.warn("Missing object " + ObjectId.toString(obj.getId()), e); - res.setStatus(SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND, e); } catch (IncorrectObjectTypeException e) { - log.warn("Incorrect object type for " + ObjectId.toString(obj.getId()), e); - res.setStatus(SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.INCORRECT_OBJECT_TYPE, e); } } @@ -159,7 +153,7 @@ try (ObjectReader reader = repo.newObjectReader()) { ObjectLoader loader = reader.open(view.getRevision().getId()); if (loader.getType() != OBJ_COMMIT) { - res.setStatus(SC_NOT_FOUND); + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_OBJECT_TYPE); } else { PathServlet.setTypeHeader(res, loader.getType()); try (Writer writer = startRenderText(req, res); @@ -188,8 +182,7 @@ break; default: // TODO(dborowitz): Support showing other types. - res.setStatus(SC_NOT_FOUND); - break; + throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_OBJECT_TYPE); } } }
diff --git a/java/com/google/gitiles/RootedDocServlet.java b/java/com/google/gitiles/RootedDocServlet.java index 96329b1..bf72b32 100644 --- a/java/com/google/gitiles/RootedDocServlet.java +++ b/java/com/google/gitiles/RootedDocServlet.java
@@ -16,6 +16,7 @@ import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_REPOSITORY; +import com.google.gitiles.GitilesRequestFailureException.FailureReason; import com.google.gitiles.doc.DocServlet; import com.google.gitiles.doc.HtmlSanitizer; import java.io.IOException; @@ -24,7 +25,6 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; @@ -74,14 +74,12 @@ RevWalk rw = new RevWalk(repo)) { ObjectId id = repo.resolve(BRANCH); if (id == null) { - res.sendError(HttpServletResponse.SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND); } RevObject obj = rw.peel(rw.parseAny(id)); if (!(obj instanceof RevCommit)) { - res.sendError(HttpServletResponse.SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.INCORRECT_OBJECT_TYPE); } req.setAttribute(ATTRIBUTE_REPOSITORY, repo); @@ -95,11 +93,10 @@ .build()); docServlet.service(req, res); - } catch (RepositoryNotFoundException - | ServiceNotAuthorizedException - | ServiceNotEnabledException e) { - log.error(String.format("cannot open repository for %s", req.getServerName()), e); - res.sendError(HttpServletResponse.SC_NOT_FOUND); + } catch (ServiceNotAuthorizedException e) { + throw new GitilesRequestFailureException(FailureReason.NOT_AUTHORIZED, e); + } catch (ServiceNotEnabledException e) { + throw new GitilesRequestFailureException(FailureReason.SERVICE_NOT_ENABLED, e); } finally { ViewFilter.removeView(req); req.removeAttribute(ATTRIBUTE_REPOSITORY);
diff --git a/java/com/google/gitiles/ViewFilter.java b/java/com/google/gitiles/ViewFilter.java index 03e8d9a..590e583 100644 --- a/java/com/google/gitiles/ViewFilter.java +++ b/java/com/google/gitiles/ViewFilter.java
@@ -16,12 +16,10 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; -import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE; -import static org.eclipse.jgit.http.server.GitSmartHttpTools.sendError; import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_REPOSITORY; import com.google.common.base.Strings; +import com.google.gitiles.GitilesRequestFailureException.FailureReason; import java.io.IOException; import java.util.Map; import javax.servlet.FilterChain; @@ -30,7 +28,6 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jgit.http.server.ServletUtils; import org.eclipse.jgit.http.server.glue.WrappedRequest; -import org.eclipse.jgit.transport.ServiceMayNotContinueException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -102,21 +99,9 @@ @Override public void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain) throws IOException, ServletException { - GitilesView.Builder view; - try { - view = parse(req); - } catch (ServiceMayNotContinueException e) { - sendError(req, res, e.getStatusCode(), e.getMessage()); - return; - } catch (IOException err) { - String name = urls.getHostName(req); - log.warn("Cannot parse view" + (name != null ? " for " + name : ""), err); - res.setStatus(SC_SERVICE_UNAVAILABLE); - return; - } + GitilesView.Builder view = parse(req); if (view == null) { - res.setStatus(SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.CANNOT_PARSE_GITILES_VIEW); } @SuppressWarnings("unchecked")
diff --git a/java/com/google/gitiles/blame/BlameServlet.java b/java/com/google/gitiles/blame/BlameServlet.java index 605ac6f..f1b9976 100644 --- a/java/com/google/gitiles/blame/BlameServlet.java +++ b/java/com/google/gitiles/blame/BlameServlet.java
@@ -15,7 +15,6 @@ package com.google.gitiles.blame; import static com.google.common.base.Preconditions.checkNotNull; -import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; @@ -29,6 +28,8 @@ import com.google.gitiles.GitilesAccess; import com.google.gitiles.GitilesView; import com.google.gitiles.Renderer; +import com.google.gitiles.GitilesRequestFailureException; +import com.google.gitiles.GitilesRequestFailureException.FailureReason; import com.google.gitiles.ViewFilter; import com.google.gitiles.blame.cache.BlameCache; import com.google.gitiles.blame.cache.Region; @@ -159,8 +160,7 @@ RevCommit currCommit = rw.parseCommit(view.getRevision().getId()); ObjectId currCommitBlobId = resolveBlob(view, rw, currCommit); if (currCommitBlobId == null) { - res.setStatus(SC_NOT_FOUND); - return null; + throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND); } ObjectId lastCommit = cache.findLastCommit(repo, currCommit, view.getPathPart()); @@ -182,8 +182,7 @@ List<Region> regions = cache.get(repo, lastCommit, view.getPathPart()); if (regions.isEmpty()) { - res.setStatus(SC_NOT_FOUND); - return null; + throw new GitilesRequestFailureException(FailureReason.BLAME_REGION_NOT_FOUND); } return new RegionResult(regions, lastCommitBlobId); }
diff --git a/java/com/google/gitiles/doc/DocServlet.java b/java/com/google/gitiles/doc/DocServlet.java index 24f8d4b..509f460 100644 --- a/java/com/google/gitiles/doc/DocServlet.java +++ b/java/com/google/gitiles/doc/DocServlet.java
@@ -32,6 +32,8 @@ import com.google.gitiles.GitilesAccess; import com.google.gitiles.GitilesView; import com.google.gitiles.Renderer; +import com.google.gitiles.GitilesRequestFailureException.FailureReason; +import com.google.gitiles.GitilesRequestFailureException; import com.google.gitiles.ViewFilter; import com.google.gitiles.doc.html.StreamHtmlBuilder; import java.io.IOException; @@ -86,8 +88,7 @@ protected void doGetHtml(HttpServletRequest req, HttpServletResponse res) throws IOException { MarkdownConfig cfg = MarkdownConfig.get(getAccess(req).getConfig()); if (!cfg.render) { - res.setStatus(SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.MARKDOWN_NOT_ENABLED); } GitilesView view = ViewFilter.getView(req); @@ -99,14 +100,12 @@ try { root = rw.parseTree(view.getRevision().getId()); } catch (IncorrectObjectTypeException e) { - res.setStatus(SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.INCORRECT_OBJECT_TYPE); } MarkdownFile srcmd = findFile(rw, root, path); if (srcmd == null) { - res.setStatus(SC_NOT_FOUND); - return; + throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND); } MarkdownFile navmd = findNavbar(rw, root, path); @@ -125,9 +124,6 @@ } catch (LargeObjectException.ExceedsLimit errBig) { fileTooBig(res, view, errBig); return; - } catch (IOException err) { - readError(res, view, err); - return; } MarkdownToHtml.Builder fmt = @@ -280,29 +276,12 @@ HttpServletResponse res, GitilesView view, LargeObjectException.ExceedsLimit errBig) throws IOException { if (view.getType() == GitilesView.Type.ROOTED_DOC) { - log.error( - String.format( - "markdown too large: %s/%s %s %s: %s", - view.getHostName(), - view.getRepositoryName(), - view.getRevision(), - view.getPathPart(), - errBig.getMessage())); - res.setStatus(SC_INTERNAL_SERVER_ERROR); + throw new GitilesRequestFailureException(FailureReason.OBJECT_TOO_LARGE); } else { res.sendRedirect(GitilesView.show().copyFrom(view).toUrl()); } } - private static void readError(HttpServletResponse res, GitilesView view, IOException err) { - log.error( - String.format( - "cannot load markdown %s/%s %s %s", - view.getHostName(), view.getRepositoryName(), view.getRevision(), view.getPathPart()), - err); - res.setStatus(SC_INTERNAL_SERVER_ERROR); - } - private static class MarkdownFile { final String path; final ObjectId id;
diff --git a/javatests/com/google/gitiles/DefaultErrorHandlingFilterTest.java b/javatests/com/google/gitiles/DefaultErrorHandlingFilterTest.java new file mode 100644 index 0000000..6832658 --- /dev/null +++ b/javatests/com/google/gitiles/DefaultErrorHandlingFilterTest.java
@@ -0,0 +1,43 @@ +package com.google.gitiles; + +import static com.google.common.truth.Truth.assertThat; +import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; + +import com.google.gitiles.GitilesRequestFailureException.FailureReason; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.eclipse.jgit.http.server.glue.MetaFilter; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class DefaultErrorHandlingFilterTest { + private final MetaFilter mf = new MetaFilter(); + + @Before + public void setUp() { + mf.serve("*").through(new DefaultErrorHandlingFilter()).with(new TestServlet()); + } + + @Test + public void renderError() throws Exception { + FakeHttpServletRequest req = FakeHttpServletRequest.newRequest(); + req.setPathInfo("/"); + FakeHttpServletResponse resp = new FakeHttpServletResponse(); + mf.doFilter(req, resp, (unusedReq, unusedResp) -> {}); + + assertThat(resp.getStatus()).isEqualTo(SC_BAD_REQUEST); + assertThat(resp.getHeader(DefaultErrorHandlingFilter.GITILES_ERROR)) + .isEqualTo("INCORECT_PARAMETER"); + } + + private static class TestServlet extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse res) { + throw new GitilesRequestFailureException(FailureReason.INCORECT_PARAMETER); + } + } +}
diff --git a/javatests/com/google/gitiles/MoreAssert.java b/javatests/com/google/gitiles/MoreAssert.java new file mode 100644 index 0000000..bd80d69 --- /dev/null +++ b/javatests/com/google/gitiles/MoreAssert.java
@@ -0,0 +1,38 @@ +// Copyright 2019 Google LLC +// +// 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 +// +// https://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.gitiles; + +/** Assertion methods for Gitiles. */ +public class MoreAssert { + private MoreAssert() {} + + /** Simple version of assertThrows that will be introduced in JUnit 4.13. */ + public static <T extends Throwable> T assertThrows(Class<T> expected, ThrowingRunnable r) { + try { + r.run(); + throw new AssertionError("Expected " + expected.getSimpleName() + " to be thrown"); + } catch (Throwable actual) { + if (expected.isAssignableFrom(actual.getClass())) { + return (T) actual; + } + throw new AssertionError( + "Expected " + expected.getSimpleName() + ", but got " + actual.getClass().getSimpleName(), + actual); + } + } + + public interface ThrowingRunnable { + void run() throws Throwable; + } +}
diff --git a/javatests/com/google/gitiles/ViewFilterTest.java b/javatests/com/google/gitiles/ViewFilterTest.java index 2a8cb65..d25b6cf 100644 --- a/javatests/com/google/gitiles/ViewFilterTest.java +++ b/javatests/com/google/gitiles/ViewFilterTest.java
@@ -16,6 +16,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static com.google.gitiles.MoreAssert.assertThrows; import com.google.common.net.HttpHeaders; import com.google.gitiles.GitilesView.Type; @@ -45,8 +46,8 @@ public void noCommand() throws Exception { assertThat(getView("/").getType()).isEqualTo(Type.HOST_INDEX); assertThat(getView("/repo").getType()).isEqualTo(Type.REPOSITORY_INDEX); - assertThat(getView("/repo/+")).isNull(); - assertThat(getView("/repo/+/")).isNull(); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+")); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+/")); } @Test @@ -136,8 +137,8 @@ public void describe() throws Exception { GitilesView view; - assertThat(getView("/repo/+describe")).isNull(); - assertThat(getView("/repo/+describe/")).isNull(); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+describe")); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+describe/")); view = getView("/repo/+describe/deadbeef"); assertThat(view.getType()).isEqualTo(Type.DESCRIBE); @@ -184,7 +185,7 @@ assertThat(view.getRevision().getId()).isEqualTo(stable); assertThat(view.getPathPart()).isNull(); - assertThat(getView("/repo/+show/stable..master")).isNull(); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+show/stable..master")); } @Test @@ -250,7 +251,7 @@ assertThat(view.getRevision().getId()).isEqualTo(master); assertThat(view.getPathPart()).isEqualTo("foo/bar"); - assertThat(getView("/repo/+show/stable..master/foo")).isNull(); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+show/stable..master/foo")); } @Test @@ -279,7 +280,7 @@ assertThat(view.getRevision().getId()).isEqualTo(master); assertThat(view.getPathPart()).isEqualTo("foo/bar.md"); - assertThat(getView("/repo/+doc/stable..master/foo")).isNull(); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+doc/stable..master/foo")); } @Test @@ -288,10 +289,10 @@ assertThat(getView("//").getType()).isEqualTo(Type.HOST_INDEX); assertThat(getView("//repo").getType()).isEqualTo(Type.REPOSITORY_INDEX); assertThat(getView("//repo//").getType()).isEqualTo(Type.REPOSITORY_INDEX); - assertThat(getView("/repo/+//master")).isNull(); - assertThat(getView("/repo/+/refs//heads//master")).isNull(); - assertThat(getView("/repo/+//master//")).isNull(); - assertThat(getView("/repo/+//master/foo//bar")).isNull(); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+//master")); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+/refs//heads//master")); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+//master//")); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+//master/foo//bar")); } @Test @@ -420,13 +421,13 @@ repo.branch("refs/heads/branch").commit().create(); GitilesView view; - assertThat(getView("/repo/+archive")).isNull(); - assertThat(getView("/repo/+archive/")).isNull(); - assertThat(getView("/repo/+archive/master..branch")).isNull(); - assertThat(getView("/repo/+archive/master.foo")).isNull(); - assertThat(getView("/repo/+archive/master.zip")).isNull(); - assertThat(getView("/repo/+archive/master/.tar.gz")).isNull(); - assertThat(getView("/repo/+archive/master/foo/.tar.gz")).isNull(); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+archive")); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+archive/")); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+archive/master..branch")); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+archive/master.foo")); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+archive/master.zip")); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+archive/master/.tar.gz")); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+archive/master/foo/.tar.gz")); view = getView("/repo/+archive/master.tar.gz"); assertThat(view.getType()).isEqualTo(Type.ARCHIVE); @@ -462,10 +463,10 @@ repo.branch("refs/heads/branch").commit().create(); GitilesView view; - assertThat(getView("/repo/+blame")).isNull(); - assertThat(getView("/repo/+blame/")).isNull(); - assertThat(getView("/repo/+blame/master")).isNull(); - assertThat(getView("/repo/+blame/master..branch")).isNull(); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+blame")); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+blame/")); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+blame/master")); + assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+blame/master..branch")); view = getView("/repo/+blame/master/foo/bar"); assertThat(view.getType()).isEqualTo(Type.BLAME);