samuelneff / MimeTypeMap

Provides a huge dictionary of file extensions to mime types.
MIT License
622 stars 201 forks source link

Recent commit introduced breaking changes and ambigous behavior #118

Closed lajjne closed 3 years ago

lajjne commented 3 years ago

Commit https://github.com/samuelneff/MimeTypeMap/commit/ace145c93cf87d2216617116567ddee551fde22f added some breaking changes and also introduced some ambiguous behavior.

Breaking

GetMimeType("") previously returned "application/octet-stream" but now throws ArgumentNullException.

Ambiguous

GetMimeType used to take a file extension as input but now takes a file name or file extension. This can lead to some cases where it is not clear what the expected output should be.

Consider for example the string "gif". This could be a valid filename for a file without file extension and should in that case return "application/octet-stream". But it could also be interpreted as the file extension ".gif". To remove this ambiguity I suggest reverting to the previous behavior of expecting a file extension only.

Another option could be to change GetMimeType to expect a path string instead and then use System.IO.Path.GetExtension(path) to get the file extension before looking in the internal dictionary.

public static string GetMimeType(string path) {
    if (path == null) {
        throw new ArgumentNullException(nameof(path));
    }
    if (path != string.Empty && Path.GetFileName(path) == string.Empty) {
        throw new ArgumentException($"{path} is not a file path", nameof(path));
    }
    var ext = Path.GetExtension(path);
    return MimeTypeMap.TryGetMimeType(ext, out var result) ? result : DefaultMimeType;
}        

The following examples would then all be valid input:

samuelneff commented 3 years ago

I've restored the previous behavior of returning application/octet-stream for an empty string and no longer throwing ArgumentNullException in this case.

Other changes would equally be breaking changes. Particularly getting mime type for an extension without a dot IMO should return the proper mime type. Changing that would be a bigger breaking change.

Thanks for bringing this to my attention.