Fixes a security vulnerability where ANSI escape sequences in user input
could be injected into terminal output, potentially allowing attackers to
manipulate terminal behavior through log messages and error displays.
The vulnerability occurred when user-controlled content was formatted using
Display (`{}`) instead of Debug (`{:?}`) formatting, allowing raw ANSI
sequences to pass through unescaped.
Changes:
- Add streaming ANSI escape wrapper to avoid string allocations
- Escape message content in default and pretty formatters
- Escape error Display content in all error formatting paths
- Add comprehensive integration tests for all formatter types
The fix specifically targets untrusted user input while preserving the
ability for applications to deliberately include formatting in trusted
contexts like thread names.
Security impact: Prevents terminal injection attacks such as title bar
manipulation, screen clearing, and other malicious terminal control
sequences that could be injected through log messages.
This is useful when using `EnvFilter` for multiple identical per-layer
filters, as well as with clap and similar libraries that have `Clone`
bounds.
We generally expect users to be cloning an `EnvFilter` before attaching it
to a subscriber, rather than cloning `EnvFilters` that are already
attached. Because of this, we reset all the accumulated dynamic state
when cloning. This means that some spans and callsites might be missed
when an already-attached `EnvFilter` is cloned, but the presence of the
dynamic state mean that detaching and attaching `EnvFilter`s to existing
subscribers (e.g. with `reload`) already doesn't work very well. This
isn't a new class of problem.
There was a previous implementation of this in #2398, that shared the
dynamic state between all cloned filters behind an `Arc`. I chose
not do go for that approach because it causes inconsistencies if the
cloned filters are attached to different subscribers.
Fixes: #2360
* Implement PartialOrd for MatchPattern, MatchDebug in terms of Ord
* Fix missing link to supported regex syntax
* Update expected trybuild output
* Fix tracing_subscriber::Layer links
## Motivation
The new `main` branch is forked from the `v0.1.x` branch. It will be
made the default branch and going forward we will merge PRs to this
branch first (and then forward port to a new `v0.2.x` branch forked from
`master`).
It looks like Netlify jobs weren't running on the `v0.1.x` branch, and so
there were quite a few errors in the docs on that branch (which isn't great
because those are the ones that get published to docs.rs).
## Solution
Separate to this PR, we've enabled Netlify on the `main` branch, and this
change fixes all the errors that were present in the docs.
This change sets the GitHub actions to run on the `main` branch instead
of `v0.1.x`. It also adds some text in the root README.md which
describes the branch set-up.
Refs: #3294
It can be useful to have a TestWriter that does not log to stdout but
stderr instead. For example, that allows for potentially easier
filtering of tracing output (because the remaining output of, say, cargo
test goes to stdout) or to mirror behavior of env_logger, which by
default logs to stderr.
Introduce the TestWriter::with_stderr() constructor to enable such
usage. The default is left unchanged.
Co-authored-by: David Barsky <me@davidbarsky.com>
There is a report in #3174 that even in release mode, building the regex
used to parse `EnvFilter` directives can take a relatively large amount
of time (600us).
This change replaces the `regex` based parsing of the directives with a
state machine implementation that is faster and also easier to reason
about.
Fixes: #3174
## Motivation
The current behaviour of `DefaultVisitor` is that it will write
padding even if it is going to skip writing a value, which results
in extraneous padding being added when values are skipped
by the `tracing-log` integration.
## Solution
With this change, `DefaultVisitor` will only insert padding if it is
actually going to write a value.
Closes: #2979
We had some broken link formatting in the `tracing-journald` docs which
clippy picked up (the text looked like a link definition, but wasn't
meant to be).
The incorrect links have now been corrected. They have to link to the
`tracing-core` crate because `tracing-journald` doesn't depend on
`tracing` directly.
Fixes for a broken link in the `tracing-subscriber` main page and
correcting the link to `Collect` from `tracing-log` (which also doesn't
depend on `tracing` directly) were also included.
The change #2954 was released in 0.3.19 (#3162).
Notably, it relied on features from tracing-core 0.1.33, however, the
version was never bumped. Users of the `tracing` feature of
`tracing-subscriber` would have no issue since it pulls in the higher
version transitively.
The specific feature used was implementing trait method `record_bytes`
from the `field::Visit` trait on `JsonVisitor` from the
tracing-subscriber json format module. (see linked #2945, or [ `impl
field::Visit for
JsonVisitor<'_>`](https://github.com/tokio-rs/tracing/blame/master/tracing-subscriber/src/fmt/format/json.rs#L491))
I believe this dependency mismatch requires users to manually select the
higher tracing-core version or suffer compilation failure.
This probably reflects some failure in how the tests utilize features
and intra-workspace dependencies, but, a resolution for that is beyond
my current comprehension of the project.
There was only a single case of the new `needless_as_bytes` lint which
was triggered and needed to be fixed.
There was also a "UI" test in `tracing-attributes` that needed to be
updated because the error text has changed (it gives more details of
course).
Most of these changes are places where lifetimes were named, but can be
elided. Then a few cases where a lifetime was elided, but actually
resolves to a named lifetime. So lots of lifetimes.
This is the `v0.1.x` branch sister PR to #3164 (for the `master`
branch), since `clippy --fix` on another branch is a much better way to
apply these changes than backporting.
The commit 1cb523b87d3d removed this cfg gate on master. However, when
the change was backported in 1cb523b87d3d the docs were updated but the
cfg change was omitted.
This made the docs misleading, since they say "This method itself is
still available without the feature flag."
This modifies the `tracing_subscriber::reload` layer to also set the
`log` crate's max level with the current max `tracing` level filter
after reloading. If reloading the subscriber caused the max `tracing`
level to change, this ensures that the change is propagated to the `log`
crate as well. In the case where the max level was made more verbose,
this will ensure that `log` records which were previously disabled are
enabled correctly; in the case where it was made less verbose, this
improve performance by not having to perfrom more costly filtering for
those `log` records.
The `log` max level is set only after rebuilding the callsite interest
cache, which is what sets the max `tracing` level filter. This ensures
that we pass the latest state to the `log` crate.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
In the documentation of the layer context span_scope method, the note
contained a reference to a `scope()` method, which was removed some time
ago. Also fixed a phrasing error above.
Fixes: #2890
Users may want to pass data to `Subscribe`s which is not valid UTF-8. Currently, it would have to be encoded into `&str` to be passed as a field value.
This branch adds a `record_bytes` method to `Visit`. It has an implementation falling back to `record_debug` so it is not be a breaking change.
`JsonVisitor` got an overridden implementation as it should not use `Debug` output and encode it as a string, but rather store the bytes as an array.
`PrettyVisitor` go an overridden implementation to make the output more pretty.
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
This results in a substantial performance improvement,
and is compatible with our MSRV.
Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
fixes https://github.com/tokio-rs/tracing/actions/runs/6785393202/job/18443641813
cargo test runs tests in the same file in parallel by default, causing race condition,
this can be proven by running
`cargo test --test reload -- --test-threads=1` => successes
`cargo test --test reload -- --test-threads=2` => flaky
multiple times
This fix runs only the two tests in serial.
We could seperate the tests in different files, but they share the same testing dependencies, so I left them in the same file.
There has been interest around publishing tracing-mock to crates.io
for some time. In order to make this possible, documentation and some
code clean up is needed.
The `expect` module, which contains constructor functions for many of
the other `tracing-mock` modules needs documentation and examples.
This change adds documentation to the `expect` module and all the public
APIs within it. This includes doctests on all the methods which serve as
examples.
The lint for `missing_docs` has been enabled for the entire
`tracing-mock` crate! This has been done together with all the
other lints that are enabled on the other crates in this project.
The `event::msg("message")` constructor was removed, in favor of
requiring an explicit construction via
`expect::event().with_fields(expect::msg("message"))`. This is
appropriate to reduce the API surface that would need to be supported in
the future and also because the `event::msg` constructor could be
overridden by a subsequent usage of `with_fields`. The shorthand
`expect::message()` was renamed to `expect::msg` to make this
change less burdensome.
The `span::named("name")` constructor was removed, in favor of requiring
an explicit construction via `expect::span.with_name("name")`. The
latter isn't much longer and since #3097, a string with the name can
be passed directly everywhere that an `ExpectedSpan` is required.
This change also sets the `missing_docs` lint to warn for the entire
`tracing-mock` crate, making it ready to publish (once backported).
Refs: #539
This change adds documentation to the tracing-mock span module and all
the public APIs within it. This includes doctests on all the methods
which serve as examples.
Additionally, the validation on `ExpectedSpan` was improved so that it
validates the level and target during `enter` and `exit` as well as on
`new_span`.
The method `ExpectedSpan::with_field` was renamed to `with_fields`
(plural) to match the same method on `ExpectedEvent` (and because
multiple fields can be passed to it).
A copy-paste typo was also fixed in the documentation for
`ExpectedEvent::with_contextual_parent`.
Refs: #539
Co-authored-by: David Barsky <me@davidbarsky.com>
With the release of Rust 1.74, there are some new or modified clippy
lints that need adaption in the code.
The main change was the removal of the `private_in_public`.
https://rust-lang.github.io/rfcs/2145-type-privacy.html
Then two more changes were required, in one case to adhere a lint and
the other to allow it. When talking about what an "application" needs to
do when setting up `tracing-error`, it makes sense to include `fn
main()` in the doctest, even though the lint recommends against it.
; Conflicts:
; examples/examples/map-traced-error.rs
The `from_env` and `try_from_env` methods on the builder had the same documentation. This change updates their docs to correctly describe their difference in behavior.
## Motivation
Make the docs more clear, so that users need not look at the source to understand the difference between these two functions.
## Solution
Updated the docs
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
As seen here #2512 and #2223. Previously pr'ed here #2525, but no progress has
been made there for quite some. I have applied the suggested changes. Not sure
the formatting of the doc is sufficient or otherwise correct
## Solution
Make the `format::Writer::new()` function public. I don't see any obvious
trade-offs, but I am not familiar with the larger direction of
tracing/subscriber, so I may be wrong.
Closes #2512Closes#2223
Co-authored-by: Cephas Storm <cephas.storm@borisfx.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
It's currently awkward to have an optional per-layer filter.
Implement `Filter<L>` for `Option<F> where F: Filter<L>`, following the example
of `Layer`. A `None` filter passes everything through.
Also, it looks like Filter for Arc/Box doesn't pass through all the methods, so
extend the `filter_impl_body` macro to include them.
This also adds a couple of tests and updates the docs.
---------
Co-authored-by: David Barsky <me@davidbarsky.com>
Co-authored-by: Ryan Johnson <ryantj@fb.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Issue https://github.com/tokio-rs/tracing/issues/2080 explains that it's not
possible to soundly use
[`tracing_subscriber::fmt::time::LocalTime`](https://docs.rs/tracing-subscriber/latest/tracing_subscriber/fmt/time/struct.LocalTime.html)
in a multithreaded context. It proposes adding alternative time formatters that
use the [chrono crate](https://docs.rs/chrono/latest/chrono/) to workaround
which is what this PR offers.
A new source file 'chrono_crate.rs' is added to the 'tracing-subscriber'
package implementing `mod chrono_crate` providing two new tag types `LocalTime`
and `Utc` with associated `time::FormatTime` trait implementations that call
`chrono::Local::now().to_rfc3339()` and `chrono::Utc::now().to_rfc3339()`
respectively. Simple unit-tests of the new functionality accompany the
additions.
---------
Co-authored-by: David Barsky <me@davidbarsky.com>
Co-authored-by: Shayne Fletcher <shaynefletcher@meta.com>
This branch fixes a handful of new warnings which have shown up after
updating to Rust 1.72.0.
This includes:
* `clippy::redundant_closure_call` in macros --- allowed because the
macro sometimes calls a function that isn't a closure, and the closure
is just used in the case where it's not a function.
* Unnecessary uses of `#` in raw string literals that don't contain `"`
characters.
* Dead code warnings with specific feature flag combinations in
`tracing-subscriber`.
In addition, I've fixed a broken RustDoc link that was making the
Netlify build sad.
It's necessary at times to be able to disable ANSI color output for
rust utilities using `tracing`. The informal standard for this is the
`NO_COLOR` environment variable described here: https://no-color.org/
Further details/discussion in #2388
This commit updates `fmt::Layer` to check the `NO_COLOR`
environment variable when determining whether ANSI color output is
enabled by default. As described in the spec, any non-empty value set
for `NO_COLOR` will cause ANSI color support to be disabled by default.
If the user manually overrides ANSI color support, such as by calling
`with_ansi(true)`, this will still enable ANSI colors, even if
`NO_COLOR` is set. The `NO_COLOR` env var only effects the default
behavior.
Fixes#2388
Currently, in many places, macros do not use fully qualified names for
items exported from the prelude. This means that naming collisions
(`struct Some`) or the removal of the std library prelude will cause
compilation errors.
- Identify and use fully qualified names in macros were we previously
assumed the Rust std prelude. We use `::core` rather than `::std`.
- Add
[`no_implicit_prelude`](https://doc.rust-lang.org/reference/names/preludes.html#the-no_implicit_prelude-attribute)
to `tracing/tests/macros.rs`. I'm unsure if this is giving us good
coverage - can we improve on this approach? I'm not confident I've
caught everything.
There has been interest around publishing `tracing-mock` to crates.io
for some time. In order to make this possible, documentation and some
code clean up is needed.
This change adds documentation to the collector module itself and to all the
public APIs in the module. This includes doctests on all the methods
that serve as examples.
Additionally the implementation for the `Expect` struct has been moved
into the module with the definition, this was missed in #2369.
Refs: #539
* mock: change helper functions to `expect::<thing>`
The current format of test expectations in `tracing-mock` isn't ideal.
The format `span::expect` requires importing `tracing_mock::<thing>` which
may conflict with imports from other tracing crates, especially
`tracing-core`.
So we change the order and move the functions into a module called
`expect` so that:
* `event::expect` becomes `expect::event`
* `span::expect` becomes `expect::span`
* `field::expect` becomes `expect::field`
This format has two advantages.
1. It reads as natural English, e.g "expect span"
2. It is no longer common to import the modules directly.
Regarding point (2), the following format was previously common:
```rust
use tracing_mock::field;
field::expect();
```
This import of the `field` module may then conflict with importing the
same from `tracing_core`, making it necessary to rename one of the
imports.
The same code would now be written:
```rust
use tracing_mock::expect;
expect::field();
```
Which is less likely to conflict.
This change also fixes an unused warning on `MockHandle::new` when the
`tracing-subscriber` feature is not enabled.
Refs: #539
The `tracing-mock` crate provides a mock collector (and a subscriber for
use by the tests in the `tracing-subscriber` crate) which is able to
make assertions about what diagnostics are emitted.
These assertions are defined by structs that match on events, span, and
their fields and metadata. The structs that matched these objects have
been called, up until now, mocks, however this terminology may be
misleading, as the created objects don't mock anything.
There were two different names for similar functionality with `only()`
and `done()` on fields and collectors/subscribers respectively. Using a
single name for these may make it easier to onboard onto `tracing-mock`.
To reduce confusion, these structs have been split into two categories:
mocks and expectations.
Additionally, the `done()` function on the `Collector` and `Subscriber`
mocks has been replaced with `only()`. This matches the similar function
for `ExpectedField`, and may be more intuitive.
The mocks replace some component in the tracing ecosystem when a library
is under test. The expectations define the assertions we wish to make
about traces received by the mocks.
Mocks (per module):
* collector - `MockCollector`, no change
* subscriber - `MockSubscriber`, renamed from `ExpectSubscriber`
Expectations (per module):
* event - `ExpectedEvent`, renamed from `MockEvent`
* span - `ExpectedSpan`, renamed from `MockSpan`
* field - `ExpectedField` and `ExpectedFields`, renamed from `MockField`
and `Expected`. Also `ExpectedValue` renamed from `MockValue`.
* metadata - `ExpectedMetadata`, renamed from `Expected`
Refs: #539
## Motivation
This was inconsistent with other features, which mention their
requirements, and a reader might assume they don't need to inspect the
feature flags page or manifest.
# 0.3.17 (April 21, 2023)
This release of `tracing-subscriber` fixes a build error when using
`env-filter` with recent versions of the `regex` crate. It also
introduces several minor API improvements.
### Fixed
- **env-filter**: Add "unicode-case" and "unicode-perl" to the `regex`
dependency, fixing a build error with recent versions of `regex`
(#2566)
- A number of minor documentation typos and other fixes (#2384, #2378,
#2368, #2548)
### Added
- **filter**: Add `fmt::Display` impl for `filter::Targets` (#2343)
- **fmt**: Made `with_ansi(false)` no longer require the "ansi" feature,
so that ANSI formatting escapes can be disabled without requiring
ANSI-specific dependencies (#2532)
### Changed
- **fmt**: Dim targets in the `Compact` formatter, matching the default
formatter (#2409)
Thanks to @keepsimple1, @andrewhalle, @LeoniePhiline, @LukeMathWalker,
@howardjohn, @daxpedda, and @dbidwell94 for contributing to this
release!
