named-lock icon indicating copy to clipboard operation
named-lock copied to clipboard

Allow locks with arbitrary names

Open ryanavella opened this issue 3 years ago • 4 comments

This would be a major breaking change and I'm not completely sure if I like it myself, but I would like to at least discuss it.

Imagine a hypothetical downstream project that creates locks using names that are only known at runtime. Let's say these names are strings with arbitrary characters. Then some calls to NamedLock::create will inevitably return Err(Error::InvalidCharacter).

This could be avoided if the strings were first encoded so that reserved characters ('\0', '/', and '\\') were transformed to non-reserved characters. The exact encoding scheme is a bikeshed I'm not ready to dive into at the moment, but something like percent-encoding or Base32 would fit the bill.

Pros:

  • We could possibly eliminate an Error variant from the public API
  • We can guarantee that lock creation can not fail due to invalid bytes
  • This can be done without additional allocations (depending on the choice of encoding scheme)

Cons:

  • This is a significant breaking change
  • The choice of encoding scheme is arbitrary, and every downstream user pays the penalty of that choice

Questions:

  • Would it make sense to "fix" NamedLock::create, or to make a new constructor method entirely?
  • Would we add an "escape hatch" for the current behavior?
    • If so, should it panic or return an error variant indicating invalid bytes?

ryanavella avatar Aug 28 '22 00:08 ryanavella

I understand the need but I don't like replacing characters or use special encoding. The main reason is because I want it to be very close to the low level representation of the locks, for example a C program can still use the flock system call to synchronize with this crate.

oblique avatar Sep 01 '22 20:09 oblique

What if, instead of a breaking change to the existing API, it were introduced as a new API entirely?

I'm imagining something like this:

/// Create/open a named lock by escaping the provided bytes.
///
/// Unlike [`create`], this will succeed even if the name
/// is empty, or if it contains `\0`, `/`, or `\`.
///
/// The escape sequence used is an implementation detail,
/// though it will produce a valid lock name on each platform,
/// and it is guaranteed that distinct names will not collide.
///
/// To avoid unintended collisions with locks
/// created through the [`create`] constructor,
/// the version 4 UUID `34e5115e-d2a8-4d57-b787-ffd5b3f0ca02`
/// is prepended to the escaped name, followed by a hyphen-minus (`-`). 
///
/// See [`create`] for more information on
/// platform-specific implementation details.
///
/// [`create`]: NamedLock::create
fn create_escaped(name: impl AsRef<[u8]>) -> NamedLock {
    todo!()
}

ryanavella avatar Feb 28 '24 18:02 ryanavella

Escaping is not supported, so in this case do you mean by replacing invalid characters with something else? For example with _?

oblique avatar Feb 28 '24 21:02 oblique

I just had a realization, it should be possible to escape arbitrary strings while addressing your earlier concern about interfacing with C programs.

This should work on both Windows and Unix, and allows empty strings, as well as '\0', '/', and '\\'. Any names that are currently allowed by NamedLock::create would remain unchanged.

fn escape(s: &str) -> String {
    #[cfg(unix)]
    const DELIM: char = '\\';
    
    #[cfg(windows)]
    const DELIM: char = '/';
    
    if s.is_empty() {
        return String::from(DELIM);
    }
    
    let mut escaped = String::with_capacity(s.len());
    for c in s.chars() {
        if matches!(c, '\0' | '\\' | '/') {
            escaped.push(DELIM);
            for b in c.escape_unicode().skip(1) {
                escaped.push(b);
            }
        } else {
            escaped.push(c);
        }
    }
    escaped
}

ryanavella avatar Feb 28 '24 22:02 ryanavella