Re: [tor-bugs] #6114 [Stem]: Implement GETCONF parsing in Stem

["Tor Bug Tracker & Wiki" <torproject-admin@xxxxxxxxxxxxxx>]

#6114: Implement GETCONF parsing in Stem
--------------------+-------------------------------------------------------
 Reporter:  neena   |          Owner:  neena       
     Type:  task    |         Status:  needs_review
 Priority:  normal  |      Milestone:              
Component:  Stem    |        Version:              
 Keywords:          |         Parent:              
   Points:          |   Actualpoints:              
--------------------+-------------------------------------------------------
Changes (by neena):

  * status:  needs_revision => needs_review


Comment:

 Replying to [comment:5 atagar]:
 > > The pop_mapping would return ("DataDirectory", "/tmp/fake") and leave
 "dir" in self._remainder for the next pop. Until some other control
 message's response has similar formatting, I'd like to leave this as it
 is.
 >
 > Gotcha. This is actually the second time that I've wanted a 'pop
 everything remaining on the line as a mapping' method (I ran into this
 same issue with GETINFO responses). Maybe we should add a 'remainder' flag
 to pop_mapping() to process everything remaining on the line as the value?

 Implementing the 'remaining' option was a little more work. To do it
 right, I had to modify _parse_entry and with the existing quoted and
 escaped switches, it was a little more effort than I expected. I'm going
 to do it this weekend.

 > > I'm not sure what a quoted value would be. Can't find any, yet.
 >
 > Did the spec somehow imply that there are quoted values? If so then
 maybe you should ask Nick (he'd, of course, be the most familiar with what
 it might be for).
 Yes, it did, here https://gitweb.torproject.org/torspec.git/blob/master
 :/control-spec.txt#l264

 {{{
  264   Value may be a raw value or a quoted string.  Tor will try to use
  265   unquoted values except when the value could be misinterpreted
 through
  266   not being quoted.
 }}}

 >
 > > +    Queries the control socket for the values of given configuration
 options. If
 > > +    provided a default then that's returned as the if the GETCONF
 option is undefined
 >
 > Minor grammatical issue...
 > s/returned as the if the GETCONF/returned if the GETCONF
 >
 Fixed. Not sure why these are happening. :/

 > > +    * :class:`stem.socket.InvalidArguments` the request's arguments
 are invalid
 >
 > Lets state the reply types that this exception might be raised by (since
 they're the minority). Also, please make the GetInfo response do this too.
 Done (in the same branch)

 >
 > > +def _getval(dictionary, key):
 > > +  try:
 > > +    return dictionary[key]
 > > +  except KeyError:
 > > +    pass
 >
 > You want the dictionary's get() method - it's very handy, suppressing
 KeyErrors and returning an optional default value (otherwise returning
 None if the key doesn't exist).
 Ah, yes. Done.

 > > +def _split_line(line):
 > > +  try:
 > > +    if '=' in line:
 > > +      if line[line.find("=") + 1] == "\"":
 > > +        return line.pop_mapping(True)
 > > +      else:
 > > +        return line.split("=", 1)
 > > +    else:
 > > +      return (line, None)
 > > +  except IndexError:
 > > +    return (line[:-1], None)
 >
 > I'm pretty sure that this can be replaced by...
 >
 > {{{
 > if line.is_next_mapping(quoted = False):
 >   return line.split("=", 1).items()[0] # TODO: make this part of the
 ControlLine?
 > elif line.is_next_mapping(quoted = True):
 >   return line.pop_mapping(True).items()[0]
 > else:
 >   return (line.pop(), None)
 > }}}

 Copied shamelessly.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/6114#comment:6>
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