Corrupt PNG file (invalid chunk length) causes read error #391
Comments
|
The error is this: |
|
I used the following code to reproduce the issue: const FileType = require('file-type');
const path = require('path');
const fixture = path.join('fixture', 'c0rrupt.png');
(async () => {
try {
const fileType = await FileType.fromFile(fixture);
console.log(fileType);
} catch(err) {
console.error(err);
}
})();The sample file is corrupt, it has a chunk which exceeds the maximum length of 2^31 - 1 bytes, ref. The issue is not related to buffered input. We could return |
Borewit
changed the title
|
Let us know if there are other cases you experience read errors @popey456963. If it is caused by a corrupt or manipulated sample, I will only address it if I believe we can improve our code (which was the case in this issue). Being able to deal with odd data, or validating files, is not goal by itself. |
|
Hey! That was all fixed vastly quicker than I could have hoped for, thank you so much. I agree that detecting the type of files like this is likely not important and shouldn't be added to this library (nor does our service want this functionality). My only desire would be for corrupted files to not crash the module. |
|
There will be cases left where an exception is thrown. I don't think that is the same as a crash. You can simply catch those and move on right? |
|
Do now! But we didn't before, no. We had error handling around the endpoint but it simply killed the enrichments we were doing on it (thus eliminating all enrichments, not just The I think it might be a good idea to add that it can crash to the documentation. I provide a file host service which allows anyone (even people with no account) to upload files to it. I then send those files to |
|
The 4100 sample size does not longer exist since version v14.0.0. The API changed drastically since then, but we desgined the
Reason for breaking with 4100 bytes sample size is that some file types could simply not determined with the first 4100 bytes, at least not for all files. The longer the sample you provide, the greater precision is since then to determine the file type. See also: #319 (comment). Again, throwing an exception is in my opinion not the same as crashing. It is a way to express something unexpected has happened. Crashing in my view prevents further execution of your application. Maybe we should document the possibility of an exception, which is could be thrown underlying dependencies. Suppressing is also a possibility, I am personally not in favor of that unless we have clear reason to assume a specific exception could occur (which we already do). That way we also learn if our code can be improved like in this issue. |
I am not sure what that means, but as long as you don't harm third parties with that, taking privacy and copyright into account, I think it is fine. If it can potentially damage (software) systems, please warn to treat it with extreme caution and zip it with a password. |
When processing some files as buffers this module seems to crash with out of bounds reads. An example file is the following slightly corrupted PNG:
Most browsers will report this as an invalid image and not render it, that is expected. A minimal test case is this, which contains the first 4,100 bytes of that file:
The text was updated successfully, but these errors were encountered: