Quick Validation Help with C#

Hi please take a look at this code:

// if the image url doesn't contain a valid image
            if (!ArticleToCreate.image.Contains(".jpg")
                || !ArticleToCreate.image.Contains(".jpeg")
                || !ArticleToCreate.image.Contains(".png")
                || !ArticleToCreate.image.Contains(".gif")
                || !ArticleToCreate.image.Contains(".bmp"))
            {
                ModelState.AddModelError("Image", "Please enter a valid URL.");
            }

Why wouldn't this work? The plan is to basically say if the value of image doesn'开发者_开发百科t have one of those extensions then error. It looks fine to me but doesn't work. If I have 'MyImage.png' it should allow it but not 'MyImage.hhh' but it doesn't allow anything. Why? Thanks

You are using OR conditions in your statement, therefore if any one of them evaluates to true, the error will show.

You could modify it to be something like this as you only want the error to be added if it doesnt contain ANY of the image extensions.

// if the image url doesn't contain a valid image
if (!ArticleToCreate.image.Contains(".jpg")
     && !ArticleToCreate.image.Contains(".jpeg")
     && !ArticleToCreate.image.Contains(".png")
     && !ArticleToCreate.image.Contains(".gif")
     && !ArticleToCreate.image.Contains(".bmp"))
{
    ModelState.AddModelError("Image", "Please enter a valid URL.");
}

Now, there are a few things that could cause you problems here just to make sure you are aware.

1. Case Sensitivity (.JPG, etc)
2. Files with multiple extensions (MyFile.jpg.txt)

You might be ok with what you have, but just wanted to make note.

It is because your Image path should have all the extensions as per your logic. change it to:

if (
            !ArticleToCreate.image.Contains(".jpg") &&
            !ArticleToCreate.image.Contains(".jpeg") &&
            !ArticleToCreate.image.Contains(".png") &&
            !ArticleToCreate.image.Contains(".gif") &&
            !ArticleToCreate.image.Contains(".bmp")
    )
{                 
    ModelState.AddModelError("Image", "Please enter a valid URL.");             
} 

To have least number of extensions checks use this version:

if (
        !(
            ArticleToCreate.image.Contains(".jpg") ||
            ArticleToCreate.image.Contains(".jpeg") ||
            ArticleToCreate.image.Contains(".png") ||
            ArticleToCreate.image.Contains(".gif") ||
            ArticleToCreate.image.Contains(".bmp")
        )
    )
{                 
    ModelState.AddModelError("Image", "Please enter a valid URL.");             
} 

To get the extension use System.IO.Path.GetExtension like this:

string filename ArticleToCreate.image;
string extension = Path.GetExtension(filename)

Then change your logic to something a little more readable:

if (!(extension == ".jpg" 
                || extension == ".jpeg"
                || extension == ".png"
                || extension == ".gif"
                || extension == ".bmp"))

That will trigger if it's anything other than those 5 extensions. Another alternative is to have a map (or array and use linq) of the extensions. A simple search in the map for that extension will tell you if it's valid or not.

string [] extensions = new string [] { ".jpeg", ".png", ".gif", ".bmp", ".jpg" };
string filename ArticleToCreate.image;
string extension = Path.GetExtension(filename)

if( !extensions.ToList().Contains(extension) )  // then handle the invalid extension case

Beside of what Mitchel said, your code does not enforce that the file name ends with ".jpg", but only that it contains ".jpg" somewhere in the string. I would use a regular expression .+\.jpg$|.+\.jpeg$ ... and match it.

In your case, this would look like:

if (!Regex.IsMatch(ArticleToCreate.image, @".+\.(jpg|jpeg|bmp|gif|png)$", RegexOptions.IgnoreCase)) {
    ModelState.AddModelError("Image", "Please enter a valid URL.");            
}