From bf0f553ced8d1e11d42869d288f7f339d1414201 Mon Sep 17 00:00:00 2001 From: Ravi Khadiwala Date: Thu, 19 Sep 2024 16:13:20 -0500 Subject: [PATCH] Don't return 204s as exceptions The jersey LoggingExceptionMapper automatically adds an entity to WebApplicationExceptions. Jersey's HTTP server later strips the body on 204 responses, but our custom WebSocketResourceProvider does not --- .../controllers/OneTimeDonationController.java | 13 +++++++------ .../controllers/SubscriptionController.java | 6 +++++- .../storage/SubscriptionException.java | 9 +++++++++ .../subscriptions/BraintreeManager.java | 6 ++++-- .../textsecuregcm/subscriptions/StripeManager.java | 8 ++++++-- 5 files changed, 31 insertions(+), 11 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/OneTimeDonationController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/OneTimeDonationController.java index 1f63a8fd6..d403f02b0 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/OneTimeDonationController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/OneTimeDonationController.java @@ -54,6 +54,7 @@ import org.whispersystems.textsecuregcm.subscriptions.BraintreeManager; import org.whispersystems.textsecuregcm.subscriptions.ChargeFailure; import org.whispersystems.textsecuregcm.subscriptions.PaymentDetails; import org.whispersystems.textsecuregcm.subscriptions.PaymentMethod; +import org.whispersystems.textsecuregcm.subscriptions.PaymentStatus; import org.whispersystems.textsecuregcm.subscriptions.StripeManager; import org.whispersystems.textsecuregcm.subscriptions.SubscriptionCurrencyUtil; import org.whispersystems.textsecuregcm.subscriptions.PaymentProvider; @@ -324,15 +325,15 @@ public class OneTimeDonationController { return paymentDetailsFut.thenCompose(paymentDetails -> { if (paymentDetails == null) { throw new WebApplicationException(Response.Status.NOT_FOUND); - } - switch (paymentDetails.status()) { - case PROCESSING -> throw new WebApplicationException(Response.Status.NO_CONTENT); - case SUCCEEDED -> { - } - default -> throw new WebApplicationException(Response.status(Response.Status.PAYMENT_REQUIRED) + } else if (paymentDetails.status() == PaymentStatus.PROCESSING) { + return CompletableFuture.completedFuture(Response.noContent().build()); + } else if (paymentDetails.status() != PaymentStatus.SUCCEEDED) { + throw new WebApplicationException(Response.status(Response.Status.PAYMENT_REQUIRED) .entity(new CreateBoostReceiptCredentialsErrorResponse(paymentDetails.chargeFailure())).build()); } + // The payment was successful, try to issue the receipt credential + long level = oneTimeDonationConfiguration.boost().level(); if (paymentDetails.customMetadata() != null) { String levelMetadata = paymentDetails.customMetadata() diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/SubscriptionController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/SubscriptionController.java index f3e73b208..0e0255c49 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/SubscriptionController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/SubscriptionController.java @@ -51,6 +51,7 @@ import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.Produces; import javax.ws.rs.QueryParam; +import javax.ws.rs.WebApplicationException; import javax.ws.rs.container.ContainerRequestContext; import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; @@ -631,7 +632,10 @@ public class SubscriptionController { UserAgentTagUtil.getPlatformTag(userAgent))) .increment(); return Response.ok(new GetReceiptCredentialsResponse(receiptCredentialResponse.serialize())).build(); - }); + }) + .exceptionally(ExceptionUtils.exceptionallyHandler( + SubscriptionException.ReceiptRequestedForOpenPayment.class, + e -> Response.noContent().build())); } @POST diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/SubscriptionException.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/SubscriptionException.java index 255ce2ad6..7eb48e0a0 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/SubscriptionException.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/SubscriptionException.java @@ -76,6 +76,15 @@ public class SubscriptionException extends Exception { } } + /** + * Attempted to retrieve a receipt for a subscription that hasn't yet been charged or the invoice is in the open state + */ + public static class ReceiptRequestedForOpenPayment extends SubscriptionException { + public ReceiptRequestedForOpenPayment() { + super(null, null); + } + } + public static class ProcessorConflict extends SubscriptionException { public ProcessorConflict(final String message) { super(null, message); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/subscriptions/BraintreeManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/subscriptions/BraintreeManager.java index 8d6ea2857..a6f8e6458 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/subscriptions/BraintreeManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/subscriptions/BraintreeManager.java @@ -49,6 +49,8 @@ import org.whispersystems.textsecuregcm.currency.CurrencyConversionManager; import org.whispersystems.textsecuregcm.http.FaultTolerantHttpClient; import org.whispersystems.textsecuregcm.metrics.MetricsUtil; import org.whispersystems.textsecuregcm.storage.PaymentTime; +import org.whispersystems.textsecuregcm.storage.SubscriptionException; +import org.whispersystems.textsecuregcm.util.ExceptionUtils; import org.whispersystems.textsecuregcm.util.GoogleApiUtil; import org.whispersystems.textsecuregcm.util.SystemMapper; import org.whispersystems.textsecuregcm.util.ua.ClientPlatform; @@ -611,7 +613,7 @@ public class BraintreeManager implements CustomerAwareSubscriptionPaymentProcess if (!getPaymentStatus(transaction.getStatus()).equals(PaymentStatus.SUCCEEDED)) { final SubscriptionStatus subscriptionStatus = getSubscriptionStatus(subscription.getStatus(), true); if (subscriptionStatus.equals(SubscriptionStatus.ACTIVE) || subscriptionStatus.equals(SubscriptionStatus.PAST_DUE)) { - throw new WebApplicationException(Response.Status.NO_CONTENT); + throw ExceptionUtils.wrap(new SubscriptionException.ReceiptRequestedForOpenPayment()); } throw new WebApplicationException(Response.status(Response.Status.PAYMENT_REQUIRED) @@ -632,7 +634,7 @@ public class BraintreeManager implements CustomerAwareSubscriptionPaymentProcess return new ReceiptItem(transaction.getId(), PaymentTime.periodStart(paidAt), metadata.level()); }) - .orElseThrow(() -> new WebApplicationException(Response.Status.NO_CONTENT))); + .orElseThrow(() -> ExceptionUtils.wrap(new SubscriptionException.ReceiptRequestedForOpenPayment()))); } private static Subscription getSubscription(Object subscriptionObj) { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/subscriptions/StripeManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/subscriptions/StripeManager.java index 2414a8b51..6623f6d07 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/subscriptions/StripeManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/subscriptions/StripeManager.java @@ -74,7 +74,9 @@ import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.whispersystems.textsecuregcm.storage.PaymentTime; +import org.whispersystems.textsecuregcm.storage.SubscriptionException; import org.whispersystems.textsecuregcm.util.Conversions; +import org.whispersystems.textsecuregcm.util.ExceptionUtils; import org.whispersystems.textsecuregcm.util.ua.ClientPlatform; public class StripeManager implements CustomerAwareSubscriptionPaymentProcessor { @@ -607,10 +609,12 @@ public class StripeManager implements CustomerAwareSubscriptionPaymentProcessor private CompletableFuture convertInvoiceToReceipt(Invoice latestSubscriptionInvoice, String subscriptionId) { if (latestSubscriptionInvoice == null) { - throw new WebApplicationException(Status.NO_CONTENT); + return CompletableFuture.failedFuture( + ExceptionUtils.wrap(new SubscriptionException.ReceiptRequestedForOpenPayment())); } if (StringUtils.equalsIgnoreCase("open", latestSubscriptionInvoice.getStatus())) { - throw new WebApplicationException(Status.NO_CONTENT); + return CompletableFuture.failedFuture( + ExceptionUtils.wrap(new SubscriptionException.ReceiptRequestedForOpenPayment())); } if (!StringUtils.equalsIgnoreCase("paid", latestSubscriptionInvoice.getStatus())) { final Response.ResponseBuilder responseBuilder = Response.status(Status.PAYMENT_REQUIRED);