Feature: `assert_impl_any!`
nvzqz opened this issue · 9 comments
Asserts a given type implements any of a given set of traits.
I implemented an assert_impl_any macro that mostly works. A blanket trait with a specific method name can cause the macro to fail when it would usually succeed but it can't cause the macro to succeed where it should fail so its not really a problem.
macro_rules! assert_impl_any {
($x:ty: $($t:path),+ $(,)?) => {
const _: fn() = || {
let previous = {
struct AssertImplAnyFallback;
AssertImplAnyFallback
};
// Ensures that blanket traits can't impersonate the method.
struct __ActualAssertImplAnyToken;
$(
let previous = {
struct __Wrapper<T, N>(core::marker::PhantomData<T>, N);
// If the `__static_assert__impl_any_trait` method for this wrapper can't be called then
// the compiler will insert a deref and try again. This forwards the compilers next
// attempt to the previous wrapper.
impl<T, N> core::ops::Deref for __Wrapper<T, N> {
type Target = N;
fn deref(&self) -> &Self::Target {
&self.1
}
}
// This impl has a trait bound on $x so the method can only be called if $x implements $t.
impl<T: $t, N> __Wrapper<T, N> {
#[allow(non_snake_case)]
fn __static_assert__impl_any_trait(&self) -> __ActualAssertImplAnyToken { __ActualAssertImplAnyToken }
}
__Wrapper::<$x, _>(core::marker::PhantomData, previous)
};
)+
{
trait AssertImplAnyToken: Sized {}
impl AssertImplAnyToken for __ActualAssertImplAnyToken {}
fn assert_impl_any_token<T: AssertImplAnyToken>(_token: T) {}
// Attempt to find a `__static_assert__impl_any_trait` method that can actually be called.
// The found method must return a type that implements the sealed `Token` trait, this ensures that blanket trait methods can't cause this macro to compile.
assert_impl_any_token(previous.__static_assert__impl_any_trait());
}
};
};
}Playground link (includes some tests)
This code uses a trick that I also made a comment about in snafu issue #88.
@Lej77 Thank you so much for this! I wouldn't even have thought of doing it this way. I implemented a simpler version of this based on your implementation (see 59ac54f).
It didn't make sense to me as to why the AssertImplAnyToken trait was necessary so I used the ActualAssertImplAnyToken type directly.
The reason for the AssertImplAnyToken trait was to ensure that a blanket trait that was in scope couldn't cause the macro to actually compile when it shouldn't. The interference_for_fail_case test case on the playground showed how this could be done:
use playground ::assert_impl_any;
trait Interfere<T> {
#[allow(non_snake_case)]
fn _static_assertions_impl_any(&self) -> T { loop {} }
}
impl<T, Token> Interfere<Token> for T {}
// These should fail to compile but the above trait might interferes and causes them to actually compile anyway. Fortunately the function must return a special type to compile and that isn't possible.
assert_impl_any!((): From<u8>);
assert_impl_any!((): From<u8>, From<u16>);If you use the type directly then the users blanket trait can have a generic return type and compile correctly while if type inference only know that the return type must implement a trait then that won't work.
It probably would be better to do something like this so that the token trait isn't in scope for the macro variables (notice that the trait is in a block with a new scope):
{
trait AssertImplAnyTokenTrait {}
impl AssertImplAnyTokenTrait for AssertImplAnyToken {}
fn assert_impl_any_token<T: AssertImplAnyTokenTrait>(_token: T) {}
assert_impl_any_token(previous._static_assertions_impl_any());
}I updated my previous comment's suggested code to try and minimize possible name collisions with user provided types and identifiers in the macro.
Ok I follow as to how it prevents that case from compiling. However, I feel as though this adds a bit too much complexity. I'm leaning towards it not being worth it just to stop people from bypassing this with _static_assertions_impl_any.
In terms of how this could be used in a malicious way outside of the user's crate... I suppose one could add _static_assertions_impl_any! to a commonly used trait with #[doc(hidden)]. But I find that to be extremely unlikely.
I'll add this protection for that reason, but I'm not happy with the added complexity. But, then again, this crate is filled with obscure and complex stuff anyway. 😅
Yeah, I would like it to be simpler and more elegant too but I thought that it was important that the assert couldn't compile when it shouldn't. The scenario I imagined was reading other peoples code and them having intentionally (maliciously?) interfered with an assert so that the reader would assume something untrue.
I saw that you used this macro to implement an assert_impl_one macro. I think that macro can actually be implemented using the same trick as the assert_not_impl_any macro instead:
macro_rules! assert_impl_one {
($x:ty: $($t:path),+ $(,)?) => {
const _: fn() = || {
// Generic trait.
trait AmbiguousIfMoreThanOne<A> {
// Required for actually being able to reference the trait.
fn some_item() {}
}
// Creates multiple scoped `Token` types for each trait `$t`, over
// which a specialized `AmbiguousIfMoreThanOne<Token>` is implemented
// for every type that implements `$t`.
$({
#[allow(dead_code)]
struct Token;
impl<T: ?Sized + $t> AmbiguousIfMoreThanOne<Token> for T {}
})+
// If there is only one specialized trait impl, type inference with
// `_` can be resolved and this can compile. Fails to compile if
// `$x` implements more than one `AmbiguousIfMoreThanOne<Token>`.
let _ = <$x as AmbiguousIfMoreThanOne<_>>::some_item;
};
};
}The scenario I imagined was reading other peoples code and them having intentionally (maliciously?) interfered with an assert so that the reader would assume something untrue.
Isn't that just shooting yourself in the foot as the crate author? I can't imagine a sane reason to trick the readers of your code that way. Seeing an unused _static_assertions_impl_any is a big red flag that something weird is up.
Huh, so while that is more obscure than the current implementation of assert_impl_one!, it does seem less complex. It's not self-referential and this sort of thing already exists in the same file. Also, it's not prone to the _static_assertions_impl_any "attack". I think I'll go with this approach instead, thanks!
I imagine that the _static_assertions_impl_any "attack" will probably only happen intentionally (it does seem quite unlikely) and I think the largest reason to do so would be to try and introduce a security vulnerability in some code that is reliant on some invariant that the assert ensures. The reason a crate author would do it to their own crate would be to try and trick the reader that their crate is safe when it really contains a vulnerability that the author could somehow exploit. I don't really know much about security but I assume something like this is possible.