Skip to content

Sealing the Array trait broke downstream crates #9184

@alamb

Description

@alamb

Is your feature request related to a problem or challenge? Please describe what you are trying to do.
We recently had a security report filed here

The core issue is that there are places in the arrow-rs codebase that rely on the results returned from the Array trait to be consistent, otherwise undefined behavior can result

Our fix was to make the Array trait as sealed (which means it can not be implemented outside the arrow-rs crate).

While this certainly avoids the undefined behavior, it is a heavy hammer. Our initial assumption that implementing the Array trait was not common / good for other crates to extend the Array trait. However, we now have at least two reports that it was done:

Describe the solution you'd like
Given that there are (seemingly) a non trivial number of uses of Array, I think it is worth reconsidering the change and finding some other way to resolve the soundness bug.

For example, we could unseal Array

Describe alternatives you've considered
One thing we could potentially do is mark the Array trait as unsafe: https://doc.rust-lang.org/book/ch20-01-unsafe-rust.html#implementing-an-unsafe-trait

That way people could still implement Array, but would have to explicitly acknowledge that there is danger in doing so

Additional context

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementAny new improvement worthy of a entry in the changelog

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions