#32410 closed defect (fixed)

--disable-module-relay causes us to reject default options

Reported by: nickm Owned by: teor
Priority: High Milestone: Tor: 0.4.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords:
Cc: Actual Points: 0.2
Parent ID: Points: 0.1
Reviewer: nickm Sponsor: Sponsor31-must

Description

The default value for DirCache is 1. But when we build with --disable-module-relay, we reject that default, and so reject our default configuration.

I think it is reasonable to allow DirCache 1 in this case.

Child Tickets

Change History (3)

comment:1 Changed 12 months ago by teor

Actual Points: 0.1
Points: 0.1
Reviewer: nickm
Status: assignedneeds_review

nickm and I spoke about this issue on IRC.

Here's what the code already does:

  • remove all direct references to the DirCache option outside disabled relay code: except for routermode and relay_config, everywhere else uses dir_server_mode()
  • interpret DirCache as "become a directory cache if we are also a relay": dir_server_mode() is DirPort_set || (server_mode() && router_has_bandwidth_to_be_dirserver())

Here are the changes in this ticket:

  • change the default to DirCache 0 when tor is built with the relay module disabled
    • but keep the default DirCache 1 when we're building the unit tests
  • add a comment to the DirCache option that tells developers to use dir_server_mode() instead

See my PR:

I tested this fix by building with relay mode disabled, and then running src/app/tor DisableNetwork 1. Before the fix, it failed with a config error. After, it starts up with a valid config.

comment:2 Changed 12 months ago by teor

Actual Points: 0.10.2

A bunch of the relay parsing tests acted differently after this change, I think it's an improvement.

Except that tor-no-relay sets ClientOnly in all torrcs. So I changed the ClientOnly default to 1, just like I changed the DirCache default to 0.

I made that fix, updated the tests, and force-pushed.

comment:3 Changed 12 months ago by nickm

Resolution: fixed
Status: needs_reviewclosed

lgtm, merged to master

Note: See TracTickets for help on using tickets.