#19965 closed defect (fixed)

Log torrc option can't handle tab after severity

Reported by: arma Owned by: jryans
Priority: Medium Milestone: Tor: 0.3.0.x-final
Component: Core Tor/Tor Version: Tor: 0.2.1.10-alpha
Severity: Normal Keywords: regression
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Set your torrc file to be

log debug       file /tmp/debug.log

where that long space is a tab.

Then tor -f /path/to/torrc
and you'll get

Aug 23 15:23:49.300 [warn] Couldn't parse log levels in Log option 'Log debug        file /tmp/debug.log'
Aug 23 15:23:49.300 [warn] Failed to parse/validate config: Failed to validate Log options. See logs for details.
Aug 23 15:23:49.300 [err] Reading config failed--see warnings above.

It looks like the issue is in parse_log_severity_config() where we do stuff like

    space = strchr(cfg, ' ');

Reported by toralf.

Child Tickets

Change History (8)

comment:1 Changed 15 months ago by arma

I think technically this is a regression, since Tor 0.2.0.32 can handle this situation just fine.

Added in f56ba5f3d which went into 0.2.1.1-alpha.

comment:2 Changed 12 months ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:3 Changed 12 months ago by jryans

Owner: set to jryans
Status: newassigned

comment:4 Changed 12 months ago by jryans

Status: assignedneeds_review

I adjusted the parsing in parse_log_severity_config to accept any whitespace after the severity and added a basic unit test for this.

https://github.com/jryans/tor/commits/log-severity

comment:5 Changed 12 months ago by teor

Keywords: regression added
Milestone: Tor: 0.3.???Tor: 0.3.0.x-final
Status: needs_reviewmerge_ready
Version: Tor: 0.2.1.10-alpha

Seems good to me.
(I checked, and we don't do this anywhere else in option parsing.)

comment:6 Changed 12 months ago by cypherpunks

The NULL check at test_config.c:4917 is unnecessary because tor_free can handle NULLs.

comment:7 Changed 12 months ago by jryans

Thanks for pointing that out cypherpunks! Updated patch to remove redundant null check in test_config.c

comment:8 Changed 11 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

merged; thanks!

Note: See TracTickets for help on using tickets.