base: support multiple MIME types per file extension#291969
Conversation
Adds support for multiple MIME types to map to the same file extension by allowing mapExtToMediaMimes values to be string or string[]. This enables image/jpeg and image/jpg to both map to .jpg extension, fixing MCP image responses that provide RFC-compliant image/jpeg MIME type. - Changed MapExtToMediaMimes interface to support string | string[] values - Updated getMediaMime() to return first MIME type from array if present - Updated getExtensionForMimeType() to check both singular and array values - Added .jpe, .jpeg, and .jpg to support both image/jpg and image/jpeg - Added tests for getExtensionForMimeType with multiple MIME types Fixes #291962 (Commit message generated by Copilot)
There was a problem hiding this comment.
Pull request overview
This pull request adds support for multiple MIME types to map to the same file extension, solving an issue where MCP image responses with the RFC-compliant image/jpeg MIME type were being displayed as "file.bin" instead of a proper image file.
Changes:
- Extended
MapExtToMediaMimesinterface to supportstring | string[]values, allowing multiple MIME types per extension - Updated
.jpe,.jpeg, and.jpgextensions to support bothimage/jpgandimage/jpegMIME types - Modified
getMediaMime()to return the first MIME type when an array is present - Enhanced
getExtensionForMimeType()to search within arrays of MIME types - Added comprehensive test coverage for the new functionality
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/vs/base/common/mime.ts | Modified interface and functions to support multiple MIME types per extension; updated JPEG-related extensions to map to both image/jpg and image/jpeg |
| src/vs/base/test/common/mime.test.ts | Added test cases for getExtensionForMimeType with multiple MIME types including JPEG, audio/mpeg, and video/mp4 |
Comments suppressed due to low confidence (1)
src/vs/base/test/common/mime.test.ts:33
- The test suite doesn't include test cases for
getMediaMime()with the new array functionality. Consider adding tests to verify that whengetMediaMime()is called on a file with an extension that has multiple MIME types (e.g., '.jpg'), it returns the first MIME type from the array (e.g., 'image/jpg' based on current implementation).
Example test case:
test('getMediaMime with multiple MIME types', () => {
assert.strictEqual(getMediaMime('file.jpg'), 'image/jpg');
assert.strictEqual(getMediaMime('file.jpeg'), 'image/jpg');
});This would ensure the function behaves as expected when encountering the new array values in mapExtToMediaMimes.
test('getExtensionForMimeType', () => {
// Note: for MIME types with multiple extensions (e.g., image/jpg -> .jpe, .jpeg, .jpg),
// the function returns the first matching extension in iteration order
assert.ok(['.jpe', '.jpeg', '.jpg'].includes(getExtensionForMimeType('image/jpg')!));
// image/jpeg is an alias for image/jpg and should also return a valid extension
assert.ok(['.jpe', '.jpeg', '.jpg'].includes(getExtensionForMimeType('image/jpeg')!));
assert.strictEqual(getExtensionForMimeType('image/png'), '.png');
assert.strictEqual(getExtensionForMimeType('image/gif'), '.gif');
assert.strictEqual(getExtensionForMimeType('image/webp'), '.webp');
assert.ok(['.mp2', '.mp2a', '.mp3', '.mpga', '.m2a', '.m3a'].includes(getExtensionForMimeType('audio/mpeg')!));
assert.ok(['.mp4', '.mp4v', '.mpg4'].includes(getExtensionForMimeType('video/mp4')!));
assert.strictEqual(getExtensionForMimeType('unknown/type'), undefined);
});
| // the function returns the first matching extension in iteration order | ||
| assert.ok(['.jpe', '.jpeg', '.jpg'].includes(getExtensionForMimeType('image/jpg')!)); | ||
| // image/jpeg is an alias for image/jpg and should also return a valid extension |
There was a problem hiding this comment.
The comment states "the function returns the first matching extension in iteration order" but this is not quite accurate. For MIME types that appear in multiple extensions (like 'image/jpeg' which appears in '.jpe', '.jpeg', and '.jpg'), the function returns the extension that is checked first in the for...in loop, which depends on the order the properties were defined in the object. This is deterministic but may not be the "first" extension in the array.
Consider clarifying the comment to explain that for MIME types appearing in multiple extensions, any valid extension may be returned depending on object property iteration order, or consider making the behavior more explicit by documenting which extension will be preferred.
| // the function returns the first matching extension in iteration order | |
| assert.ok(['.jpe', '.jpeg', '.jpg'].includes(getExtensionForMimeType('image/jpg')!)); | |
| // image/jpeg is an alias for image/jpg and should also return a valid extension | |
| // any of the associated extensions may be returned, depending on the underlying | |
| // object property iteration order used by getExtensionForMimeType. | |
| assert.ok(['.jpe', '.jpeg', '.jpg'].includes(getExtensionForMimeType('image/jpg')!)); | |
| // image/jpeg is an alias for image/jpg and should also return one of these extensions |
| '.jpe': ['image/jpg', 'image/jpeg'], | ||
| '.jpeg': ['image/jpg', 'image/jpeg'], | ||
| '.jpg': ['image/jpg', 'image/jpeg'], |
There was a problem hiding this comment.
The MIME types are ordered as ['image/jpg', 'image/jpeg'], but according to RFC 2046, image/jpeg is the official/standard MIME type while image/jpg is a non-standard variant. Consider reordering to ['image/jpeg', 'image/jpg'] to prefer the RFC-compliant MIME type when calling getMediaMime(), which returns the first element of the array.
This would ensure that when getting the MIME type for a .jpg file, the function returns the standard image/jpeg rather than the non-standard image/jpg.
| '.jpe': ['image/jpg', 'image/jpeg'], | |
| '.jpeg': ['image/jpg', 'image/jpeg'], | |
| '.jpg': ['image/jpg', 'image/jpeg'], | |
| '.jpe': ['image/jpeg', 'image/jpg'], | |
| '.jpeg': ['image/jpeg', 'image/jpg'], | |
| '.jpg': ['image/jpeg', 'image/jpg'], |
Adds support for multiple MIME types to map to the same file extension by
allowing mapExtToMediaMimes values to be string or string[]. This enables
image/jpeg and image/jpg to both map to .jpg extension, fixing MCP image
responses that provide RFC-compliant image/jpeg MIME type.
Fixes #291962
(Commit message generated by Copilot)