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

VIP Request Block: Add function to allow for blocking by partial User Agent string. #4001

Merged

Conversation

mslinnea
Copy link
Contributor

Description

I would like to block requests by a partial match on the user agent string. The use case is a User Agent header like 'WordPress/6.1.1; https://www.example.com' which is the default format of a User Agent string to WordPress. If I block by exact match, as soon as WordPress is upgraded, this exact match will no longer work.

Changelog Description

Adds VIP_Request_Block::ua_partial_match

Checklist

Please make sure the items below have been covered before requesting a review:

  • This change works and has been tested locally (or has an appropriate fallback).
  • This change works and has been tested on a Go sandbox.
  • This change has relevant unit tests (if applicable).
  • This change uses a rollout method to ease with deployment (if applicable - especially for large scale actions that require writes).
  • This change has relevant documentation additions / updates (if applicable).
  • I've created a changelog description that aligns with the provided examples.

Steps to Test

Outline the steps to test and verify the PR here.

Example:

  1. Check out PR.
  2. Follow documentation in https://docs.wpvip.com/how-tos/block-requests/ to test, except use VIP_Request_Block::ua_partial_match with a substring instead of VIP_Request_Block::ua

@mslinnea mslinnea requested a review from a team as a code owner December 30, 2022 19:07
lib/class-vip-request-block.php Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jan 3, 2023

Codecov Report

Merging #4001 (f05ae23) into develop (b6fd950) will increase coverage by 0.01%.
The diff coverage is 62.50%.

@@              Coverage Diff              @@
##             develop    #4001      +/-   ##
=============================================
+ Coverage      32.27%   32.29%   +0.01%     
- Complexity      3768     3771       +3     
=============================================
  Files            226      226              
  Lines          16803    16807       +4     
=============================================
+ Hits            5424     5428       +4     
  Misses         11379    11379              
Impacted Files Coverage Δ
lib/class-vip-request-block.php 58.13% <62.50%> (+4.29%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@rebeccahum
Copy link
Contributor

@sjinks I committed the changes on behalf.

@rebeccahum rebeccahum force-pushed the vip-request-block-by-partial-ua-match branch from 177a1d9 to 5dbc3ff Compare January 5, 2023 22:24
@rebeccahum rebeccahum enabled auto-merge (squash) January 5, 2023 22:24
@sjinks
Copy link
Member

sjinks commented Jan 5, 2023

@rebeccahum thank you, I will take a closer look tomorrow - I see SonarCloud complains etc.

@sjinks sjinks force-pushed the vip-request-block-by-partial-ua-match branch from 5dbc3ff to 4efcce1 Compare January 5, 2023 22:44
@sjinks sjinks disabled auto-merge January 5, 2023 22:44
@sjinks sjinks self-assigned this Jan 5, 2023
@GaryJones
Copy link
Contributor

It's suggesting that this line should be static::block_and_log() and not self::block_and_log() - i.e. make use of late static binding (PHP 5.3) to reference any child class that may have extended this class which may overwrite block_and_log().

Same fix will be needed in at least one other method too.

@rebeccahum rebeccahum force-pushed the vip-request-block-by-partial-ua-match branch from 4efcce1 to cc4e537 Compare January 6, 2023 17:14
@sonarcloud
Copy link

sonarcloud bot commented Jan 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants