Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using 428 Precondition Required status is unreliable on some hosting platforms #2345

Open
mikkamp opened this issue Apr 2, 2024 · 0 comments
Labels
type: enhancement The issue is a request for an enhancement.

Comments

@mikkamp
Copy link
Contributor

mikkamp commented Apr 2, 2024

Describe the bug:

When we create or link a new Ads account we run through several steps which must be completed. Currently these steps are:
'set_id', 'billing', 'link_merchant', 'conversion_action'

The billing step has 2 pre conditions that must be completed:

  1. A new account must be authorized
  2. Billing details must be completed

If these pre-conditions are not completed we return the status code 428 and the contents of the request contain some additional details about what to do next to complete the pre-conditions. However the problem is that status codes aren't always all interpreted in the same way, especially in cases where the traffic is proxied or the server is placed behind a reverse proxy.

When testing we discovered that the status 428 is reserved for other purposes on some hosting platforms like Pressable: https://pressable.com/knowledgebase/rate-limiting-on-404-pages-http-428-error/

This means when we return a response with the status 428 the content of the request is overwritten with a custom page:
image
We get that same page whether we use the status 428 or 429.

This same response is returned on several different hosting platforms (all based on the same stack), I tested with the following:

  • Pressable
  • Jurassic Ninja
  • Atomic
  • WordPress.com

Alternatives

One workaround we could try is to use a different status code or even a custom one. However it seems that's not really recommended, and from my testing it seems we can get unexpected results when the data is proxied.

For example using the status code 418 I am a Teapot works well. But trying a status like 425 Too early or a custom status like 477 doesn't work as expected. The final response does come through but the status code is rewritten to a 200 response.

Based on this I think our safest bet is to just use a regular 200 response and return the required pre-condition not met within the payload of the response. In order to do so we will need to rewrite some of the logic both in the API endpoints and the UI that consumes the endpoint for creating/linking an Ad account.

Steps to reproduce:

  1. Create a test site on for example Jurassic Ninja
  2. Upload and activate a plugin like the following: api-test.zip
  3. Open the browser DevTools
  4. Visit the endpoint https://<name>.jurassic.ninja/wp-json/custom/api/test/status
  5. Confirm that the response code is 428
  6. Confirm that the actual response body has been overwritten with a HTML page with the message "429 Too Many Requests"

Additional details:

This is reproducible with just the current onboarding, however we were able to get around the issue with the fix in PR #2301
However in PR #2270 we are creating another pre-condition which resurfaces the problem.

@jorgemd24 jorgemd24 added the type: enhancement The issue is a request for an enhancement. label Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement The issue is a request for an enhancement.
Projects
None yet
Development

No branches or pull requests

2 participants