[metrics-bugs] #30373 [Core Tor/Tor]: Most headers non-compliant with spec

[Tor Bug Tracker & Wiki]

#30373: Most headers non-compliant with spec
-------------------------------------------+-------------------------------
 Reporter:  irl                            |          Owner:  (none)
     Type:  defect                         |         Status:  needs_review
 Priority:  Medium                         |      Milestone:  Tor:
                                           |  unspecified
Component:  Core Tor/Tor                   |        Version:  Tor:
                                           |  unspecified
 Severity:  Normal                         |     Resolution:
 Keywords:  bandwidth-file-spec, tor-spec  |  Actual Points:
Parent ID:                                 |         Points:  1
 Reviewer:                                 |        Sponsor:
-------------------------------------------+-------------------------------
Changes (by juga):

 * status:  new => needs_review


Comment:

 Replying to [comment:5 irl]:
 > Replying to [comment:4 juga]:
 > > Replying to [comment:3 irl]:
 > > > Replying to [comment:1 juga]:
 > > > > Does it sounds fine that we just re-define `Keyword` and
 `KeywordChar` in the bandwidth file to include `_`?.
 > > > >
 > > > > I think we would need to increment the version though.
 > > >
 > > > I don't think the version needs changing, this is a "typo fix" in my
 mind. Anyone that has an implementation that works with the current sbws
 files is already complying with the change that we haven't made yet.
 > >
 > > A complying version is 1.0.0. A parser that is ignoring anything (as
 it should) that contains `_` will still parse the timestamp, which is the
 only required thing in 1.0.0.
 >
 > The finite state automaton that I derived from the spec is shown here:

 Did you used a tool to automatically generate that?.
 You reminded me to do something i thought time ago: define the grammar
 using a tool that can check it [0]. If we come out with a nice tool, it
 would be very useful for ensuring our grammars are correct and not
 repeating code in several places.

 > The spec requires that Keywords have only KeywordChar in them, so
 basically any file with headers *fails* to parse. Where Keywords are
 expected, they are not found. So no, the files with headers are not
 compliant with 1.0.0.

 ok, i see.
 >
 > I think we fix this by fixing the spec though. Clearly everyone managed
 just fine until now with having _ in the headers, and people wrote parsers
 that handle them. The parsers I'm adding to bushel are just super strict,
 and designed to catch issues like this.
 >
 > > > Keyword and KeywordChar are already defined, so I wouldn't want to
 cause confusion be re-defining them, even though it's a different spec
 people have ideas in their heads about what these mean.
 > > >
 > > > Maybe just a new `Key`:
 > > >
 > > > {{{Key ::= (KeywordChar | "_")+}}}
 > >
 > > I agree and that sounds good, for some reason i thought we were
 redefining something else, but checked and i don't think so.
 >
 > Can you come up with a patch for the spec that replaces Keyword with
 Key? We're essentially just pretending this is the way it was all along,
 because no one implemented the spec as it's defined.

 https://github.com/torproject/torspec/pull/82

 [0]
 https://github.com/juga0/bandwidth_file_grammar/blob/master/bw_file_grammar.py#L4

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