Add more phone numbers to "Shields Up" action
ClosedPublic

Authored by epriestley on Tue, Feb 7, 8:27 PM.

Details

Summary

This diff adds phone numbers that were giving us trouble on https://developer.blender.org/

The specific lists of task that included these numbers can be found at https://developer.blender.org/search/query/RVlO.cUv.asA/#R

Test Plan

Create task with these phone numbers

Also, added test coverage.

Diff Detail

Repository
R25 Secure
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Blendify created this revision.Tue, Feb 7, 8:27 PM
epriestley requested changes to this revision.Tue, Feb 7, 8:56 PM

Let's really AMP THIS UP to the NEXT LEVEL πŸš€

src/abuse/SecureSheldsUpAction.php
112

Are these "O" ("oh"), not "0" ("zero")? I think the logic below will never match "O" ("oh").

Maybe do this:

  • Put "0" ("zero") in the patterns.
  • Make the logic below replace all "O" ("oh") with "0" ("zero") before performing the match (if ambitious: also replace "i", "I", "l", "L", "|" with "1").
  • While you're here, fix the spelling of "betwee" a few lines below this.
This revision now requires changes to proceed.Tue, Feb 7, 8:56 PM
Blendify updated this revision to Diff 41662.Tue, Feb 7, 9:40 PM

I changed some things you requested but I do not have enough knowledge of php to do the other part. Maybe use str_replace? Anyway... feel free to commandeer this and make the change

Out of curiosity, why are there "Context not available." indicators here? Am I not supposed to see R25 Secure?

chad added a subscriber: chad.Wed, Feb 8, 12:04 AM

My guess is it was a cut & paste diff

Aha that makes sense. Thanks.

Yes, I just copied a pasted the diff. I was having an issue getting arc working for https://secure.phabricator.com/

epriestley commandeered this revision.Wed, Feb 8, 5:18 PM
epriestley edited reviewers, added: Blendify; removed: epriestley.

I'll make the other adjustments -- str_replace() is the right approach, but this vital production infrastructure could probably use some unit tests anyway.

epriestley updated this revision to Diff 41711.Mon, Feb 13, 3:55 PM
epriestley retitled this revision from Secure: add more phone numbers to Add more phone numbers to "Shields Up" action.
epriestley edited the test plan for this revision. (Show Details)
epriestley edited reviewers, added: chad; removed: Blessed Reviewers, Blendify.
epriestley edited projects, added Abuse; removed Security.
  • Add unit tests.
  • Match "o" and "O" for 0, etc.
chad accepted this revision.Mon, Feb 13, 4:12 PM
This revision is now accepted and ready to land.Mon, Feb 13, 4:12 PM
This revision was automatically updated to reflect the committed changes.