[tor-bugs] #7325 [pyobfsproxy]: Scrub IPs before displaying them in logs
Tor Bug Tracker & Wiki
blackhole at torproject.org
Mon Nov 12 05:24:36 UTC 2012
#7325: Scrub IPs before displaying them in logs
-------------------------+--------------------------------------------------
Reporter: sysrqb | Owner: asn
Type: defect | Status: needs_review
Priority: normal | Milestone:
Component: pyobfsproxy | Version:
Keywords: | Parent:
Points: | Actualpoints:
-------------------------+--------------------------------------------------
Comment(by gsathya):
Replying to [comment:3 sysrqb]:
Great! Thanks for making the changes :)
> Replying to [comment:2 gsathya]:
> >
> > 2)
> > {{{
> > + log.obfslogger = log.ObfsLogger(args)
> > +
> > }}}
> > Generally good practice to avoid instantiating objects in other
modules.
>
> Makes sense, but see next.
>
> > {{{
> > +""" 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?
>
> Yes, but I was making the assumption that the logger was being
instantiated in obfsproxy.py so, assuming normal operation, it should
never be the case that obfslogger was not set.
I completely agree that in normal operation, it should never be the case
that obfslogger was not set. But I'm just wondering if it'd be better to
make it modular if other modules wanted to access it in the future.
> >
> > 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.
> >
>
> I agree that this makes much more sense than what I had. I'm curious
about why the obfslogger variable is all caps, is it so it's assumed to be
a constant?
Yep. From the PEP8 style guide -
http://www.python.org/dev/peps/pep-0008/#constants
The rest look good, thanks! I'm leaving the status as 'needs_review' for
asn's comments.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/7325#comment:4>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
More information about the tor-bugs
mailing list