This would be a break in higher-layer crates which incorproate this
error but:
1. That's just arti-client which hides it behind the detailed errors
cargo feature
2. I'm hoping cargo-semver-checks would spot it, anyway.
The effect is that everywhere a RetryError is used, the error sources
for the contained errors will be Display'd.
In tor-hsclient we no longer need to explicitly wrap things up in
tor_error::Report.
We don't use OsString now except where it appears in our public API,
or where we get it from std::env.
Moving the `use` statements into the use sites enabled me to see
that I had found all the places I wanted to change.
users is unmaintained. pwd-grp is the crate I have just written to
replace it. In this commit:
Change the cargo dependency and imports.
Replace the cacheing arrangements. users has a built-in cache;
pwd-grp doesn't. Now, instead of cashing individual lookups, we cache
the trusted user and trusted gid calculation results.
This saves on some syscalls, and is also more convenient to write.
(Mocking is still done via the dependency.)
Many systematic consequential changes of details:
* The entrypoint names to the library are different:
pwd-grp uses the names of the corresponding Unix functions.
* pwd-grp's returned structs are transparent, so we don't
call accessors for .uid(), .name(), etc.
* pwd-grp's methods are much more often fallible
(returning io::Result<Option<...>)
* We're using the non-UTF-8 pwd-grp API, which means we must
use turbofish syntax in some places.
* The mocking API is a bit different.
Add some wrapper functions for convenience.
The pwd-grp crate has a richer and more faithful, but not so
convenient, way of creating dummy user/group entries. Also the type
names are all going to change.
Doing this now reduces churn.
The actual underlying operations here *are* fallible.
The `users` crate hides those errors in several cases.
(Failures are very rare (at least unless NIS is involved), so this is
not of much practical import, but it's going to be necessary when we
use the more careful pwd-grp crate.
When we implemented bridges, we added code in 08473872ab to
conditionally mark their directory info as present or not present.
But the we didn't remove the old code to mark them present
unconditionally!
Fixes#638.
- When the `geoip` feature flag of `tor-netdir` is enabled, perform
GeoIP lookups for all relays added to the directory and add the
resulting country code to the `Relay` struct.
- The GeoIP database is provided in a new
`PartialNetDir::new_with_geoip` constructor.
- A new trait was also added to `tor-linkspec`, `HasCountryCode`, to
enable getting this data out from other crates.
Part of onionmasq#47.
Previously, the keystore config consisted of a single field in
`StorageConfig`, which encoded 2 bits of information: whether the
keystore is enabled, and its root directory:
```
[storage]
# use this path, fail if compiled out
# keystore = "/path/to/arti/keystore"
#
# use default path, fail if compiled out
# keystore = true
#
# disable
# keystore = false
```
This commit adds `ArtiNativeKeystoreConfig`, which will replace the
multi-purpose `keystore` field. The new config will look like this:
```
#[storage.keystore]
# Whether the keystore is enabled.
#
# If the `keymgr` feature is enabled and this option is:
# * set to false, we will ignore the configured keystore path.
# * set to "auto", the configured keystore, or the default keystore, if the
# keystore path is not specified, will be used
# * set to true, the configured keystore, or the default keystore, if the
# keystore path is not specified, will be used
#
# If the `keymgr` feature is disabled and this option is:
# * set to false, we will ignore the configured keystore path.
# * set to "auto", we will ignore the configured keystore path.
#
# Setting this option to true when the `keymgr` feature is disabled is a
# configuration error.
#enabled = "auto"
# The root directory of the arti keystore
#path = "${ARTI_LOCAL_DATA}/keystore"
```
While `ArtiNativeKeystoreConfig` currently only has 2 fields, `enabled`
and `path`, future versions of the keystore might require additional
config options.
There are a number of places where we generate random Durations
in a range which starts at zero.
These call sites currently (i) have to write out Duration::ZERO
or equivalent, and (ii) would have to use gen_range_checked and expect
the result, even though it can be statically proven to be OK.
To make this slightly smoother, provide `GenRangeInfallible` and
`gen_range_infallible`.
Ideally we would be allowed to use vanilla gen_range() here, but there
doesn't seem to be a way to allow a specific clippy-forbidden method
using #[allow] and we probably don't want to make a blanket allow.
This launders the closure so that clippy's
clippy::redundant_closure_call can't see it.
We can't have a local #[allow] because it would be on an expression,
which isn't allowed on stable.
This avoids having to use more clumsy idioms at call sites.
Originally they didn't check err.kind(), since err.kind() can never
increase their severity. We lost that behavior with !1386, and we
became dependent on it with arti!1383. Since they both merged at
the same time, CI broke.
This patch restores their original behavior.
This abolishes some quintuplication.
The output is identical except that:
* The syntax display in the rustdoc output for the resulting macros
seems to have somewhat less whitepsace.
* The whimsical error messages in the examples are all identical.
Ah well.
I identified the cases to replace by searching for the string
`.report()`. There are a few that I didn't change:
* A couple of cases that used anyhow::Error,
* One case that reported two Errors.
* Two cases in `tor_hsclient::err` that just did
`error!("Bug: {}")`.
I have also not audited the cases in `tor-hsclient` where we're using
`tor_error::Report` manually.
Nonetheless, closes#949.
Many of these call sites would panic if, somehow, the upper bound was
zero. In most cases it is very complicated to see if whether this
could happen.
However, there is a better answer:
Durations are (conceptually) dense, so picking the closed set (which
includes its boundary) rather than the open one (which doesn't) will
make little practical difference.
So change four call sites to use `..=` instead of just `..`.
Since we're going to be using `Option<CountryCode>` all over, let's
save the extra byte.
Sadly this required std::mem::transmute(), which is unsafe, so maybe
we should think twice.
The main contribution here is a set of convenience macros for
logging error `Report`s. Notably, this macros always logs
`Internal` and `BadAspiUsage` errors at `WARN`, unless they
are already at `ERROR` or more.
This is a little tricky because `tracing::event!()` requires
its Level argument to be a constant.
Nick Mathewson
Ian Jackson
juga
Alexander Færøy
gabi-250
eta
Saksham Mittal
Ian Jackson
Kunal Mehta
trinity-1686a