coobird / thumbnailator

Thumbnailator - a thumbnail generation library for Java
MIT License
5.08k stars 780 forks source link

By default, do not change file extensions when it doesn't match output format's #186

Open coobird opened 2 years ago

coobird commented 2 years ago

Expected behavior

When giving a File (or Files) to the toFile (or asFile and their plural forms), the thumbnail file name should be exactly the files that are provided.

Actual behavior

If the File's extension does not match the output format name, then the "correct" extension is appended.

Steps to reproduce the behavior

Thumbnails.of("/path/to/image")
    .size(100, 100)
    .toFile("/path/to/thumbnail");

When a JPEG file is the input, then the output file name will become /path/to/thumbnail.jpg rather than /path/to/thumbnail.

Environment

Workaround

The behavior is only triggered when given a filename through a File (or String.) By using the toOutputStream (or asOutputStream and their plural forms), the automatically renaming behavior will not be triggered.

For example,

Thumbnails.of("/path/to/image")
    .size(100, 100)
    .toOutputStream(new FileOutputStream("/path/to/thumbnail"));
coobird commented 2 years ago

This is intended behavior in Thumbnailator right now.

However, as seen in #150 and #66, this can be unexpected and undesirable behavior. This behavior should be configurable and by default, it's better to not make changes to the output file name automatically. (To reduce the principle of least surprise.)

This behavior change would be a fairly large breaking change. Therefore, it would have to be implemented in the next major version like 0.5.0.

devxzero commented 1 year ago

It took me some time to figure out that Thumbnailator was not using the file that I specificied in toFile(File), but was using a changed version of it under certain conditions.

(I had some files to store in the operating system tmp directory for further processing, created using Files.createTempFile(). and then use that file as toFile output for Thumbnailator, and then move it to the final destination. But the files stayed empty after the move. I had to dive in the Thumbnailator source code to figure out that FileImageSink was modifying the filename that I specified.)

Even though it is intended behavior, it is unexpected and undocumented behavior in the context of a client developer. (Although it is common for desktop applications to add extensions when a file is specified without an extension in a SaveAsDialog, it is uncommon for API's to alter the file specified as a function argument by a client developer.) From that perspective, it looks more like a bug than intended behavior. Also, the doc says:

Create a thumbnail and writes it to a File. [...] Params: outFile – The file to which the thumbnail is to be written to.

The outFile doc should be more like: The file to which the thumbnail is to be written to if and only if the file extension matches the outputFormat, otherwise the outputFormat extension is added to the filename.

A solution could be to add a method like toFile(File file, boolean forceFilename), or to add it as a builder method like outputForceFilename(boolean), but the latter could still cause unexpected behavior for those who miss the existence of the builder method.