Skip to content

Commit

Permalink
Harmonize http status code usage
Browse files Browse the repository at this point in the history
Given that we run servlets, it makes sense to always use the status
code contants from the servlet class.
  • Loading branch information
jianglai committed May 24, 2024
1 parent 0781010 commit 8bbd572
Show file tree
Hide file tree
Showing 28 changed files with 157 additions and 175 deletions.
23 changes: 8 additions & 15 deletions config/presubmits.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,23 +173,11 @@ def fails(self, file):
):
"JavaScript files should not include console logging.",
PresubmitCheck(
r".*org\.testcontainers\.shaded.*",
r".*\nimport (static )?.*\.shaded\..*",
"java",
{"/node_modules/"},
):
"Do not use shaded dependencies from testcontainers.",
PresubmitCheck(
r".*autovalue\.shaded.*",
"java",
{"/node_modules/"},
):
"Do not use shaded dependencies from autovalue.",
PresubmitCheck(
r".*avro\.shaded.*",
"java",
{"/node_modules/"},
):
"Do not use shaded dependencies from avro.",
"Do not use shaded dependencies",
PresubmitCheck(
r".*com\.google\.common\.truth\.Truth8.*",
"java",
Expand All @@ -202,7 +190,12 @@ def fails(self, file):
{"/node_modules/", "JpaTransactionManagerImpl.java"},
):
"Do not use java.util.Date. Use classes in java.time package instead.",

PresubmitCheck(
r".*com\.google\.api\.client\.http\.HttpStatusCodes.*",
"java",
{"/node_modules/"},
):
"Use status code from jakarta.servlet.http.HttpServletResponse.",
}

# Note that this regex only works for one kind of Flyway file. If we want to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
import static google.registry.persistence.transaction.TransactionManagerFactory.replicaTm;
import static google.registry.request.Action.Method.GET;
import static google.registry.request.Action.Method.POST;
import static jakarta.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
import static java.nio.charset.StandardCharsets.US_ASCII;

import com.google.api.client.http.HttpStatusCodes;
import com.google.cloud.storage.BlobId;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -190,7 +190,7 @@ boolean uploadToBsa(String unavailableDomains, DateTime runTime) {
return true;
} catch (IOException e) {
logger.atSevere().withCause(e).log("Error while attempting to upload to BSA, aborting.");
response.setStatus(HttpStatusCodes.STATUS_CODE_SERVER_ERROR);
response.setStatus(SC_INTERNAL_SERVER_ERROR);
response.setPayload("Error while attempting to upload to BSA: " + e.getMessage());
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@

package google.registry.rdap;

import static com.google.api.client.http.HttpStatusCodes.STATUS_CODE_OK;
import static com.google.common.net.HttpHeaders.ACCEPT_ENCODING;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.request.UrlConnectionUtils.gUnzipBytes;
import static google.registry.request.UrlConnectionUtils.isGZipped;
import static jakarta.servlet.http.HttpServletResponse.SC_OK;
import static java.nio.charset.StandardCharsets.UTF_8;

import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -114,7 +114,7 @@ private ImmutableMap<String, String> getIanaIdsToUrls()
connection.setRequestProperty(ACCEPT_ENCODING, "gzip");
String csvString;
try {
if (connection.getResponseCode() != STATUS_CODE_OK) {
if (connection.getResponseCode() != SC_OK) {
throw new UrlConnectionException("Failed to load RDAP base URLs from ICANN", connection);
}
// With GZIP encoding header in the request (see above) ICANN had still sent response in plain
Expand Down
8 changes: 4 additions & 4 deletions core/src/main/java/google/registry/rde/RdeReporter.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@

package google.registry.rde;

import static com.google.api.client.http.HttpStatusCodes.STATUS_CODE_BAD_REQUEST;
import static com.google.api.client.http.HttpStatusCodes.STATUS_CODE_OK;
import static google.registry.request.UrlConnectionUtils.getResponseBytes;
import static google.registry.request.UrlConnectionUtils.setBasicAuth;
import static google.registry.request.UrlConnectionUtils.setPayload;
import static google.registry.util.DomainNameUtils.canonicalizeHostname;
import static jakarta.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import static jakarta.servlet.http.HttpServletResponse.SC_OK;
import static java.nio.charset.StandardCharsets.UTF_8;

import com.google.api.client.http.HttpMethods;
Expand Down Expand Up @@ -85,7 +85,7 @@ public void send(byte[] reportBytes) throws XmlException, GeneralSecurityExcepti

try {
responseCode = connection.getResponseCode();
if (responseCode != STATUS_CODE_OK && responseCode != STATUS_CODE_BAD_REQUEST) {
if (responseCode != SC_OK && responseCode != SC_BAD_REQUEST) {
logger.atWarning().log("Connection to RDE report server failed: %d", responseCode);
throw new UrlConnectionException("PUT failed", connection);
}
Expand All @@ -96,7 +96,7 @@ public void send(byte[] reportBytes) throws XmlException, GeneralSecurityExcepti

// We know that an HTTP 200 response can only contain a result code of
// 1000 (i. e. success), there is no need to parse it.
if (responseCode != STATUS_CODE_OK) {
if (responseCode != SC_OK) {
XjcIirdeaResult result = parseResult(responseBytes);
logger.atWarning().log(
"Rejected when trying to PUT RDE report to ICANN server: %d %s\n%s",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@

package google.registry.reporting.icann;

import static com.google.api.client.http.HttpStatusCodes.STATUS_CODE_OK;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.net.MediaType.CSV_UTF_8;
import static google.registry.model.tld.Tlds.assertTldExists;
import static jakarta.servlet.http.HttpServletResponse.SC_OK;

import com.google.api.client.http.HttpMethods;
import com.google.common.base.Ascii;
Expand Down Expand Up @@ -95,7 +95,7 @@ public boolean send(byte[] reportBytes, String reportFilename)
try {
responseCode = connection.getResponseCode();
content = UrlConnectionUtils.getResponseBytes(connection);
if (responseCode != STATUS_CODE_OK) {
if (responseCode != SC_OK) {
XjcIirdeaResult result = parseResult(content);
logger.atWarning().log(
"PUT rejected, status code %s:\n%s\n%s",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@
package google.registry.ui.server.console;

import static google.registry.request.Action.Method.GET;
import static jakarta.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import static jakarta.servlet.http.HttpServletResponse.SC_FORBIDDEN;
import static jakarta.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
import static jakarta.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;

import com.google.api.client.http.HttpStatusCodes;
import com.google.common.base.Throwables;
import com.google.common.flogger.FluentLogger;
import google.registry.model.console.ConsolePermission;
Expand Down Expand Up @@ -50,7 +53,7 @@ public final void run() {
AuthResult authResult = consoleApiParams.authResult();
if (authResult.userAuthInfo().isEmpty()
|| authResult.userAuthInfo().get().consoleUser().isEmpty()) {
consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_UNAUTHORIZED);
consoleApiParams.response().setStatus(SC_UNAUTHORIZED);
return;
}
User user = consoleApiParams.authResult().userAuthInfo().get().consoleUser().get();
Expand All @@ -77,15 +80,13 @@ public final void run() {
}
} catch (ConsolePermissionForbiddenException e) {
logger.atWarning().withCause(e).log("Forbidden");
setFailedResponse("", HttpStatusCodes.STATUS_CODE_FORBIDDEN);
setFailedResponse("", SC_FORBIDDEN);
} catch (HttpException.BadRequestException | IllegalArgumentException e) {
logger.atWarning().withCause(e).log("Error in request");
setFailedResponse(
Throwables.getRootCause(e).getMessage(), HttpStatusCodes.STATUS_CODE_BAD_REQUEST);
setFailedResponse(Throwables.getRootCause(e).getMessage(), SC_BAD_REQUEST);
} catch (Throwable t) {
logger.atWarning().withCause(t).log("Internal server error");
setFailedResponse(
Throwables.getRootCause(t).getMessage(), HttpStatusCodes.STATUS_CODE_SERVER_ERROR);
setFailedResponse(Throwables.getRootCause(t).getMessage(), SC_INTERNAL_SERVER_ERROR);
}
}

Expand Down Expand Up @@ -118,7 +119,7 @@ private boolean verifyXSRF() {
.findFirst();
if (maybeCookie.isEmpty()
|| !consoleApiParams.xsrfTokenManager().validateToken(maybeCookie.get().getValue())) {
consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_UNAUTHORIZED);
consoleApiParams.response().setStatus(SC_UNAUTHORIZED);
return false;
}
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
package google.registry.ui.server.console;

import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static jakarta.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import static jakarta.servlet.http.HttpServletResponse.SC_OK;

import com.google.api.client.http.HttpStatusCodes;
import com.google.gson.Gson;
import google.registry.model.EppResourceUtils;
import google.registry.model.console.ConsolePermission;
Expand Down Expand Up @@ -59,16 +60,16 @@ protected void getHandler(User user) {
EppResourceUtils.loadByForeignKeyCached(
Domain.class, paramDomain, tm().getTransactionTime()));
if (possibleDomain.isEmpty()) {
consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_NOT_FOUND);
consoleApiParams.response().setStatus(SC_NOT_FOUND);
return;
}
Domain domain = possibleDomain.get();
if (!user.getUserRoles()
.hasPermission(domain.getCurrentSponsorRegistrarId(), ConsolePermission.DOWNLOAD_DOMAINS)) {
consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_NOT_FOUND);
consoleApiParams.response().setStatus(SC_NOT_FOUND);
return;
}
consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_OK);
consoleApiParams.response().setStatus(SC_OK);
consoleApiParams.response().setPayload(gson.toJson(domain));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
import static com.google.common.base.Preconditions.checkArgument;
import static google.registry.model.console.ConsolePermission.DOWNLOAD_DOMAINS;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static jakarta.servlet.http.HttpServletResponse.SC_OK;

import com.google.api.client.http.HttpStatusCodes;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Ascii;
import com.google.gson.Gson;
Expand Down Expand Up @@ -118,7 +118,7 @@ private void runInTransaction() {
consoleApiParams
.response()
.setPayload(gson.toJson(new DomainListResult(domains, checkpoint, actualTotalResults)));
consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_OK);
consoleApiParams.response().setStatus(SC_OK);
}

/** Creates the query to get the total number of matching domains, interpolating as necessary. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
import static com.google.common.base.Preconditions.checkArgument;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.request.Action.Method.POST;
import static jakarta.servlet.http.HttpServletResponse.SC_FORBIDDEN;
import static jakarta.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import static jakarta.servlet.http.HttpServletResponse.SC_OK;

import com.google.api.client.http.HttpStatusCodes;
import com.google.common.base.Strings;
import com.google.gson.annotations.Expose;
import google.registry.flows.EppException.AuthenticationErrorException;
Expand Down Expand Up @@ -92,14 +94,14 @@ protected void postHandler(User user) {
try {
registrar = registrarAccessor.getRegistrar(eppRequestBody.registrarId());
} catch (RegistrarAccessDeniedException e) {
setFailedResponse(e.getMessage(), HttpStatusCodes.STATUS_CODE_NOT_FOUND);
setFailedResponse(e.getMessage(), SC_NOT_FOUND);
return;
}

try {
credentials.validate(registrar, eppRequestBody.oldPassword());
} catch (AuthenticationErrorException e) {
setFailedResponse(e.getMessage(), HttpStatusCodes.STATUS_CODE_FORBIDDEN);
setFailedResponse(e.getMessage(), SC_FORBIDDEN);
return;
}

Expand All @@ -113,7 +115,7 @@ protected void postHandler(User user) {
new InternetAddress(registrar.getEmailAddress(), true)));
});

consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_OK);
consoleApiParams.response().setStatus(SC_OK);
}

public record EppPasswordData(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@
import static google.registry.request.RequestParameters.extractOptionalParameter;
import static google.registry.request.RequestParameters.extractRequiredParameter;
import static google.registry.ui.server.registrar.RegistryLockPostAction.VERIFICATION_EMAIL_TEMPLATE;
import static jakarta.servlet.http.HttpServletResponse.SC_OK;
import static jakarta.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;

import com.google.api.client.http.HttpStatusCodes;
import com.google.common.collect.ImmutableList;
import com.google.gson.Gson;
import google.registry.flows.EppException;
Expand Down Expand Up @@ -90,7 +91,7 @@ public ConsoleRegistryLockAction(
protected void getHandler(User user) {
checkPermission(user, registrarId, ConsolePermission.REGISTRY_LOCK);
consoleApiParams.response().setPayload(gson.toJson(getLockedDomains()));
consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_OK);
consoleApiParams.response().setStatus(SC_OK);
}

@Override
Expand Down Expand Up @@ -124,8 +125,7 @@ protected void postHandler(User user) {
if (!isAdmin) {
checkArgument(maybePassword.isPresent(), "No password provided");
if (!user.verifyRegistryLockPassword(maybePassword.get())) {
setFailedResponse(
"Incorrect registry lock password", HttpStatusCodes.STATUS_CODE_UNAUTHORIZED);
setFailedResponse("Incorrect registry lock password", SC_UNAUTHORIZED);
return;
}
}
Expand All @@ -147,7 +147,7 @@ protected void postHandler(User user) {
relockDurationMillis.map(Duration::new));
sendVerificationEmail(registryLock, registryLockEmail, isLock);
});
response.setStatus(HttpStatusCodes.STATUS_CODE_OK);
response.setStatus(SC_OK);
}

private void sendVerificationEmail(RegistryLock lock, String userEmail, boolean isLock) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
package google.registry.ui.server.console;

import static google.registry.request.Action.Method.GET;
import static jakarta.servlet.http.HttpServletResponse.SC_OK;

import com.google.api.client.http.HttpStatusCodes;
import com.google.common.collect.ImmutableMap;
import google.registry.config.RegistryConfig.Config;
import google.registry.model.console.User;
Expand Down Expand Up @@ -88,6 +88,6 @@ protected void getHandler(User user) {
"technicalDocsUrl", technicalDocsUrl));

consoleApiParams.response().setPayload(json.toString());
consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_OK);
consoleApiParams.response().setStatus(SC_OK);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.request.Action.Method.GET;
import static google.registry.request.Action.Method.POST;
import static jakarta.servlet.http.HttpServletResponse.SC_FORBIDDEN;
import static jakarta.servlet.http.HttpServletResponse.SC_OK;

import com.google.api.client.http.HttpStatusCodes;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Streams;
import com.google.gson.Gson;
Expand Down Expand Up @@ -72,7 +73,7 @@ public RegistrarsAction(
@Override
protected void getHandler(User user) {
if (!user.getUserRoles().hasGlobalPermission(ConsolePermission.VIEW_REGISTRARS)) {
consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_FORBIDDEN);
consoleApiParams.response().setStatus(SC_FORBIDDEN);
return;
}

Expand All @@ -82,13 +83,13 @@ protected void getHandler(User user) {
.collect(ImmutableList.toImmutableList());

consoleApiParams.response().setPayload(gson.toJson(registrars));
consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_OK);
consoleApiParams.response().setStatus(SC_OK);
}

@Override
protected void postHandler(User user) {
if (!user.getUserRoles().isAdmin()) {
consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_FORBIDDEN);
consoleApiParams.response().setStatus(SC_FORBIDDEN);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.request.Action.Method.GET;
import static google.registry.request.Action.Method.POST;
import static jakarta.servlet.http.HttpServletResponse.SC_OK;

import com.google.api.client.http.HttpStatusCodes;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
Expand Down Expand Up @@ -78,7 +78,7 @@ protected void getHandler(User user) {
.filter(r -> !r.getTypes().isEmpty())
.collect(toImmutableList()));

consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_OK);
consoleApiParams.response().setStatus(SC_OK);
consoleApiParams.response().setPayload(gson.toJson(am));
}

Expand Down Expand Up @@ -112,6 +112,6 @@ protected void postHandler(User user) {
}

RegistrarPoc.updateContacts(registrar, updatedContacts);
consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_OK);
consoleApiParams.response().setStatus(SC_OK);
}
}
Loading

0 comments on commit 8bbd572

Please sign in to comment.