Skip to content

Conversation

@maurosilber
Copy link
Contributor

@maurosilber maurosilber commented Dec 17, 2025

Description

If the target prefix is larger than the placeholder prefix, it would overwrite into other regions of the binary truncate the prefix. It would likely lead to broken binaries: https://discord.com/channels/1082332781146800168/1212673297347518464/1450859607072182374

Possible issue outside rattler

Being a public function, this function might be being used outside rattler in some valid ways that would now return an error. For example, relocating an environment:

  1. On installation, the new prefix is not longer than the old prefix (the build prefix placeholder) -> ✅
  2. On relocation, the new prefix is longer than the old prefix (but not longer that the build prefix placeholder) -> returns an error although it would be correct.

Alternatives

It might be more "rustic" to encode this restriction into a type such as:

struct PlaceholderChange {
    old_placeholder: &str,
    new_placeholder: &str,
}

pub fn copy_and_replace_cstring_placeholder(
    mut source_bytes: &[u8],
    mut destination: impl Write,
    placeholder_chance: PlaceholderChange,
) -> Result<(), std::io::Error>

instead of accepting any placeholders:

pub fn copy_and_replace_cstring_placeholder(
    mut source_bytes: &[u8],
    mut destination: impl Write,
    prefix_placeholder: &str,
    target_prefix: &str,
) -> Result<(), std::io::Error>

but would be a breaking change in a public function.

Also, it would not solve the relocation use case (for which I have an idea that would imply changing the info/has_prefix file, which would be an even bigger breaking change in the conda ecosystem not work after thinking about it some more).

If the target prefix is larger than the placeholder prefix,
it would overwrite into other regions of the binary.
@wolfv wolfv merged commit 01d2cd6 into conda:main Dec 18, 2025
17 of 18 checks passed
@wolfv
Copy link
Contributor

wolfv commented Dec 18, 2025

Thank you @maurosilber!

@github-actions github-actions bot mentioned this pull request Dec 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants