[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-bugs] Re: #1270 [Tor - Tor client]: Spec conformance issues reported by outofwords



#1270: Spec conformance issues reported by outofwords
-------------------------------+--------------------------------------------
  Reporter:  Sebastian         |       Owner:  nickm             
      Type:  defect            |      Status:  assigned          
  Priority:  minor             |   Milestone:  Tor: 0.2.2.x-final
 Component:  Tor - Tor client  |     Version:  0.2.1.22          
Resolution:  None              |    Keywords:                    
    Parent:                    |  
-------------------------------+--------------------------------------------

Comment(by Sebastian):

 Comments on get_next_token_v2:

 Some places check for get_next_token's return value, some don't. We should
 add the check to all places.


 5 and 6 are magic numbers (usually either strlen("-----") or
 strlen("-----\n") that are used frequently. Maybe adding some #defines
 would be nicer?
 +  /* Skip to the first character after the newline... */
 +  s += 6;


 strcmpstart_len() belongs in util.c, not routerparse.c


 The dir-spec change says "It is also an error for whitespace to appear
 after a keyword that does not accept arguments." yet the comment for
 args_len in token_boundaries_t explains how the structure looks for this
 case. We also don't reject such documents later, so this is a spec
 violation.


 We also don't do any verification that the arguments for keywords are
 really ascii only.


 In lex_token(), we never accept @ as part of a keyword. This breaks when
 we use it for annotations.


 What is b for in this code?
 +    char b[64];
 +    out->end_of_token = s;
 +    strlcpy(b, s, MIN(eos-s,64));
 +    return 1;


 Add a comment to explain what's going on in
 +  if (nl < s+6 || 0 != memcmp("-----\n", nl-5, 6)) {
 took me a bit to see what it is trying to do


 We should check we're not reading to far here
 +  /* Point s (and the object body) to the first char on the next line.*/
 +  s = nl+1;
 and also here
 +  /* Skip to the first character after the newline... */
 +  s += 6;

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/1270#comment:15>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online