tracing-subscriber: EnvFilter parsing regression in 0.3.20

[jet-tong]

Bug Report

Version

tracing-subscriber v0.3.20
tracing v0.1.41
tracing-core v0.1.34
tracing-log v0.2.0

Platform

Github Runners: manylinux x86_64/aarch64, macosx x86_64/arm64.

Crates

tracing-subscriber v0.3.20

Description

Parsing regression in tracing-subscriber 0.3.20: Invalid filter directives are incorrectly accepted as valid target names. This broke our negative tests that expect invalid filters to be rejected.

This looks like it just needs the same character validation applied to the LevelOrTarget state that's already used in the Start state.

Reproduction

[dependencies]
tracing-subscriber = { version = "0.3.20", features = ["env-filter"]}

use std::env;
use tracing_subscriber::EnvFilter;

fn main() {
    env::set_var("TEST_ENV_VARIABLE", "invalid123.&/?");
    
    match EnvFilter::try_from_env("TEST_ENV_VARIABLE") {
        Ok(filter) => println!("PARSED OK: {:?}", filter),
        Err(e) => println!("ERROR: {}", e),
    }
}

0.3.19 (expected behaviour):

ERROR: invalid filter directive

0.3.20 (regression):

PARSED OK: EnvFilter { statics: DirectiveSet { directives: [StaticDirective { target: Some("invalid123.&/?"), level: LevelFilter::TRACE }], max_level: LevelFilter::TRACE }, dynamics: DirectiveSet { directives: [], max_level: LevelFilter::OFF }, has_dynamics: false, by_id: RwLock { data: {}, poisoned: false, .. }, by_cs: RwLock { data: {}, poisoned: false, .. }, scope: ThreadLocal { local_data: None }, regex: true }

Root Cause

PR #3243 replaced regex parsing with a state machine in directive.rs, but it accepts any characters after a valid start, allowing invalid characters like &, /, ? to pass through in directive.rs#L170:

(state @ LevelOrTarget { .. }, _) => state,

The fallback logic in directive.rs#219 then treats the invalid string as a valid target name:

// Setting the target without the level enables every level for that target
Err(_) => (LevelFilter::TRACE, Some(level_or_target.to_owned())),

Potential Fix

Add character validation in the LevelOrTarget state, using the same logic used in the Start state:

(state @ LevelOrTarget { .. }, c) => {
    if !['-', ':', '_'].contains(&c) && !c.is_alphanumeric() {
        return Err(ParseError::new())
    }
    state
},

[barafael]

I'm affected by this too.

[location] build_filter("not-'a'-filter")? = EnvFilter {
    statics: DirectiveSet {
        directives: [
            StaticDirective {
                target: Some(
                    "not-'a'-filter",   # <-- this
                ),
                field_names: [],
                level: LevelFilter::TRACE,
            },
        ],
        max_level: LevelFilter::TRACE,
    },
    dynamics: DirectiveSet {
        directives: [],
        max_level: LevelFilter::OFF,
    },
    has_dynamics: false,
    by_id: RwLock {
        data: {},
        poisoned: false,
        ..
    },
    by_cs: RwLock {
        data: {},
        poisoned: false,
        ..
    },
    scope: ThreadLocal { local_data: None },
    regex: false,
}