3
\$\begingroup\$

I had a friend ask me to write this up for her as she discovered her logs were saving private bank information in plain text. This is used to parse a few hundred gigs of text files. I've never worked with python for anything other then pet projects so i'd like to send my friend a nice piece of code since this is my first time helping her. I'm not looking to make this multi-threaded however.

Any suggestions on improvements or lessons to be learned from this? Thank you in advance!

import os

scan1_location=6
scan2_location=3
scan3_location_a=38
scan3_location_b=60
scan1="bankAccount: "
scan2="bankAccount,"
scan3=" accountNumber="
input_directory="/home/ubuntu/CCTech_Dev/Stories/S-68080/logFiles/"
output_directory="/home/ubuntu/CCTech_Dev/Stories/S-68080/newLogs/"
found=0
count=0

# 1. Parse the given line for the number to be masked
# 2. Mask the number
# 3. Insert masked number back into line
def maskLine(line, scan):
    # Depending on the scan, the number is in a different location in the line
    if scan == 1:
        number = line[scan1_location]
        number = maskNumber(number)
        line[scan1_location] = number
    elif scan == 2:
        subLine = line[scan2_location].split(",")
        subLine[3] = maskNumber(subLine[3])
        line[scan2_location] = ",".join(subLine)
    elif scan == 3:
        # The number in this scan can either occur once in a short line
        # or twice in a long line.
        if len(line) < 50:
            subLine = line[len(line) - 4].split("=")
            subLine[1] = maskNumber(subLine[1])
            line[len(line) - 4] = "=".join(subLine)
        else:
            subLine = line[scan3_location_a].split("=")
            subLine[1] = maskNumber(subLine[1])
            line[scan3_location_a] = "=".join(subLine)
            line[scan3_location_b] = "=".join(subLine)
    else:
        raise Exception("Location Error")
    return " ".join(line)

# Example 1: Input -> 123,       Output -> *****
# Example 2: Input -> 123456789, Output -> *****6789
def maskNumber(number):
    if len(number) > 0 and len(number) < 5:
        number = "*****"
    else:
        number = "*" * (len(number) - 4) + number[-4:]
    return number




# Run through each line of every file to mask every bank account number found

# Iterate over every file in directory
for filename in os.listdir(input_directory):
    print(input_directory + filename)

    # Output file may be in the same location as input depending on
    # output_directory but will always have a different file name
    file = open(input_directory + filename, 'r')
    output = open(output_directory + filename + "-new", 'w+')
    # Initial read of file
    line = file.readline()
    length = len(line)

    # Read until the end of the file
    # Ensures any erroneous reads are not parsed
    while length > 0:

        # Check if line contains a number that needs to be masked
        size = line.find(scan1, 0, length)
        location=scan1_location
        scan=1
        if size == -1:
            size = line.find(scan2, 0, length)
            scan=2
        if size == -1:
            size = line.find(scan3, 0, length)
            scan=3

        if size > -1:
            # Count the number of numbers masked (because I like numbers)
            found+=1
            line = line.split(" ")
            line = maskLine(line, scan)
            # Newline is removed during the parsing process
            output.write(line + "\n")
        else:
            output.write(line)

        # Count total number of lines (because I like numbers)
        count+=1

        # Read next line
        line = file.readline()
        length = len(line)
    file.close()
print(count)
print(found)
output.close()

Update: Per RootTwo's request, here are a few sample strings that I would be editing. And to answer their question, out of 2,000,000+ lines in my test run, I only captured 17,000 lines with protected information. So they are very interspersed. But all numbers I need to mask all come after three cases:

  1. "bankAccount: "

  2. ",bankAccount,"

  3. " accountNumber="

scanX_location were supposed to be the static array locations in "line" that the numbers were located but I recently discovered the size of "line" is dynamic in a few cases. So I need to rework the way I find where the numbers are in each line. Here are a few sample cases.

(The fifth one is an example of a line that doesn't require masking.)

  1. 11.11.11.11.1111111111111.111111.IVR,09/16/2019 00:46:24.190,Make Checking Payment,custom,FYI,Inputs - bankAccount: 111111111111 routingNumber: 111111111

  2. 11.11.11.11.1111111111111.111111.IVR,09/16/2019 00:49:54.992,Make Checking Payment,custom,bankAccount,******3060

  3. 11.11.11.11.111111111111.111111.IVR,09/16/2019 00:49:54.992,Make Checking Payment,custom,ivrUser,IvrUser{environment=prod accountId=111111111 primaryPerson=1111111111 personIdList=[1111111111] houseNumber=11111 streetName=N Main St N Unit 11 zipCode=11111 phoneNumber=1111111111 phoneNumberFormatted=(111) 111-1111 customerClass=Normal numberOfAccounts=11 accountBalance=11.11 accountDueDate=2019-09-10 isEligible=true isEligibleReason=null amountDue=AmountDue{currentAmount=11.11 latestPayment=11 latestpaymentDate=2019-08-15} cashOnlyEligible=true cashOnlyScore=0 checkPaymentAmountDue=11.12 accountSearchCount=0 savedBankInfo=[BankInfo{routingNumber=111111111 accountNumber=1111111111 encrAccountNumber= eligibleForOneTimePayment=false personPaymentOptionId=null} BankInfo{routingNumber=111111111 accountNumber=1111111111 encrAccountNumber= eligibleForOneTimePayment=false personPaymentOptionId=null}] payPlan=PayPlan{hasPayPlan=false payPlanId=null nextAmount=null nextDate=null} hasPendingPayment=false pendingDisconnect=false payOption=null payPlanConfirmation=null debugSteps=[Starting: MakePayment: getSavedBankInfo personCount: 1 call MakePayment: listBankInfo.add: BankInfo{routingNumber=111111111 accountNumber=1111111111 encrAccountNumber= eligibleForOneTimePayment=false personPaymentOptionId=null}; listBankInfo.add: BankInfo{routingNumber=111111111 accountNumber=1111111111 encrAccountNumber= eligibleForOneTimePayment=false personPaymentOptionId=null}; setHasPendingPayment: false; setSavedBankInfo and return;] Report=null phonePersonDetails=[1111111111]}

  4. 11.11.11.11.1111111111111.111111.IVR,09/16/2019 18:37:35.857,Make Checking Payment,custom,FYI,Starting Delete of Saved Payment Info for: Person{personId=1111111111 personOrBusiness=P version=11 paymentOptionId=null bankInfoList=[BankInfo{routingNumber=111111111 accountNumber=111111111111 encrAccountNumber=null eligibleForOneTimePayment=false personPaymentOptionId=111111111111} BankInfo{routingNumber=111111111 accountNumber=1111111111 encrAccountNumber=null eligibleForOneTimePayment=false personPaymentOptionId=111111111111} BankInfo{routingNumber=111111111 accountNumber=1111111111 encrAccountNumber=null eligibleForOneTimePayment=false personPaymentOptionId=111111111111}]}

  5. 11.11.11.11.1111111111111.111111.IVR,09/16/2019 18:37:36.904,Make Checking Payment,custom,FYI,Starting Delete of Saved Payment Info for: Person{personId=1111111111 personOrBusiness=P version=3 paymentOptionId=null bankInfoList=[]}

\$\endgroup\$
1
  • \$\begingroup\$ Are the different kinds of lines intermixed within a file, or does a file only have one kind of line in it? Can you provide example lines (with made up data obviously)? \$\endgroup\$
    – RootTwo
    Sep 22, 2019 at 6:35

1 Answer 1

3
\$\begingroup\$

Document everything

You will now be on the hook to maintain this code (which is fine). So, document everything very well. What does scan1_location mean and why is it equal to 6, but scan2_location = 3. Will you remember why a year from now when your friend asks you to update the code to handle a new kind of line, or when one of the line format changes?

Include a top level docstring explaining what the file does any why. Include a sample of every kind of line.

proper command line program

It looks like this is to be a command line program. So make it a proper command line program. Take a look at the 'click' library. At a minimum, it should have a way to get help with proper usage of the program. I would add a way to turn on logging, to help with tracking down the inevitable bug.

separate output directory

Presumably, the account numbers are being masked in the files, so the files can be seen or used by people that aren't allowed to see the account numbers. Intermixing the masked and unmasked files in the same directory is asking for someone to accidentally send a wrong file.

open() can be used as a context manager

It looks like output.close() if misplaced. A new output file is opened whenever a new input filed is opened, and they should probably be closed be together too. The easiest way to ensure files are closed when you are done with them is to use a with statement:

with open(...) as input_file, open(...) as output_file:

    ... process the files in this block of code ...

... the files automatically get closed here ...

open files are iterable

To iterate over the lines in a file use:

for line in input_file:
    ... process the lines ...

str.find(substring, start, end)

start and end default to 0 and the length of the string, so it is not necessary to provide them.

That's all for now.

\$\endgroup\$
2
  • \$\begingroup\$ I completely reorganized my program to use regex and your recommendation on context management and its now much more readable (though now bottlenecked by CPU instead of I/O). Thank you for the wonderful suggestions. This was my first CodeReview post and it was exactly what I needed. I'll be sure to utilize CodeReview more from now on! \$\endgroup\$
    – Jacob C
    Sep 22, 2019 at 21:05
  • \$\begingroup\$ I was going to suggest a regex approach (using re.sub), but needed to see the sample data to know if it would work. \$\endgroup\$
    – RootTwo
    Sep 22, 2019 at 22:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.