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

Include file name in response, and support "Accept" header that contain charset property #415

Closed
wants to merge 2 commits into from

Conversation

johan-perso
Copy link

This PR has been made to allow clients to use the API with an Accept header defined to application/json; charset=utf-8, which is especially useful for Flutter apps, as they automatically set it instead of the regular application/json.

Additionally, when requesting a download, the API will now return the name of the file. This enhancement aims to improve the developer experience when programmatically accessing Cobalt.

@dumbmoron
Copy link
Member

dumbmoron commented Apr 1, 2024

This PR has been made to allow clients to use the API with an Accept header defined to application/json; charset=utf-8, which is especially useful for Flutter apps, as they automatically set it instead of the regular application/json.

this is a good idea, but if you're going to update this, the check should be (or rather should have been) probably done using req.is for content-type and req.accepts for accept

@dumbmoron
Copy link
Member

dumbmoron commented Apr 1, 2024

Additionally, when requesting a download, the API will now return the name of the file. This enhancement aims to improve the developer experience when programmatically accessing Cobalt.

i'm not entirely opposed to it, but I think it would be a better option to simply get the filename from the stream request as an API consumer, since you are usually going to download the stream anyways - if you don't want the data you could probably make a HEAD request (something to check here, if you'd like to contribute a fix, is whether this spawns a ffmpeg process when it's not necessary - i believe it does, but it doesn't need to.)

@johan-perso
Copy link
Author

this is a good idea, but if you're going to update this, the check should be (or rather should have been) probably done using req.is for content-type and req.accepts for accept

It seems that req.accepts also refuses Accept header that contains charset and only accepts the application/json value

@johan-perso
Copy link
Author

I think it would be a better option to simply get the filename from the stream request as an API consumer

This would work for stream requests but not for medias that aren't downloaded through the Cobalt backend (such as Twitter, they don't return file names when fetching the media url).

johan-perso added a commit to hoststend/stend-mobile that referenced this pull request Apr 17, 2024
- L'instance officielle de Cobalt est utilisée, cela nécessite une requête supplémentaire et ralentit d'environ 1sec le téléchargement (imputnet/cobalt#415 aurait pu améliorer ça 🥲)
- La progression du téléchargement est affichée même si la taille totale est manquante (sur certains services)
- La requête pour obtenir les infos d'un potentiel client Stend vérifie que la réponse ne fait pas plus de 8 Mo
@dumbmoron
Copy link
Member

closing this since both parts were re-done are are now implemented

@dumbmoron dumbmoron closed this Sep 18, 2024
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.

2 participants