Tor changes default value of DirReqStatistics, then wants to SAVECONF the new default

#4237 (closed) was caused by [ticket:4237#comment:2 an underlying Tor bug].
We should fix it, and someday we should redesign the configuration-handling code to make this class of bugs go away.

changed milestone to %Tor: 0.2.6.x-final

added 026-triaged-1 component::core tor/tor milestone::Tor: 0.2.6.x-final priority::medium resolution::fixed status::closed tor-relay type::defect labels

This is also related to... https://trac.torproject.org/projects/tor/ticket/2362

Sebastian found that when Tor's geoip file is not available, Tor changes its default value for DirReqStatistics from 1 to 0, but then the SAVECONF code doesn't know about the new default value, so it wants to save DirReqStatistics 0 in a regenerated torrc.

Trac:
Summary: Fix #4237 (closed) to Tor changes default value of DirReqStatistics, then wants to SAVECONF the new default

Trac:
Milestone: Tor: 0.2.3.x-final to Tor: 0.2.4.x-final

(moved this to 0.2.4.x because of the amount of mechanism I suspect will be involved.)

Trac:
Keywords: N/A deleted, tor-relay added

Trac:
Component: Tor Relay to Tor

We might be able to do this by massaging global_default_options and global_options in sync.

Trac:
Keywords: tor-relay deleted, tor-relay 024-deferrable added

We can do this with global_default_options, but it turns out that there are enough other places we do this that it would be a mistake. Instead, we should have another or_options_t object that tracks torrc+settings, without changes made internally by Tor.

That's doable, but it's too feature-y for 0.2.4 IMO. I have a TOTALLY UNTESTED fix in branch "bug4244" in my public repository.

Trac:
Status: new to needs_review
Milestone: Tor: 0.2.4.x-final to Tor: 0.2.5.x-final

Trac:
Keywords: tor-relay 024-deferrable deleted, tor-relay, 024-deferrable, 025-triaged added

I can't see anything obviously wrong with this patch, but it seems a little complex for this stage in the game in 0.2.5 given the comparatively small size of the problem. Also, apparently the person who wrote it didn't write any tests for it. It might be best to let this alone until 0.2.6.... unless this is a much more serious problem than I think?

Deferring to 0.2.6. Needs testing to even be considered.

Trac:
Milestone: Tor: 0.2.5.x-final to Tor: 0.2.6.x-final
Status: needs_review to needs_revision

Trac:
Keywords: tor-relay, 024-deferrable, 025-triaged deleted, tor-relay 026-triaged-1 added

Add nickm-patch keyword to needs_revision tickets in 0.2.6 where (I think) I wrote the patch, so I know which ones are my responsibility to revise.

Trac:
Keywords: tor-relay 026-triaged-1 deleted, 026-triaged-1, tor-relay, nickm-patch added

My bug4244b branch has a much simpler, more targeted fix. I tested it and it works fine for me.

More generally, I think a trend of "thou shalt not modify the options to make it look like the user picked A when actually it said B" is something we should try to enforce.

I use NodeFamilySets, MaxMemInQueues, Socks4ProxyAddr, etc as my precedent here.

Note that this patch doesn't actually go through and fix every instance where we internally clobber what the torrc file said. But that makes it a reviewable and mergeable patch. :) Maybe it should even go into 0.2.5.x since TB users are seeing this one.

Trac:
Keywords: nickm-patch deleted, N/A added

Trac:
Status: needs_revision to needs_review

Almost right, but two problems:

• I have no way to remember whether the _ version or the no-_ version is the one we should be looking at. Let's make the name that the code is supposed to use easier to remember, so that we're less likely to use the wrong one by mistake.
• We no use identifiers that start with _, since those are reserved. Instead we could suffix with _.

Proposed fixup in "bug4244b" in my public repository. If you like, I'll squash and merge (or even cherry-pick to 0.2.5 is this is causing serious trouble)

Squashed as bug4244b_squashed and merged to master.

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

closed