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

[Bug fix] Sanitize csv mails #453

Merged
merged 10 commits into from
May 16, 2024
Merged

[Bug fix] Sanitize csv mails #453

merged 10 commits into from
May 16, 2024

Conversation

Mast3rwaf1z
Copy link
Member

in progress fix to unsanitized values in csv files, there are two issues atm

  • utf-8 values like æøå
  • commas

@Mast3rwaf1z Mast3rwaf1z linked an issue May 15, 2024 that may be closed by this pull request
@Mast3rwaf1z Mast3rwaf1z requested a review from krestenlaust May 15, 2024 20:37
Copy link
Member

@krestenlaust krestenlaust left a comment

Choose a reason for hiding this comment

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

What about products with " in the name

@Mast3rwaf1z
Copy link
Member Author

What about products with " in the name

Hmm good point, i'll just have a google to see how others solve that as well, no way we have to consider everything :P

I have tested it using mailhog like in #434, mailhog --smtp-bind-addr 0.0.0.0:25 and this python script:

import csv
with open("Downloads/sales.csv") as file:
    reader = csv.reader(file)
    for row in reader:
        print(row)

the two issues i've mentioned is gone now at least

@Mast3rwaf1z
Copy link
Member Author

Mast3rwaf1z commented May 15, 2024

What about products with " in the name

@krestenlaust this should be covered now

@Mast3rwaf1z Mast3rwaf1z requested a review from krestenlaust May 15, 2024 21:14
@Mast3rwaf1z
Copy link
Member Author

I have an alternative idea that i just tested, instead of having the rows_to_csv function we have now, we can go with your idea of using the csv library and mock a file class:

import csv
class fakefile:
    data = ""
    def write(self, data):
        self.data += data

def rows_to_csv(rows):
    file = fakefile()
    writer = csv.writer(file)
    writer.writerows(rows)
    return file.data

@Mast3rwaf1z Mast3rwaf1z marked this pull request as ready for review May 15, 2024 21:40
Copy link
Member

@krestenlaust krestenlaust left a comment

Choose a reason for hiding this comment

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

Looks promising, I have tested it yet

stregsystem/mail.py Outdated Show resolved Hide resolved
stregsystem/fixtures/testdata-csv.json Outdated Show resolved Hide resolved
Copy link
Member

@krestenlaust krestenlaust left a comment

Choose a reason for hiding this comment

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

Nice work!

I'm satisfied with this for now, but I've created these two issues #455 and #456, I would be greatful if you could take a gander at one of them at some point :)

@krestenlaust krestenlaust merged commit 981ea82 into next May 16, 2024
4 checks passed
@krestenlaust krestenlaust deleted the sanitize-csv branch May 16, 2024 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Unsanitized input in the user data request csvs
2 participants