[tor-bugs] #19170 [Metrics/CollecTor]: make parsing more robust (extra-info)

[Tor Bug Tracker & Wiki]

#19170: make parsing more robust (extra-info)
-------------------------------+--------------------------
 Reporter:  iwakeh             |          Owner:  iwakeh
     Type:  defect             |         Status:  accepted
 Priority:  Medium             |      Milestone:
Component:  Metrics/CollecTor  |        Version:
 Severity:  Normal             |     Resolution:
 Keywords:  ctip               |  Actual Points:
Parent ID:                     |         Points:
 Reviewer:                     |        Sponsor:
-------------------------------+--------------------------

Comment (by iwakeh):

 Replying to [comment:11 karsten]:
 > ...
 > Regarding the `LenientParser` idea, I wonder whether we should just skip
 the metrics-lib check to see whether we can parse a descriptor before
 writing it to disk.  See `ArchiveWriter#store()`.  At that point we
 already parsed all relevant fields that we need for storing the descriptor
 without using metrics-lib, and that check is only there to make sure that
 metrics-lib will be able parse the descriptor later.  But if we want to
 take that check out, which I think we should, then let's just change that
 code to print out an informational log statement and store the file
 anyway.  What do you think?

 I agree, removing the metrics-lib check and informing known clients that
 do not use metrics-lib for parsing seems to be the simplest option here.
 It will decrease the number of missing descriptors immediately and is a
 'minimal-invasive' code change.

 Summary:

 CollecTor should not require parsability - except for the fields necessary
 - of descriptors when downloading/storing and only logs a warning if
 metrics-lib complains. (Eventually CollecTor could verify the signature.
 But, that's a different thing.)
  Without adapting metrics-lib these descriptors would trigger an exception
 during parsing in a metrics-lib dependent client, which handles parser
 exception already in some way (probably drop the descriptor). Clients
 without metrics-lib might have additional problems now and should be
 warned before such a change is deployed.

  Possible future option: When augmenting metrics-lib with a LenientParser
 the decision has to be made for the various clients, if they also want to
 use that parser option. If the client keeps the strict parser this will be
 like case 1.  If the client uses the LenientParser, it might provide
 unreadable characters (didn't we have tickets about that in Onionoo
 somewhere?).

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