fix: support legacy mime types for file upload#351
Conversation
Summary of ChangesHello @hanakannzashi, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request expands the list of recognized MIME types to improve compatibility with legacy or alternative content declarations. It specifically adds support for additional XML and YAML MIME types, ensuring that services can correctly process files using these formats. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Code Review - PR #351Reviewed the addition of legacy MIME type support for XML and YAML files. AnalysisChanges:
FindingsNo critical issues found. The implementation is correct: ✅ Encoding consistency: All three new MIME types correctly set
✅ Backward compatibility: This change only expands the allowed MIME types - no breaking changes for existing functionality ✅ Security: No security concerns - the validation logic remains unchanged and properly enforces UTF-8/UTF-16/ASCII encoding for text files ✅ Code quality: Comments clearly document the purpose ("alternative", "deprecated but still used", "RFC 9512 standard") Recommendation✅ APPROVED - Safe to merge. This is a straightforward compatibility improvement with no risks. |
There was a problem hiding this comment.
Code Review
This pull request adds support for legacy MIME types for XML and YAML files, specifically text/xml, text/yaml, and text/x-yaml. The changes are correct and improve compatibility with different clients that might use these alternative MIME types. My suggestion remains to add test coverage for these new types to prevent future regressions.
| ("text/xml", true), // .xml (alternative MIME type) | ||
| ("application/x-bibtex", true), // .bib | ||
| // YAML | ||
| ("application/yaml", true), // .yaml, .yml | ||
| ("application/yaml", true), // .yaml, .yml (RFC 9512 standard) | ||
| ("text/yaml", true), // .yaml, .yml (deprecated but still used) | ||
| ("text/x-yaml", true), // .yaml, .yml (deprecated but still used) |
There was a problem hiding this comment.
This change adds support for legacy MIME types for XML and YAML, which is great for improving compatibility. However, the new types are not covered by tests. To ensure this functionality works as expected and prevent future regressions, please consider adding tests for these new MIME types (text/xml, text/yaml, text/x-yaml) in crates/api/tests/e2e_files.rs. You could add a new test case similar to test_upload_json_file or test_upload_file_success for each new type.
Note
Expands accepted MIME types for uploads to cover common legacy/alternative labels.
text/xml,text/yaml, andtext/x-yamltoALLOWED_MIME_TYPESapplication/yamlas RFC 9512 standard in commentsWritten by Cursor Bugbot for commit 9f8133f. This will update automatically on new commits. Configure here.