Page MenuHomePhabricator
Differential D17323

Add more phone numbers to "Shields Up" action
ClosedPublic
Actions

Authored by epriestley on Feb 7 2017, 8:27 PM.
Tags
Referenced Files
Unknown Object (File)
Tue, Dec 17, 5:34 AM
Unknown Object (File)
Tue, Dec 17, 12:39 AM
Unknown Object (File)
Sat, Dec 14, 3:26 PM
Unknown Object (File)
Sun, Dec 8, 10:21 AM
Unknown Object (File)
Sat, Dec 7, 6:25 PM
Unknown Object (File)
Thu, Dec 5, 10:14 AM
Unknown Object (File)
Fri, Nov 29, 10:11 PM
Unknown Object (File)
Wed, Nov 27, 4:59 PM
View All Files
Subscribers
chad
cspeckmim
Korvin

Details

Reviewers
chad
Commits
R25:515f367b0e36: Add more phone numbers to "Shields Up" action
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
Lint Skipped
Unit
Tests Skipped

Event Timeline

Blendify created this revision.Feb 7 2017, 8:27 PM
Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptFeb 7 2017, 8:27 PM
Herald added a subscriber: Korvin. · View Herald Transcript
epriestley requested changes to this revision.Feb 7 2017, 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.Feb 7 2017, 8:56 PM
Blendify updated this revision to Diff 41662.Feb 7 2017, 9:40 PM
Blendify edited edge metadata.

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

cspeckmim added a subscriber: cspeckmim.Feb 8 2017, 12:01 AM

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

Screen Shot 2017-02-07 at 5.54.21 PM.png (1×2 px, 337 KB)

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

My guess is it was a cut & paste diff

cspeckmim added a comment.Feb 8 2017, 12:05 AM

Aha that makes sense. Thanks.

Blendify added a comment.Feb 8 2017, 2:19 AM

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.Feb 8 2017, 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.Feb 13 2017, 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.
Harbormaster completed remote builds in B15610: Diff 41711.Feb 13 2017, 3:55 PM
chad accepted this revision.Feb 13 2017, 4:12 PM
This revision is now accepted and ready to land.Feb 13 2017, 4:12 PM
Closed by commit R25:515f367b0e36: Add more phone numbers to "Shields Up" action (authored by epriestley, committed by epriestley). · Explain WhyFeb 13 2017, 4:46 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
src/
abuse/
5 lines

Diff 41662

View Options

src/abuse/SecureSheldsUpAction.php

Loading...
Phabricator · Built by Phacility