[tor-bugs] #24742 [Core Tor/Fallback Scripts]: Add fallback list spec to torspec
Tor Bug Tracker & Wiki
blackhole at torproject.org
Wed Dec 27 10:49:04 UTC 2017
#24742: Add fallback list spec to torspec
---------------------------------------+-----------------------------------
Reporter: teor | Owner: teor
Type: enhancement | Status: needs_review
Priority: Medium | Milestone: Tor:
| 0.3.3.x-final
Component: Core Tor/Fallback Scripts | Version:
Severity: Normal | Resolution:
Keywords: fallback, torspec | Actual Points: 1.0
Parent ID: #24725 | Points: 0.5
Reviewer: atagar | Sponsor:
---------------------------------------+-----------------------------------
Changes (by teor):
* reviewer: => atagar
* actualpoints: => 1.0
Comment:
The revised spec is:
Branch fallback-format-2 on https://github.com/teor2345/torspec.git
The revised code is:
Branch fallback-format-2-v2 on https://github.com/teor2345/tor.git
The revised example file is:
https://trac.torproject.org/projects/tor/attachment/ticket/22759/fallback_dirs_new_format_version.4.inc
In particular, I rewrote the spec to be line-based, and to reference
definitions in the Onionoo spec or dir-spec.txt as appropriate.
Replying to [comment:5 atagar]:
> Thanks Tim. For other's reference
[https://github.com/teor2345/torspec/blob/fallback-format-2/fallback-
spec.txt here's the link].
>
> Very well written, nice work! Only a few minor thoughts...
>
> * In the 'Fallback Summary' lets explicitly say 'The separator MUST not
appear within the user readable comments.' Since parsers will be using it
to determine where it ends that would of course be a bad thing. :)
I made this much clearer:
https://github.com/teor2345/torspec/blob/fallback-format-2/fallback-
spec.txt#L189
In particular:
{{{
The fallback summary section MUST NOT be a valid fallback entry.
The fallback summary MUST end with a section separator:
separator WS* NL
There MUST NOT be any section separations in the fallback summary
section, other than the terminating section separator.
}}}
> * Concerning the headers I just had an idea: how about a revision
number? Stem uses tor's git commit to determine "Have I already read this
copy of the fallbacks file?". If the file instead had an incrementing
number I could use that instead ("Oh hey, I've already parsed revision 64.
Nothing to do.").
>
> Not necessary in the least. Just an idea.
The file generation does not parse the previous file, so a revision number
would be difficult.
But a timestamp should allow you to work out if the file has changed:
https://github.com/teor2345/torspec/blob/fallback-format-2/fallback-
spec.txt#L152
> * Not certain I know what all "A fallback entry consists of a C string
constant" entails. If you simply mean that the fields are quoted then lets
say that instead.
https://github.com/teor2345/torspec/blob/fallback-format-2/fallback-
spec.txt#L217
{{{
DQUOTE dir_address WS "orport=" ipv4_or_port WS
"id=" fingerprint DQUOTE WS* NL
}}}
> * For the ipv6_address lets specify that the address is fully expanded
(ie, nothing like '::').
The script just outputs whatever Onionoo provides, so we're stuck with
that,
unless we change the code. I've made this clearer:
https://github.com/teor2345/torspec/blob/fallback-format-2/fallback-
spec.txt#L93
{{{
The ipv6_or_address is an IPv6 address and port from the Onionoo
or_addresses list. The address MAY be in the canonical RFC 5952
IPv6 address format.
}}}
> * Concerning nickname and extrainfo it's not jumping out to me to expect
those to be C-style comments rather than quoted.
Yes, the previous description was very lax. And the likely outcome of that
was N different parser implementations. (It's already hard enough to
define a format that's C code, that parses as torrc lines, *and* includes
some other structure that the compiler will ignore.)
Here's what they look like now:
https://github.com/teor2345/torspec/blob/fallback-format-2/fallback-
spec.txt#L279
{{{
"/*" WS "nickname=" nickname WS "*/" WS* NL
[Zero or more times.]
The nickname for this FallbackDir, as defined by Onionoo. The
transitional 2.0.0 format has zero-length nicknames.
}}}
> * Total aesthetic thing but I'd include more equal signs in the divider,
but do whatever you think makes the file the nicest to read. :)
I spent several minutes doing internal bikeshedding on this.
And then I resolved not to spend any more time on it.
I think this depends on the user's editor, screen, and visual aesthetic.
> * On my first read I missed that you were listing fields rather than
lines. Every other spec we have this would mean to expect the orport line
to be separate from id line, etc.
>
> Mind if we instead do a per-line spec instead? That is to say tell
users they can expect...
>
> DQUOTE AddressIPv4 ":" DirPort SP "orport=" OrPort SP "id="
Fingerprint DQUOTE NL [[BR]]
> [ Exactly once ] [[BR]][[BR]]
> DQUOTE SP "ipv6=^^[^^" AddressIPv6 "]:" Port DQUOTE NL [[BR]]
> [ Any number of times ][[BR]][[BR]]
> ... etc...
It's now a per-line spec, see above for examples.
Once you're happy, please flip this to merge_ready, and nickm will look at
it when he's back.
(I'll send an update to tor-dev@, and give irl a chance to weigh in. But
we can always do revisions later.)
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/24742#comment:6>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
More information about the tor-bugs
mailing list