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

Fix request interception when using a black/whitelist #140

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jimryan
Copy link

@jimryan jimryan commented Dec 9, 2020

This PR aims to resolve two issues:

  1. Intercepted requests would always be continued when a black or whitelist was used, regardless of if the callback that applies the white/blacklist was the last callback. This prohibited callbacks registered after cuprite's from aborting the request or responding to it, since it was already continued.
  2. Whitelists and blacklists could not be combined. If a blacklist was configured, the whitelist was never considered, and the request was either aborted or continued based on if it matched the blacklist or not.

The callback that applies the black/whitelist now:

  1. Aborts the request and stops if the blacklist is configured and the request matches it.
  2. Aborts the request and stops if the whitelist is configured and the request does not match it.
  3. Continues the request if there are no other callbacks.
  4. Does nothing if the request does not match the blacklist, matches the whitelist, and there are other callbacks.

I also question if continuing the request should be the callback's responsibility or not, or if Ferrum should just continue it if no callbacks responded or aborted. As things stand, all callbacks need to do something like this.


Thank you so much for cuprite/ferrum, my test suites run faster, are easier to debug, and I actually understand what's going on and can read the code powering the entire stack. Awesome work ❤️

…d the black/whitelist callback is the last callback

Previously, if the request didn't match the blacklist,
or matched the whitelist, it'd be continued, without regard
for if there were other callbacks. This prohibited those
callbacks from aborting the request or responding to it.

This also permits blacklists and whitelists to be combined.
Before, if both a white and blacklist were configured,
the whitelist would be ignored. Now, if both are configured,
only requests that do not match the blacklist and match
the whitelist are permitted.
@route
Copy link
Member

route commented Jan 11, 2021

Sorry for a late reply, tough time. I'll start checking all the PRs and Issues and prepare a release.

@jimryan
Copy link
Author

jimryan commented Jan 11, 2021

@route Totally understandable! Thanks. Let me know if I can help in any way.

@Mifrill

This comment has been minimized.

Copy link
Collaborator

@Mifrill Mifrill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Mifrill Mifrill requested a review from route August 29, 2021 17:06
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.

None yet

3 participants