if statement does not seem to ever evaluate false

Question

Goal: User uploads an image, a validator checks to make sure it is an image file the user uploaded, returns message if it is not an image, doesn't if it is.

Issue: When Upload button is clicked, regardless of whether the uploaded file is an image or not an image, the validator message is always returned.

Area of Focus: In the Validator class, the line System.out.println(partValueContentType); has written the content type to the console, ex. image/jpeg, but when it is tested in the if statement it does not seem to evaluate the content type at all.

        String partValueContentType = part.getContentType();
        System.out.println(partValueContentType);

        if (!partValueContentType.equals("image/jpeg")
                || !partValueContentType.equals("image/jpg")
                || !partValueContentType.equals("image/gif")
                || !partValueContentType.equals("image/png"))
        {
            FacesMessage msg = new FacesMessage("File is not an image.",
                    "Acceptable image types (jpeg, jpg, gif, png)");
            msg.setSeverity(FacesMessage.SEVERITY_ERROR);
            throw new ValidatorException(msg);
        }

How is this caused and how can I solve it?

Take a good cup of coffee and look at it again. Replace each individual condition by "true" or "false". Does it in the end look all right? — BalusC, May 19 '15 at 04:53
For debugging purposes, include the actual `partValueContentType` value. i.e. `FacesMessage msg = new FacesMessage("File is not an image.", "Acceptable image types (jpeg, jpg, gif, png) but got " + partValueContentType);` — Dilum Ranatunga, May 19 '15 at 04:56
Tell yourself aloud what the clauses in the if-statement do. When is it true? — , May 19 '15 at 04:56
@BalusC - LOL you are right. That structure looked odd but I didn't dig deeper... — Dilum Ranatunga, May 19 '15 at 04:58
On a separate but related note, you can avoid this sort of problem by having a `SUPPORTED_MIME_TYPES` set or something. `if (!SUPPORTED_MIME_TYPES.contains(partValueContentType) { ... }` — Dilum Ranatunga, May 19 '15 at 04:59
all files will be either a non-jpeg, a non-jpg, a non-gif, or a non-png, and possibly all four at once. — samgak, May 19 '15 at 05:10

score 1 · Accepted Answer · answered May 19 '15 at 05:51

Your if statement is a little off:

String partValueContentType = part.getContentType();
System.out.println(partValueContentType);

if (!(partValueContentType.equals("image/jpeg")
        || partValueContentType.equals("image/jpg")
        || partValueContentType.equals("image/gif")
        || partValueContentType.equals("image/png")))
{
    FacesMessage msg = new FacesMessage("File is not an image.",
            "Acceptable image types (jpeg, jpg, gif, png)");
    msg.setSeverity(FacesMessage.SEVERITY_ERROR);
    throw new ValidatorException(msg);
}

In terms of validation, you might want to check the file itself to make sure it's truly a picture (it's not a .zip hiding as a .jpeg) and maybe enforcing file size limits...

Or use a HashSet:

String partValueContentType = part.getContentType();
System.out.println(partValueContentType);
Set<String> acceptedMimeTypes = new HashSet<>(Arrays.asList("image/jpeg", "image/jpg", "image/gif", "image/png"));

if (!acceptedMimeTypes.contains(partValueContentType))
{
    FacesMessage msg = new FacesMessage("File is not an image.",
            "Acceptable image types " + Arrays.toString(acceptedMimeTypes.toArray()));
    msg.setSeverity(FacesMessage.SEVERITY_ERROR);
    throw new ValidatorException(msg);
}

Or just check if it starts with `image/`. See also a.o. http://stackoverflow.com/questions/4169713/how-to-check-a-uploaded-file-whether-it-is-a-image-or-other-file — BalusC, May 19 '15 at 05:59
This is true, but this answer was more towards the question at hand - OP should follow the link you provided for a better implementation. — justderb, May 19 '15 at 06:04
Hey @justderb. I like the HashSet example you provided and will utilize it once I get to the laptop I'm developing on. I agree that the file should be checked to make sure it really is an image-type file. It is the reason I am using this code. Is the getContentType() method not an efficient way to do this? If not, then what would you suggest? — Weston Sazehn, May 19 '15 at 18:01

if statement does not seem to ever evaluate false

1 Answers1