Opened 8 years ago

Closed 3 years ago

#4208 closed defect (wontfix)

Proposed fix for Weather Bug # 2719

Reported by: buzachaka Owned by: kaner
Priority: Medium Milestone:
Component: Metrics/Tor Weather Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: #2719 Points:
Reviewer: Sponsor:

Description

This is a fix for the broken e-mail parser in the ctlutil.py utility of TOR Weather. The original parser could not handle symbol replacement like ".AT." for @ and would allow illegal characters in the returned e-mail address.

This fix adds two levels of address sanitation with intermediate checks for the existence of a valid string. There is also a final check to make sure that the returned address conforms to RFC 2822.

The repository is located at git@…:buzachaka/TOR-Weather.git.

The only differences between this repository and the current master are in the ctlutil.py file.

Child Tickets

Attachments (1)

patch (3.5 KB) - added by kraai 7 years ago.
Patch from github without accidentally included changes to emails.py

Download all attachments as: .zip

Change History (5)

comment:1 Changed 8 years ago by arma

Parent ID: #2719
Status: newneeds_review

comment:2 in reply to:  description Changed 8 years ago by kaner

Replying to buzachaka:

This is a fix for the broken e-mail parser in the ctlutil.py utility of TOR Weather. The original parser could not handle symbol replacement like ".AT." for @ and would allow illegal characters in the returned e-mail address.

This fix adds two levels of address sanitation with intermediate checks for the existence of a valid string. There is also a final check to make sure that the returned address conforms to RFC 2822.

The repository is located at git@…:buzachaka/TOR-Weather.git.

The only differences between this repository and the current master are in the ctlutil.py file.

Note that next time you should add a single commit on top of the latest master. This makes it easier to review.

Is the patch tested?

Changed 7 years ago by kraai

Attachment: patch added

Patch from github without accidentally included changes to emails.py

comment:3 Changed 5 years ago by kaner

Thanks for your patch.

I've tested your regex against the previously used regex with all current contact lines in the consensus. Looks good, the regex works as good as the old one (fishes 3431 addesses out of 3983 contact lines), and fixes the problem with the '.at.' obfuscation.

However, I think maybe for the final validation (lines 570-573 in your patch), we should think about using the validate_email() function from django.core.validators instead of baking our own?

Also, how about fixing #7035 together with this?

comment:4 Changed 3 years ago by karsten

Resolution: wontfix
Status: needs_reviewclosed

Tor Weather has been discontinued as of May 24, 2016: https://lists.torproject.org/pipermail/tor-relays/2016-June/009424.html. Batch-closing all remaining tickets as announced in #19382. A list of these tickets and any other Weather tickets modified after June 26, 2016 will be available here: https://trac.torproject.org/projects/tor/query?changetime=Jun+27%2C+2016..&component=^Metrics%2FTor+Weather

Note: See TracTickets for help on using tickets.