Handle bad input more consistently in metrics code bases
We started thinking about handling bad input while sanitizing bridge descriptors in CollecTor (legacy/trac#19834 (moved)) and while reading relay descriptors in Onionoo (legacy/trac#20412 (moved)). But before we implement any changes we should generalize our strategies for handling bad input to avoid solving the same problem over and over. The result of this discussion can also serve as guide for future code.
What we're not covering here (but what we should think about anyway) is how we're handling issues during processing that are not directly related to bad input, like problems with sanitizing bridge IP addresses.
So, we can distinguish a couple of use cases where we're handling descriptors as input:
- CollecTor downloads descriptors from the directory or TorDNSEL or Torperf or processes previously downloaded descriptors.
- CollecTor synchronizes descriptors from other CollecTor instances.
- CollecTor reads previously uploaded bridge descriptors and produces sanitized versions of them.
- Metrics, Onionoo, ExoneraTor, and other applications download descriptors from CollecTor and use them locally.
The requirements on input data are quite different for these four use cases. Let's go through them:
- As of a few weeks ago we're storing and serving descriptors even if metrics-lib cannot parse them. The idea is that CollecTor shouldn't decide what goes into the archive, but the directory authorities (or TorDNSEL and Torperf) should. As long as we can detect the descriptor type, extract the publication date, and possibly calculate the digest, we can accept a descriptor.
- The requirements are pretty much the same as for 1.
- We need to be very picky about bridge descriptors, in particular about unknown parts, because those might contain sensitive information that we'd rather not copy over to sanitized bridge descriptors.
- Most applications would want to skip bad descriptors and not bother much about it. As of a few days ago, they all use metrics-lib for parsing descriptors.
So much about the differences. Let's also list the commonalities or possible goals for common behavior:
- Regardless of the chosen strategy, we should apply it to all variants of descriptor badness. This may sound obvious, but we're currently not doing this. For example, if we encounter an invalid "a" line in a bridge descriptor, we skip that line, whereas an invalid "r" line makes us skip the entire descriptor. This is part of what we're trying to streamline in legacy/trac#19834 (moved), even though this case is not explicitly listed there.
- Whenever we encounter an error in processing a descriptor, we should attempt to recover and continue with subsequent good descriptors. The reason is that descriptors in a common source can come from different original sources, and we cannot blame valid descriptors for following a faulty descriptor. This is the cause for legacy/trac#20412 (moved).
This is just the start. Let's add more thoughts to this ticket and assemble a guide that we can apply to existing tickets and future code changes.