Opened 7 months ago

Closed 6 months ago

#25290 closed defect (fixed)

refactor to use should_record_bridge_info() more

Reported by: arma Owned by:
Priority: Medium Milestone: Tor: 0.3.4.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: easy, tor-bridge, refactoring, 034-triage-20180328
Cc: neel@… Actual Points:
Parent ID: Points:
Reviewer: dgoulet Sponsor:

Description

We have this function
should_record_bridge_info
which simply checks
options->BridgeRelay && options->BridgeRecordUsageByCountry.

But in geoip_note_client_seen(), where we just added the DoS mitigation stuff, we don't use the function, instead choosing to check the variables directly. We make the same choice in the options_need_geoip_info() function.

We should refactor things so we use the function in all cases.

Child Tickets

Attachments (1)

b25290-001.patch (2.0 KB) - added by neel 6 months ago.
Switch to should_record_bridge_info() in geoip_note_client_seen() and options_need_geoip_info()

Download all attachments as: .zip

Change History (7)

Changed 6 months ago by neel

Attachment: b25290-001.patch added

Switch to should_record_bridge_info() in geoip_note_client_seen() and options_need_geoip_info()

comment:1 Changed 6 months ago by neel

I have a patch under the filename b25290-001.patch.

comment:2 Changed 6 months ago by neel

Cc: neel@… added

comment:3 Changed 6 months ago by dgoulet

Keywords: tor-bridge refactoring added
Reviewer: dgoulet
Status: newmerge_ready

Patch looks good! It will need a changes file but that is fine, we can add it!

Big thanks neel!

comment:4 Changed 6 months ago by nickm

Keywords: 034-triage-20180328 added

comment:5 Changed 6 months ago by dgoulet

Never mind the above, there is a changes file :P.

I've taken it in branch bug25290_034_01 and currently running our CI:

https://github.com/torproject/tor/pull/40

Will merge once passed. Thanks!

comment:6 Changed 6 months ago by dgoulet

Resolution: fixed
Status: merge_readyclosed
Note: See TracTickets for help on using tickets.