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

Apply request processing only when Fastly is enabled #486

Merged
merged 2 commits into from
Oct 6, 2021
Merged

Apply request processing only when Fastly is enabled #486

merged 2 commits into from
Oct 6, 2021

Conversation

fredden
Copy link
Contributor

@fredden fredden commented Oct 6, 2021

Issue

When Fastly has been previously in use, but is no longer in use, some Fastly settings are still effective.

We have noticed this when working on development and staging copies of a website where Fastly is configured properly on production, but not in use on development nor staging environments; Magento is configured in these environments to use 'built-in' or 'varnish' as its full-page cache application.

Steps to reproduce

  1. Configure Magento to use Fastly as its Full-Page Cache.
  2. Configure Rate Limiting in the Fastly-specific settings.
  3. Navigate to the front-end and exceed the configured rate limiting threshold.
  4. Witness that Fastly rate limiting is effective.
  5. Configure Magento to use another application for Full-Page Cache (such as 'built-in').
  6. Witness that Fastly rate limiting is effective, even though Magento is not configured to use Fastly at all.

@smaeda-ks
Copy link
Contributor

@fredden Thank you for the PR. Looks good to me.

@smaeda-ks smaeda-ks merged commit be0bdc1 into fastly:master Oct 6, 2021
@fredden fredden deleted the no-rate-limiting-when-fastly-disabled branch October 7, 2021 17:44
MickaelDatadome pushed a commit to DataDome/fastly-magento2 that referenced this pull request Oct 5, 2023
* Correct type hint for property

* Skip rate limiting when Fastly is not enabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants