Repository navigation
Change the method of get file extension. #475
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
base: main
Are you sure you want to change the base?
Changes from all commits
4e03901
74e71ca
b6105f7
6028618
4ec2651
b8ebe93
1d73d2e
64ff206
1a55b01
b0ed040
2132fb4
3a498ba
a7dac2a
780d662
e9eca3a
fe77da7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,32 @@ import 'package:flutter/widgets.dart'; | |
| import 'package:flutter_cache_manager/flutter_cache_manager.dart'; | ||
|
|
||
| const supportedFileNames = ['jpg', 'jpeg', 'png', 'tga', 'cur', 'ico']; | ||
|
|
||
| /// Signatures (magic numbers) of the image formats that can be recognized by | ||
| /// the first bytes of the file, instead of by its extension. | ||
| /// | ||
| /// Gif is deliberately missing: the resizer re-encodes to png, which would turn | ||
| /// an animation into a single frame. | ||
| /// | ||
| /// The ico and cur values overlap with tga, which starts with the same four | ||
| /// bytes when it has no id field and no colour map (byte 2 is then the image | ||
| /// type, 1 for a colour mapped and 2 for a true colour image). Telling them | ||
| /// apart takes more than four bytes, but it does not matter here: all of them | ||
| /// are resizable, and only the answer to "is this an image" is used. | ||
| const _imageSignatures = <List<int>>[ | ||
| [0xFF, 0xD8], // jpeg | ||
| [0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A], // png | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An animated PNG (APNG) matches this signature too, so it gets resized to a single static frame. That goes against the doc on |
||
| [0x42, 0x4D], // bmp | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The two-byte signatures ( |
||
| [0x00, 0x00, 0x01, 0x00], // ico | ||
| [0x00, 0x00, 0x02, 0x00], // cur | ||
| ]; | ||
|
|
||
| /// Extensions of the formats that have no usable signature at the start of the | ||
| /// file, so the only way to recognize them is by their extension. Tga only | ||
| /// stores a signature in its footer, and wbmp starts with a two byte header | ||
| /// (`00 00`) that is too generic to match on. | ||
| const _extensionOnlyFormats = ['tga', 'wbmp']; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Flutter's image codecs can't decode TGA, so a |
||
|
|
||
| mixin ImageCacheManager on BaseCacheManager { | ||
| /// Returns a resized image file to fit within maxHeight and maxWidth. It | ||
| /// tries to keep the aspect ratio. It stores the resized image by adding | ||
|
|
@@ -65,6 +91,33 @@ mixin ImageCacheManager on BaseCacheManager { | |
| _runningResizes.remove(resizedKey); | ||
| } | ||
|
|
||
| /// Whether [file] holds an image that can be decoded and resized, based on | ||
| /// the content of the file rather than on the extension of its url. A | ||
| /// response can be served without a file extension, or with an extension | ||
| /// that does not match the actual content, for example when it is served | ||
| /// with an incorrect `Content-Type` header. | ||
| /// | ||
| /// Images that would lose something when they are re-encoded as png, such as | ||
| /// an animated gif or an animated webp, are not considered an image here, so | ||
| /// they are passed through untouched. | ||
| Future<bool> _isImage(File file) async { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than keeping a table of magic numbers plus a webp-only animation check, could we let the engine decide? For example, |
||
| if (!await file.exists()) { | ||
| return false; | ||
| } | ||
|
|
||
| final header = await _readHeader(file); | ||
| if (_imageSignatures.any((signature) => _matches(header, signature))) { | ||
| return true; | ||
| } | ||
| // WebP is a RIFF container: "RIFF" + 4 bytes of length + "WEBP". | ||
| if (_matches(header, 'RIFF'.codeUnits) && | ||
| _matches(header, 'WEBP'.codeUnits, 8)) { | ||
| return !_isAnimatedWebp(header); | ||
| } | ||
| final extension = file.path.split('.').last.toLowerCase(); | ||
| return _extensionOnlyFormats.contains(extension); | ||
| } | ||
|
|
||
| final Map<String, Stream<FileResponse>> _runningResizes = {}; | ||
|
|
||
| Future<FileInfo> _resizeImageFile( | ||
|
|
@@ -73,9 +126,7 @@ mixin ImageCacheManager on BaseCacheManager { | |
| int? maxWidth, | ||
| int? maxHeight, | ||
| ) async { | ||
| final originalFileName = originalFile.file.path; | ||
| final fileExtension = originalFileName.split('.').last; | ||
| if (!supportedFileNames.contains(fileExtension)) { | ||
| if (!await _isImage(originalFile.file)) { | ||
| return originalFile; | ||
| } | ||
|
|
||
|
|
@@ -111,7 +162,11 @@ mixin ImageCacheManager on BaseCacheManager { | |
| resizedFile, | ||
| key: key, | ||
| maxAge: maxAge, | ||
| fileExtension: fileExtension, | ||
| // The resized image is always encoded as png. The extension of the | ||
| // original file cannot be used: it is derived from the url, so it can | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The stored file's extension doesn't come from the url. |
||
| // be missing, or be the last segment of the path when the url has no | ||
| // extension at all. | ||
| fileExtension: 'png', | ||
| ); | ||
|
|
||
| return FileInfo( | ||
|
|
@@ -147,6 +202,44 @@ mixin ImageCacheManager on BaseCacheManager { | |
| } | ||
| } | ||
|
|
||
| /// Reads the first bytes of [file], up to [length] bytes, which is enough to | ||
| /// hold the longest signature that is checked, including the webp flags. | ||
| Future<List<int>> _readHeader(File file, {int length = 30}) async { | ||
| final raf = await file.open(); | ||
| try { | ||
| return await raf.read(length); | ||
| } finally { | ||
| await raf.close(); | ||
| } | ||
| } | ||
|
|
||
| /// Whether [bytes] holds [signature] at [offset]. | ||
| bool _matches(List<int> bytes, List<int> signature, [int offset = 0]) { | ||
| if (bytes.length < offset + signature.length) { | ||
| return false; | ||
| } | ||
| for (var i = 0; i < signature.length; i++) { | ||
| if (bytes[offset + i] != signature[i]) { | ||
| return false; | ||
| } | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| /// Whether [header] belongs to a webp that contains an animation. | ||
| /// | ||
| /// An extended webp starts with a `VP8X` chunk, whose flags byte marks the file | ||
| /// as animated. Resizing re-encodes the image to png, which keeps a single | ||
| /// frame, so an animated webp is left as it is, the same way gif is. | ||
| bool _isAnimatedWebp(List<int> header) { | ||
| // "RIFF" + size + "WEBP" + "VP8X" + chunk size, the flags follow at 20. | ||
| const flagsOffset = 20; | ||
| const animationFlag = 0x02; | ||
| return header.length > flagsOffset && | ||
| _matches(header, 'VP8X'.codeUnits, 12) && | ||
| (header[flagsOffset] & animationFlag) != 0; | ||
| } | ||
|
|
||
| Future<ui.Image> _decodeImage( | ||
| File file, { | ||
| int? width, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import 'dart:convert'; | ||
| import 'dart:io'; | ||
| import 'dart:typed_data'; | ||
| import 'dart:ui'; | ||
|
|
@@ -127,6 +128,57 @@ void main() { | |
| expect(progress, isNotEmpty); | ||
| }); | ||
| }); | ||
|
|
||
| group('Test image detection', () { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add tests for the new formats too: resizing a static webp and a bmp, checking that the resized file is stored with a |
||
| test('Image without file extension should be resized', () async { | ||
| const urlWithoutExtension = 'baseflow.com/avatar/123456'; | ||
| var config = createTestConfig(); | ||
| await config.returnsFile('avatar', data: await getExampleImage()); | ||
| config.returnsCacheObject(urlWithoutExtension, 'avatar', validTill); | ||
|
|
||
| var cacheManager = TestCacheManager(config); | ||
| var result = | ||
| await cacheManager | ||
| .getImageFile(urlWithoutExtension, maxWidth: 100) | ||
| .last | ||
| as FileInfo; | ||
|
|
||
| var image = await result.file.readAsBytes(); | ||
| await verifySize(image, 100, 100); | ||
| }); | ||
|
|
||
| test('Non image should not be resized', () async { | ||
| const urlWithImageExtension = 'baseflow.com/image.png'; | ||
| var payload = utf8.encode('{"message":"not an image"}'); | ||
| var config = createTestConfig(); | ||
| await config.returnsFile('image.png', data: payload); | ||
| config.returnsCacheObject(urlWithImageExtension, 'image.png', validTill); | ||
|
|
||
| var cacheManager = TestCacheManager(config); | ||
| var result = | ||
| await cacheManager | ||
| .getImageFile(urlWithImageExtension, maxWidth: 100) | ||
| .last | ||
| as FileInfo; | ||
|
|
||
| expect(await result.file.readAsBytes(), payload); | ||
| }); | ||
|
|
||
| test('Animated webp should not be resized', () async { | ||
| const urlOfAnimatedWebp = 'baseflow.com/animation.webp'; | ||
| var payload = animatedWebpHeader(); | ||
| var config = createTestConfig(); | ||
| await config.returnsFile('animation.webp', data: payload); | ||
| config.returnsCacheObject(urlOfAnimatedWebp, 'animation.webp', validTill); | ||
|
|
||
| var cacheManager = TestCacheManager(config); | ||
| var result = | ||
| await cacheManager.getImageFile(urlOfAnimatedWebp, maxWidth: 100).last | ||
| as FileInfo; | ||
|
|
||
| expect(await result.file.readAsBytes(), payload); | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| Future<TestCacheManager> setupCacheManager() async { | ||
|
|
@@ -157,3 +209,17 @@ Future verifySize( | |
| Future<Uint8List> getExampleImage() { | ||
| return File('test/images/image-120.png').readAsBytes(); | ||
| } | ||
|
|
||
| /// The header of an animated webp: a `VP8X` chunk with the animation flag set. | ||
| /// Only the header is needed, the file is never decoded because it is not | ||
| /// resized. | ||
| Uint8List animatedWebpHeader() { | ||
| return Uint8List.fromList(<int>[ | ||
| ...'RIFF'.codeUnits, 0x1A, 0x00, 0x00, 0x00, // | ||
| ...'WEBP'.codeUnits, // | ||
| ...'VP8X'.codeUnits, 0x0A, 0x00, 0x00, 0x00, // | ||
| 0x02, // flags: animation | ||
| 0x00, 0x00, 0x00, // canvas width - 1 | ||
| 0x00, 0x00, 0x00, // canvas height - 1 | ||
| ]); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
supportedFileNamesis exported publicly, but nothing uses it anymore and it no longer reflects what gets resized (no webp or bmp). Removing it would be breaking, so maybe mark it@Deprecatedwith a note that detection is now content-based?