Opened 6 years ago

Last modified 4 weeks ago

#2362 new defect

"GETINFO config-text" adds spurious DataDirectory, Log entries

Reported by: atagar Owned by:
Priority: Low Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-relay tor-control torrc easy
Cc: Actual Points:
Parent ID: Points: 2
Reviewer: Sponsor:


Repro steps:

  • use a blank torrc (just setting the ControlPort)
  • run tor
  • issue a GETINFO get-text

result: it provides a 'Log notice stdout' entry
expected: we should only get back entries that differ from their defaults (in this case only the ControlPort)

from irc:
15:03 < armadev> atagar: it is a tor bug, i believe. i just reproduced.
15:03 < atagar> ok, I'll file a ticket - thanks :)
15:04 < armadev> if (smartlist_len(elts) == 0)
15:04 < armadev> smartlist_add(elts, tor_strdup("stdout"));
15:04 < armadev> in options_init_logs()
15:04 < armadev> that's where it comes from. we add it if there are no other logs.
15:04 < armadev> hmmmm. no, i take that back.
15:05 < armadev> /* Special case on first boot if no Log options are given. */
15:05 < armadev> if (!options->Logs && !options->RunAsDaemon && !from_setconf)
15:05 < armadev> config_line_append(&options->Logs, "Log", "notice stdout");
15:05 < armadev> there we go.

Child Tickets

Change History (9)

comment:1 Changed 6 years ago by arma

  • Milestone set to Tor: 0.2.3.x-final

The trouble here is that logs are a linelist, so a controller that appends a new log entry to the linelist will leave the old one around, and our original behavior goal was to have the stdout log entry only when there aren't any others.

Option one is to ignore it until a controller that uses Tor where stdout is relevant actually wants to do this behavior of modifying log lines. Is arm there yet, atagar?

Option two is to tag torrc options with "not for save" or something. Ugh.

Option three is to special-case a stdout log and not write it if it's the only line and it wasn't explicitly asked for. Semi-ugh.

Option four is ...?

comment:2 follow-up: Changed 6 years ago by atagar

Just discovered that it also autogenerates a nickname field if configured to be a relay. For instance with the torrc of:
ORPort 9001
ExitPolicy reject *:*
ControlPort 9051
CookieAuthentication 1

"GETINFO config-text" provides:
ORPort 9001
ControlPort 9051
Nickname fenrir
ExitPolicy reject *:*
CookieAuthentication 1

This isn't really a problem, though we should be better documenting SAVECONF (which by extension defines 'GETINFO config-text' behavior). Currently it doesn't really say what options it decides to save or omit - in practice this seems to be "Writes config options that differ from their defaults, with the exception of Log and Nickname which might be saved anyway".

comment:3 Changed 5 years ago by nickm

  • Milestone changed from Tor: 0.2.3.x-final to Tor: unspecified

comment:4 Changed 5 years ago by nickm

  • Keywords tor-relay added

comment:5 Changed 5 years ago by nickm

  • Component changed from Tor Relay to Tor

comment:6 in reply to: ↑ 2 Changed 3 years ago by arma

Replying to atagar:

Just discovered that it also autogenerates a nickname field if configured to be a relay.

For posterity: this behavior was removed in as part of #2979.

comment:7 Changed 3 years ago by arma

Imo the fix here should be to separate the list of logs that the torrc requests from the list of logs that are active right now. That way we can add a new temporary stdout log to the 'active' list without modifying the 'requested' list.

comment:8 Changed 4 weeks ago by nickm

  • Keywords tor-control torrc added
  • Points set to 2
  • Severity set to Normal
  • Summary changed from get-text always reports Log entry to "GETINFO config-text" adds spurious DataDirectory, Log entries

The current output, given an empty file with only a ControlPort, is:

ControlPort 9999
DataDirectory /home/nickm/.tor
Log notice stdout

comment:9 Changed 4 weeks ago by nickm

  • Keywords easy added

The correct fix here is to apply the same pattern we apply elsewhere when we're thinking about changing the get_options() content depending on anything besides a new torrc or whatever: we have one option that represents the value-as-set-by-the-user, and one that represents the value-as-used.

Note: See TracTickets for help on using tickets.