Opened 16 months ago

Closed 4 months ago

#18146 closed defect (fixed)

Tor's control protocol misspells "Dependent"

Reported by: teor Owned by: jryans
Priority: Medium Milestone: Tor: 0.3.0.x-final
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Trivial Keywords: doc tor-controller
Cc: jryans@…, atagar Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In getinfo_helper_config(), tor misspells "Dependent" as "Dependant".

There's probably nothing we can do about this, because it's part of the controller interface.

But we should at least document it, preferably in the code and the control spec.

Child Tickets

Change History (18)

comment:1 Changed 16 months ago by teor

  • Milestone changed from Tor: very long term to Tor: 0.2.8.x-final

comment:2 Changed 16 months ago by nickm

  • Milestone changed from Tor: 0.2.8.x-final to Tor: 0.2.???

It is impossible that we will fix all 277 currently open 028 tickets before 028 releases. Time to move some out. This is my first pass through the "new" and "reopened" tickets, looking for things to move to ???.

comment:3 Changed 6 months ago by teor

  • Milestone changed from Tor: 0.2.??? to Tor: 0.3.???

Milestone renamed

comment:4 Changed 6 months ago by jryans

  • Cc jryans@… added
  • Owner set to jryans
  • Status changed from new to assigned

comment:5 Changed 6 months ago by jryans

  • Status changed from assigned to needs_review

comment:6 Changed 6 months ago by nickm

  • Milestone changed from Tor: 0.3.??? to Tor: 0.3.0.x-final

comment:7 Changed 5 months ago by arma

Is there some way to fix it, and tolerate both for now or something, so in the distant future we can get rid of the typo?

comment:8 Changed 5 months ago by arma

  • Cc atagar added

Damian, does arm or stem use the "getinfo config/names" feature? Maybe we can just fix it and break backward compatibility? :)

comment:9 Changed 5 months ago by teor

Since it's an output of the control protocol, we have to choose one or the other.

I suggest we fix the bug in 0.3.0, and note that it's a breaking change, and then say in the control spec that controllers should accept both, noting the version that the fix was made in (0.3.0.1-alpha, if we're lucky).

comment:10 Changed 5 months ago by teor

  • Status changed from needs_review to needs_revision

comment:11 Changed 5 months ago by atagar

Damian, does arm or stem use the "getinfo config/names" feature? Maybe we can just fix it and break backward compatibility? :)

Yes, both Stem and Nyx use 'GETINFO config/names'. Its used for tor-prompt's auto-completion for instance...

https://gitweb.torproject.org/stem.git/tree/stem/interpreter/autocomplete.py#n40

comment:12 Changed 5 months ago by nickm

It's not the end of the world to leave a misspelled string in an interface. HTTP has "Referer", after all.

comment:13 Changed 5 months ago by jryans

  • Status changed from needs_revision to needs_information

It seems nice to try fixing the misspelling if it's possible to do so. Maybe the community of Tor control protocol clients is small enough that it's within reason to try...?

Doing a quick skim of Stem and Nyx, it looks like:

  • Stem only uses the config name from getinfo config/names, not the type, so it would be unaffected by the proposed change here
  • Nyx does use the config type for a few cases, but seems to ignore this specific type (currently misspelled as Dependant)

atagar, is this analysis correct?

comment:14 Changed 5 months ago by atagar

Hi jryans, sorry about the delay. Was down for a week with a cold. Yup, looks right to me. I'm not spotting anywhere in Stem or Nyx where renaming this would impact us.

comment:15 Changed 5 months ago by jryans

  • Status changed from needs_information to needs_review

Thanks for the confirmation atagar!

Here's a different branch where I've make a breaking change to fix the spelling as teor suggested in comment 9, since it appears to be safe for the most common control protocol clients.

tor: https://github.com/torproject/tor/compare/master...jryans:dependant-corrected
torspec: https://github.com/torproject/torspec/compare/master...jryans:dependant-corrected

comment:16 Changed 5 months ago by teor

  • Status changed from needs_review to needs_revision
  • Version set to Tor: unspecified

Ok, one fix needed:

Replace "Tor XXX" with "Tor 0.3.0.2-alpha", as our best guess at the version this will be merged into.

comment:17 Changed 5 months ago by jryans

  • Status changed from needs_revision to merge_ready

Okay, updated with the expected fix version. (I assume another round of review is not needed for this change.)

comment:18 Changed 4 months ago by nickm

  • Resolution set to fixed
  • Status changed from merge_ready to closed

merged!

Note: See TracTickets for help on using tickets.