[tor-bugs] #21934 [Metrics/metrics-lib]: Allow extra arguments in lines containing comma-separated key-value lists

[Tor Bug Tracker & Wiki]

#21934: Allow extra arguments in lines containing comma-separated key-value lists
-------------------------------------+--------------------------
     Reporter:  karsten              |      Owner:  metrics-team
         Type:  defect               |     Status:  new
     Priority:  Medium               |  Milestone:
    Component:  Metrics/metrics-lib  |    Version:
     Severity:  Normal               |   Keywords:
Actual Points:                       |  Parent ID:
       Points:                       |   Reviewer:
      Sponsor:                       |
-------------------------------------+--------------------------
 In the context of simulating changes to directory-request statistics I
 added a few extra arguments to `dirreq-v3-resp` lines to include
 parameters like `bin_size=8`.  But metrics-lib refuses to parse those
 descriptors, throwing a `DescriptorParseException`.  I think this violates
 the [https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n255
 specification] which says:

 {{{
    For forward compatibility, each item MUST allow extra arguments at the
    end of the line unless otherwise noted.  So if an item's description
 below
    is given as:
        "thing" int int int NL
    then implementations SHOULD accept this string as well:
        "thing 5 9 11 13 16 12" NL
    but not this string:
        "thing 5" NL
    and not this string:
        "thing 5 10 thing" NL
    .

    Whenever an item DOES NOT allow extra arguments, we will tag it with
    "no extra arguments".
 }}}

 I put in a quick fix which might even become the real fix:

 {{{
 diff --git a/src/main/java/org/torproject/descriptor/impl/ParseHelper.java
 b/src/main/java/org/torproject/descriptor/impl/ParseHelper.java
 index f73a591..2de05b5 100644
 --- a/src/main/java/org/torproject/descriptor/impl/ParseHelper.java
 +++ b/src/main/java/org/torproject/descriptor/impl/ParseHelper.java
 @@ -341,10 +341,6 @@ public class ParseHelper {
      if (partsNoOpt.length < index) {
        throw new DescriptorParseException("Line '" + line + "' does not "
            + "contain a key-value list at index " + index + ".");
 -    } else if (partsNoOpt.length > index + 1 ) {
 -      throw new DescriptorParseException("Line '" + line + "' contains "
 -          + "unrecognized values beyond the expected key-value list at "
 -          + "index " + index + ".");
      } else if (partsNoOpt.length > index) {
        if (!commaSeparatedKeyValueListPatterns.containsKey(keyLength)) {
          String keyPattern = "[0-9a-zA-Z?<>\\-_]"
 }}}

 I can confirm that this solves my issue.  But does it produce any new
 issues by accepting lines that it shouldn't accept?  And are there similar
 issues where we're throwing an exception instead of simply ignoring extra
 arguments?

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