Skip to content
New issue

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

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

Already on GitHub? # to your account

fix, feat: Add MIME type to HTTP file transfer API #643

Merged
merged 1 commit into from
Apr 2, 2023

Conversation

Mishura4
Copy link
Member

@Mishura4 Mishura4 commented Mar 29, 2023

As I found out the hard way on my bot B-12 while adding a command to add stickers, it turns out the sticker create API expects the file it receives to have a MIME type that corresponds to its data. I spent hours trying to troubleshoot my bot's code, but while digging into DPP code it became clear that the application/octet-stream MIME type it sent by default was what was causing the issue, as changing it to image/png allowed it to be accepted by the API.

This pull request adds MIME types to APIs that deal with raw file uploading through the REST API. This includes data members in dpp::http_request, in dpp::message, and function signature changes. MIME type defaults to application/octet-stream if unspecified by the user.
This effectively :

  • Fixes the creation of new stickers through the REST API
  • Allows attachments to have their MIME type set, which allows them to be displayed in corresponding ways in the Discord app. For example a file with text/plain as type will have its text shown in the app, a file with application/octet-stream will be available for download, and so on.

Changes of note :

⚠️ This breaks ABI, as function signatures were changed around and data members were added. API is preserved through the new parameters being defaulted.

Tested on my own bot, on MSVC, clang-cl, Linux-GCC. Unit tests also pass (I also added the new MIME type parameters to it).

BREAKING CHANGES: ABI is broken due to function signature changes
(API is preserved through the use of default parameters)
@netlify
Copy link

netlify bot commented Mar 29, 2023

Deploy Preview for dpp-dev ready!

Name Link
🔨 Latest commit 4ddadd5
🔍 Latest deploy log https://app.netlify.com/sites/dpp-dev/deploys/64247ea2f123d200089928f4
😎 Deploy Preview https://deploy-preview-643--dpp-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@Mishura4 Mishura4 changed the title Add MIME type to HTTP file transfer API fix, feat: Add MIME type to HTTP file transfer API Mar 29, 2023
@Commandserver
Copy link
Member

What sticker API do you mean? This one?: https://discord.com/developers/docs/resources/sticker#create-guild-sticker

@Mishura4
Copy link
Member Author

Mishura4 commented Mar 29, 2023

This one yes, and subsequently dpp::cluster::guild_sticker_create.

@Commandserver
Copy link
Member

Commandserver commented Mar 29, 2023

What was your code that didn't work? And what was the error message?

@Mishura4
Copy link
Member Author

Mishura4 commented Mar 29, 2023

Starting here, guild_sticker_create gets called by sticker_add below, defined here (the executor is basically a wrapper for a mutex/condvar and std::invoke).

The error message was simply "Invalid Asset".

@Mishura4
Copy link
Member Author

Mishura4 commented Mar 29, 2023

I tested extensively, the content I retrieved through the sticker URL was valid, the resized image as well, I kept writing my data to a PNG file and it was working fine. Even uploading those processed PNG files through the discord app worked, while my bot didn't, even after re-reading the PNG files and uploading that data. Changing the MIME type is the only thing that worked.

Discord was even accepting my files as an attachment to messages, which were displaying fine as well. It was only for stickers that it rejected it.

@Commandserver
Copy link
Member

I'm just curious because i can't find any mime type in discord's documentation

@Mishura4
Copy link
Member Author

Mishura4 commented Mar 29, 2023

Yeah 😆 I was really confused as well, my data was working fine, I thought I had all the requirements the API docs specified, except it kept rejecting my sticker through the bot. This could be a suggestion to send them for sure.

@braindigitalis braindigitalis merged commit 58cde9a into brainboxdotcc:dev Apr 2, 2023
@Mishura4 Mishura4 deleted the http-multipart-fix branch April 8, 2023 12:26
@Mishura4 Mishura4 restored the http-multipart-fix branch April 8, 2023 12:26
@Mishura4 Mishura4 deleted the http-multipart-fix branch April 8, 2023 12:27
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants