Mimetype.getMimeType() incorrectly returns null

[garretwilson]

The documentation for software.amazon.awssdk.core.internal.util.getMimetype(Path path) says that the method will return application/octet-stream if the MIME type cannot be determined from the filename:

    /**
     * Determines the mimetype of a file by looking up the file's extension in an internal listing
     * to find the corresponding mime type. If the file has no extension, or the extension is not
     * available in the listing contained in this class, the default mimetype
     * <code>application/octet-stream</code> is returned.
     * <p>
     * A file extension is one or more characters that occur after the last period (.) in the file's name.
     * If a file has no extension,
     * Guesses the mimetype of file data based on the file's extension.
     *
     * @param path the file whose extension may match a known mimetype.
     * @return the file's mimetype based on its extension, or a default value of
     * <code>application/octet-stream</code> if a mime type value cannot be found.
     */
    public String getMimetype(Path path) {

Most of the work is performed by getMimetype(String fileName), which does return application/octet-stream if the the MIME type could not be determined from the given:

    public String getMimetype(Path path) {
        Validate.notNull(path, "path");
        Path file = path.getFileName();

        if (file != null) {
            return getMimetype(file.toString());
        }
        return null;
    }

The problem is if the original Path doesn't have a filename at all. In that case you return null, breaking your API contract.

It is highly unlikely that a given Path would not have a filename, and indeed this is probably why no one has ran into this bug before. Nevertheless the fact that it is rare doesn't change the fact that it breaks the API. The API contract says nothing of returning null, and indeed the code that uses this, RequestBody.fromFile(Path path), would throw a NullPointerException if this method did return null.

You need to decide what you intend to do if the given path doesn't have a filename. If you don't intend to support paths without filenames, you should state that in the API contract and deal with it:

    public String getMimetype(Path path) {
        Validate.notNull(path, "path");
        Path file = path.getFileName();

        if (file != null) {
            return getMimetype(file.toString());
        }
        throw new IllegalArgumentException("Path "+ path + " has no filename.");
    }

Or if you merely intend for a missing filename to also result in a application/octet-stream (which better matches what the current API implies), you could do that:

    public String getMimetype(Path path) {
        Validate.notNull(path, "path");
        Path file = path.getFileName();

        if (file != null) {
            return getMimetype(file.toString());
        }
        return MIMETYPE_OCTET_STREAM;
    }

Whatever you decide, the current return of null is incorrect; it breaks the API contract and would result in a NullPointerException in the caller in this admittedly unlikely scenario.

In addition your API docs likely have typos. Look at these lines:

     * A file extension is one or more characters that occur after the last period (.) in the file's name.
     * If a file has no extension,
     * Guesses the mimetype of file data based on the file's extension.

That makes no sense. It looks like someone never finished their train of thought. Not only did this get copied to two other locations, it later got erroneously combined into a single sentence:

     * A file extension is one or more characters that occur after the last
     * period (.) in the file's name. If a file has no extension, Guesses the
     * mimetype of file data based on the file's extension.

So all these Mimetype methods need to be revisited and reviewed for correctness, preferably with unit tests. (I feel your pain; I also created many file, path, and extension manipulating methods years ago, and only later I found many errors and inconsistencies and realized I should have had more unit tests.)

[zoewangg]

Hi @garretwilson, thank you for reporting the issue! I agree thatreturning null in that case is indeed a bug that we should fix.

We will also revisit the javadocs and add more unite tests.