Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attachment names string treatment is not consistent between different functions #2635

Open
asibahi opened this issue Nov 29, 2023 · 7 comments
Labels
bug Something misbehaves or is not working.

Comments

@asibahi
Copy link

asibahi commented Nov 29, 2023

Serenity version: I am using poise 0.6.0

Rust version (rustc -V): 1.74

Minimal test case if possible:

    // inside a command function
    let attachment_name = format!("{}'s_{}.png", "some", "one"); // mind the apostrophe

    let attachment = serenity::CreateAttachment::bytes(image_data, &attachment_name); // 👈 first use here

    let embed = serenity::CreateEmbed::new()
         // other things here
        .attachment(attachment_name); // 👈 second use here

    let reply = poise::CreateReply::default()
        .attachment(attachment)
        .embed(embed);
}

the apostrophe in attachment_name causes the two functions to give different output somehow. The attachment is no longer in the embed.

attachment out of the embed

Removing the apostrophe resolved that problem.

attachment inside the embed
@kangalio kangalio added the bug Something misbehaves or is not working. label Nov 30, 2023
@ivinjabraham
Copy link
Contributor

I'll try looking into this

@ivinjabraham
Copy link
Contributor

ivinjabraham commented Aug 23, 2024

This is caused by a URL sanitation issue. The url field in CreateEmbed(Embed) shouldn't contain invalid characters. Perhaps serde replaces the invalid characters, but somehow that messes up where the attachment should go.

This is a temporary fix:

    pub fn attachment(self, filename: impl Into<String>) -> Self {
        let mut filename = filename.into();
        filename.retain(|c| c != '\'');
        filename.insert_str(0, "attachment://");
        self.image(filename)
    }

Will try and check if there are better ways to ensure url is valid and also why it causes the attachment to go out of the embed.

UPDATE: Found that when there are invalid characters, the Message struct has attachments, and the embed does not have an image. As expected, when it does not have invalid characters, there are no attachments and the embed as an EmbedImage.

@asibahi
Copy link
Author

asibahi commented Aug 23, 2024

Thanks for looking into this. I know of the work around (just not add invalid characters .. duh!). Yes what you're describing is the same behavior I found.

@jamesbt365
Copy link
Member

jamesbt365 commented Aug 23, 2024

Discord converts the names when they include characters it doesn't like, which means that when supplying the attachment name to the embed, it doesn't point to the real attachment anymore.

This is more of a quirk than a bug, and would require filtering the name by the lib, which is currently out of scope incase discord changes in the future.

@ivinjabraham
Copy link
Contributor

ivinjabraham commented Aug 26, 2024

Maybe we should add a warning in the docs about it? And if there's nothing more to do about this, then perhaps this issue can be closed?

@asibahi
Copy link
Author

asibahi commented Aug 26, 2024

A “fix” would have serenity do Discord’s treatment internally to all strings passed through it.

@ivinjabraham
Copy link
Contributor

Is that not what was discussed here:

would require filtering the name by the lib, which is currently out of scope incase discord changes in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something misbehaves or is not working.
Projects
None yet
Development

No branches or pull requests

4 participants