diff --git a/Documentation/quota.txt b/Documentation/quota.txt index 611bba842d..a647e333fe 100644 --- a/Documentation/quota.txt +++ b/Documentation/quota.txt @@ -14,8 +14,33 @@ associated. The following quota groups are defined in core Gerrit: === REST API +[[rest-api]] -TODO(hiesel,luca.milanesio,matthias.sohn,david.pursehouse): Add more :) +The REST API enforces quota after the resource was parsed (if applicable) and before the +endpoint's logic is executed. This enables quota enforcer implementations to throttle calls +to specific endpoints while knowing the general context (user and top-level entity such as +change, project or account). + +If the quota enforcer wants to throttle HTTP requests, they should use +link:quota.html#http-requests[HTTP Requests] instead. + +The quota groups used for checking follow the exact definition of the endoint in the REST +API, but remove all IDs. The schema is: + +/restapi/: + +Examples: + +[options="header",cols="1,6"] +|======================= +|HTTP call |Quota Group |Metadata +|GET /a/changes/1/revisions/current/detail |/changes/revisions/detail:GET |CurrentUser, Change.Id, Project.NameKey +|POST /a/changes/ |/changes/:POST |CurrentUser +|GET /a/accounts/self/detail |/accounts/detail:GET |CurrentUser, Account.Id +|======================= + +The user provided in the check's metadata is always the calling user (having the +impersonation bit and real user set in case the user is impersonating another user). GERRIT ------ diff --git a/Documentation/rest-api.txt b/Documentation/rest-api.txt index 8f6a47b233..a8ab35347a 100644 --- a/Documentation/rest-api.txt +++ b/Documentation/rest-api.txt @@ -191,6 +191,11 @@ Preconditions] section. "`422 Unprocessable Entity`" is returned if the ID of a resource that is specified in the request body cannot be resolved. +==== 429 Too Many Requests +"`429 Too Many Requests`" is returned if the request exhausted any set +quota limits. Depending on the exhausted quota, the request may be retried +with exponential backoff. + [[tracing]] === Request Tracing For each REST endpoint tracing can be enabled by setting the diff --git a/java/com/google/gerrit/httpd/restapi/RestApiQuotaEnforcer.java b/java/com/google/gerrit/httpd/restapi/RestApiQuotaEnforcer.java new file mode 100644 index 0000000000..c7678c2c86 --- /dev/null +++ b/java/com/google/gerrit/httpd/restapi/RestApiQuotaEnforcer.java @@ -0,0 +1,83 @@ +// Copyright (C) 2018 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.httpd.restapi; + +import com.google.gerrit.extensions.restapi.RestResource; +import com.google.gerrit.server.account.AccountResource; +import com.google.gerrit.server.change.ChangeResource; +import com.google.gerrit.server.project.ProjectResource; +import com.google.gerrit.server.quota.QuotaBackend; +import com.google.gerrit.server.quota.QuotaException; +import com.google.gerrit.util.http.RequestUtil; +import javax.inject.Inject; +import javax.servlet.http.HttpServletRequest; + +/** + * Enforces quota on specific REST API endpoints. + * + *

Examples: + * + *

+ * + *

Adds context (change, project, account) to the quota check if the call is for an existing + * entity that was successfully parsed. This quota check is generally enforced after the resource + * was parsed, but before the view is executed. If a quota enforcer desires to throttle earlier, + * they should consider quota groups in the {@code /http/*} space. + */ +public class RestApiQuotaEnforcer { + private final QuotaBackend quotaBackend; + + @Inject + RestApiQuotaEnforcer(QuotaBackend quotaBackend) { + this.quotaBackend = quotaBackend; + } + + /** Enforce quota on a request not tied to any {@code RestResource}. */ + void enforce(HttpServletRequest req) throws QuotaException { + String pathForQuotaReporting = RequestUtil.getRestPathWithoutIds(req); + quotaBackend + .currentUser() + .requestToken(quotaGroup(pathForQuotaReporting, req.getMethod())) + .throwOnError(); + } + + /** Enforce quota on a request for a given resource. */ + void enforce(RestResource rsrc, HttpServletRequest req) throws QuotaException { + String pathForQuotaReporting = RequestUtil.getRestPathWithoutIds(req); + // Enrich the quota request we are operating on an interesting collection + QuotaBackend.WithResource report = quotaBackend.currentUser(); + if (rsrc instanceof ChangeResource) { + ChangeResource changeResource = (ChangeResource) rsrc; + report = + quotaBackend.currentUser().change(changeResource.getId(), changeResource.getProject()); + } else if (rsrc instanceof AccountResource) { + AccountResource accountResource = (AccountResource) rsrc; + report = quotaBackend.currentUser().account(accountResource.getUser().getAccountId()); + } else if (rsrc instanceof ProjectResource) { + ProjectResource accountResource = (ProjectResource) rsrc; + report = quotaBackend.currentUser().account(accountResource.getUser().getAccountId()); + } + + report.requestToken(quotaGroup(pathForQuotaReporting, req.getMethod())).throwOnError(); + } + + private static String quotaGroup(String path, String method) { + return "/restapi" + path + ":" + method; + } +} diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index ec71d8fea9..c9f3a77e3b 100644 --- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -114,6 +114,7 @@ import com.google.gerrit.server.logging.TraceContext; import com.google.gerrit.server.permissions.GlobalPermission; import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackendException; +import com.google.gerrit.server.quota.QuotaException; import com.google.gerrit.server.update.UpdateException; import com.google.gerrit.server.util.time.TimeUtil; import com.google.gerrit.util.http.CacheHeaders; @@ -227,6 +228,7 @@ public class RestApiServlet extends HttpServlet { final AuditService auditService; final RestApiMetrics metrics; final Pattern allowOrigin; + final RestApiQuotaEnforcer quotaChecker; @Inject Globals( @@ -236,6 +238,7 @@ public class RestApiServlet extends HttpServlet { PermissionBackend permissionBackend, AuditService auditService, RestApiMetrics metrics, + RestApiQuotaEnforcer quotaChecker, @GerritServerConfig Config cfg) { this.currentUser = currentUser; this.webSession = webSession; @@ -243,6 +246,7 @@ public class RestApiServlet extends HttpServlet { this.permissionBackend = permissionBackend; this.auditService = auditService; this.metrics = metrics; + this.quotaChecker = quotaChecker; allowOrigin = makeAllowOrigin(cfg); } @@ -317,6 +321,7 @@ public class RestApiServlet extends HttpServlet { viewData = new ViewData(null, null); if (path.isEmpty()) { + globals.quotaChecker.enforce(req); if (rc instanceof NeedsParams) { ((NeedsParams) rc).setParams(qp.params()); } @@ -339,6 +344,7 @@ public class RestApiServlet extends HttpServlet { IdString id = path.remove(0); try { rsrc = rc.parse(rsrc, id); + globals.quotaChecker.enforce(rsrc, req); if (path.isEmpty()) { checkPreconditions(req); } @@ -346,6 +352,7 @@ public class RestApiServlet extends HttpServlet { if (!path.isEmpty()) { throw e; } + globals.quotaChecker.enforce(req); if (isPost(req) || isPut(req)) { RestView createView = rc.views().get(PluginName.GERRIT, "CREATE./"); @@ -602,6 +609,9 @@ public class RestApiServlet extends HttpServlet { status = SC_INTERNAL_SERVER_ERROR; responseBytes = handleException(e, req, res); } + } catch (QuotaException e) { + responseBytes = + replyError(req, res, status = 429, messageOr(e, "Quota limit reached"), e.caching(), e); } catch (Exception e) { status = SC_INTERNAL_SERVER_ERROR; responseBytes = handleException(e, req, res); diff --git a/java/com/google/gerrit/util/http/RequestUtil.java b/java/com/google/gerrit/util/http/RequestUtil.java index 2a359cacd0..2543adaa74 100644 --- a/java/com/google/gerrit/util/http/RequestUtil.java +++ b/java/com/google/gerrit/util/http/RequestUtil.java @@ -56,5 +56,38 @@ public class RequestUtil { return pathInfo; } + /** + * Trims leading '/' and 'a/'. Removes the context path, but keeps the servlet path. Removes all + * IDs from the rest of the URI. + * + *

The returned string is a good fit for cases where one wants the full context of the request + * without any identifiable data. For example: Logging or quota checks. + * + *

Examples: + * + *

+ */ + public static String getRestPathWithoutIds(HttpServletRequest req) { + String encodedPathInfo = req.getRequestURI().substring(req.getContextPath().length()); + if (encodedPathInfo.startsWith("/")) { + encodedPathInfo = encodedPathInfo.substring(1); + } + if (encodedPathInfo.startsWith("a/")) { + encodedPathInfo = encodedPathInfo.substring(2); + } + + String[] parts = encodedPathInfo.split("/"); + StringBuilder result = new StringBuilder(parts.length); + for (int i = 0; i < parts.length; i = i + 2) { + result.append("/"); + result.append(parts[i]); + } + return result.toString(); + } + private RequestUtil() {} } diff --git a/javatests/com/google/gerrit/util/http/RequestUtilTest.java b/javatests/com/google/gerrit/util/http/RequestUtilTest.java index 0bf34e7026..adda5e7092 100644 --- a/javatests/com/google/gerrit/util/http/RequestUtilTest.java +++ b/javatests/com/google/gerrit/util/http/RequestUtilTest.java @@ -15,6 +15,8 @@ package com.google.gerrit.util.http; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.util.http.RequestUtil.getEncodedPathInfo; +import static com.google.gerrit.util.http.RequestUtil.getRestPathWithoutIds; import com.google.gerrit.testing.GerritBaseTests; import com.google.gerrit.util.http.testutil.FakeHttpServletRequest; @@ -22,36 +24,45 @@ import org.junit.Test; public class RequestUtilTest extends GerritBaseTests { @Test - public void emptyContextPath() { - assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("", "/s", "/foo/bar"))) - .isEqualTo("/foo/bar"); - assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("", "/s", "/foo%2Fbar"))) - .isEqualTo("/foo%2Fbar"); + public void getEncodedPathInfo_emptyContextPath() { + assertThat(getEncodedPathInfo(fakeRequest("", "/s", "/foo/bar"))).isEqualTo("/foo/bar"); + assertThat(getEncodedPathInfo(fakeRequest("", "/s", "/foo%2Fbar"))).isEqualTo("/foo%2Fbar"); } @Test - public void emptyServletPath() { - assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("", "/c", "/foo/bar"))) - .isEqualTo("/foo/bar"); - assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("", "/c", "/foo%2Fbar"))) - .isEqualTo("/foo%2Fbar"); + public void getEncodedPathInfo_emptyServletPath() { + assertThat(getEncodedPathInfo(fakeRequest("", "/c", "/foo/bar"))).isEqualTo("/foo/bar"); + assertThat(getEncodedPathInfo(fakeRequest("", "/c", "/foo%2Fbar"))).isEqualTo("/foo%2Fbar"); } @Test - public void trailingSlashes() { - assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("/c", "/s", "/foo/bar/"))) - .isEqualTo("/foo/bar/"); - assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("/c", "/s", "/foo/bar///"))) - .isEqualTo("/foo/bar/"); - assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("/c", "/s", "/foo%2Fbar/"))) - .isEqualTo("/foo%2Fbar/"); - assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("/c", "/s", "/foo%2Fbar///"))) + public void getEncodedPathInfo_trailingSlashes() { + assertThat(getEncodedPathInfo(fakeRequest("/c", "/s", "/foo/bar/"))).isEqualTo("/foo/bar/"); + assertThat(getEncodedPathInfo(fakeRequest("/c", "/s", "/foo/bar///"))).isEqualTo("/foo/bar/"); + assertThat(getEncodedPathInfo(fakeRequest("/c", "/s", "/foo%2Fbar/"))).isEqualTo("/foo%2Fbar/"); + assertThat(getEncodedPathInfo(fakeRequest("/c", "/s", "/foo%2Fbar///"))) .isEqualTo("/foo%2Fbar/"); } @Test public void emptyPathInfo() { - assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("/c", "/s", ""))).isNull(); + assertThat(getEncodedPathInfo(fakeRequest("/c", "/s", ""))).isNull(); + } + + @Test + public void getRestPathWithoutIds_emptyContextPath() { + assertThat(getRestPathWithoutIds(fakeRequest("", "/a/accounts", "/123/test"))) + .isEqualTo("/accounts/test"); + assertThat(getRestPathWithoutIds(fakeRequest("", "/accounts", "/123/test"))) + .isEqualTo("/accounts/test"); + } + + @Test + public void getRestPathWithoutIds_nonEmptyContextPath() { + assertThat(getRestPathWithoutIds(fakeRequest("/c", "/a/accounts", "/123/test"))) + .isEqualTo("/accounts/test"); + assertThat(getRestPathWithoutIds(fakeRequest("/c", "/accounts", "/123/test"))) + .isEqualTo("/accounts/test"); } private FakeHttpServletRequest fakeRequest(