[tor-bugs] #11243 [Tor]: Don't fetch any descriptor which we already fetched and found to be ill-formed

[Tor Bug Tracker & Wiki]

#11243: Don't fetch any descriptor which we already fetched and found to be ill-
formed
------------------------+-------------------------------------------------
     Reporter:  nickm   |      Owner:
         Type:  defect  |     Status:  needs_review
     Priority:  major   |  Milestone:  Tor: 0.2.6.x-final
    Component:  Tor     |    Version:
   Resolution:          |   Keywords:  tor-relay 026-triaged-1 nickm-patch
Actual Points:          |  Parent ID:
       Points:          |
------------------------+-------------------------------------------------

Comment (by nickm):

 Replying to [comment:11 andrea]:
 > Begin code review for nickm's bug11243 branch:
 >
 > c062758486dd75b3fb3030324cd76a2a206676bd:
 >
 >  - /* XXXX Change this to digest256_len */ ?

 This is a good idea; I've opened a ticket for it as #13399.

 [...]
 >  - In router_parse_list_from_string(), it'd be possible for a suitably
 formed
 >    string to result in the same hash being added to invalid_digests_out
 >    multiple times.  Are the callers of this code aware of that?
 >
 >  - The same comments as for router_parse_list_from_string() apply to
 >    microdescs_parse_from_string().

 Hm. Interesting.  I should check these out.  I believe the answer is "yes
 that's okay", but I should check. '''(TODO 1)'''

 [...]
 >  - In both router_parse_entry_from_string() and
 >    extrainfo_parse_entry_from_string(), correctness depends on all of
 the
 >    reasons for rejecting an input before setting can_dl_again = 1 be
 functions
 >    only of the input itself as covered by the hash, and with no
 dependencies
 >    on any other state of the router.  Seems unlikely we'd want to ever
 change
 >    the parsing to be any other way, but perhaps a comment warning future
 >    editors of those functions would be advisable.

 ok. '''(TODO 2)'''

 >  - Use of digestmap_t in microdescs_add_to_cache() looks like it will
 handle
 >    repeated hashes, so that worry is assuaged.  These magic 1/2/3
 constants
 >    seem a bit ugly though.  Can we get some #defines or something?

 Will do. '''(TODO 3)'''

 [...]
 > 24b24e4a97117c295b310ccf0f8da327e9d7e8a3:
 >
 >  - Yay, we get new unit tests even for code we already had!

 It seemed important and worthwhile.  Also it was fun.

 [...]
 >  - Change description sounds fine, but isn't it redundant with the one
 >    before?

 Fixed.

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