Re: [tor-bugs] #7325 [pyobfsproxy]: Scrub IPs before displaying them in logs

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#7325: Scrub IPs before displaying them in logs
-------------------------+--------------------------------------------------
 Reporter:  sysrqb       |          Owner:  asn           
     Type:  defect       |         Status:  needs_revision
 Priority:  normal       |      Milestone:                
Component:  pyobfsproxy  |        Version:                
 Keywords:               |         Parent:                
   Points:               |   Actualpoints:                
-------------------------+--------------------------------------------------
Changes (by gsathya):

  * status:  needs_review => needs_revision


Comment:

 I was just reading your patch and I had some thoughts. Feel free to
 disagree with anything/everything I've said.
 1)
 {{{

 +    log.obfslogger = log.ObfsLogger(args)

 }}}

 You could just pass args.no_safe_logging instead of passing the entire
 list of args.

 2)
 {{{
 +    log.obfslogger = log.ObfsLogger(args)
 +
 }}}
 Generally good practice to avoid instantiating objects in other modules.
 {{{
 +""" Global variable that will track our Obfslogger instance """
 +obfslogger = None
 +
 }}}
 {{{
 +def get_obfslogger():
 +    """ Return our instance of Obfslogger class """
 +
 +    assert(obfslogger)
 +    return obfslogger
 }}}

 Wouldn't be better to check if there is a obfslogger, and if not create a
 new instance instead of exiting out?

 These can be changed to -
 In bfsproxy.py -
 {{{
 obfslogger = log.get_logger(args.no_safe_logging)
 }}}
 In obfsproxy/common/log.py -
 {{{
 """ Global variable that will track our Obfslogger instance """
 OBFSLOGGER = None
 }}}
 {{{
 def get_obfslogger(no_safe_logging):
 """ Return Obfslogger class """
   global OBFSLOGGER:
   if not OBFSLOGGER: OBFSLOGGER = ObfsLogger(no_safe_logging)
   return OBFSLOGGER
 }}}
 or even just a create_obfslogger(no_safe_logger) which doesn't return
 anything.

 3)
 {{{
 +      if self.we_should_scrub_address():
 +        return '[scrubbed]'
 }}}
 why use an accessor function?

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/7325#comment:2>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs