Question

I built an edit page to update data and it works fine if the correct ID is passed in but when an invalid ID is passed in I get a null reference exception. I know this is due to the fact the LINQ query does not find any valid data from the database but I am not sure how to deal with this unless I add a bunch of IF statements in my view to check for null every time I reference the model. Here is the code I currently have for the controller.

    public ActionResult EditSection(Int16 id = -1)
    {
        Section section = db.Sections.Find(id);
        SectionAddEditVM model = new SectionAddEditVM { Section = section };

        if (section != null)
        {
            if (section.Type == "Collection")
            {
                RedirectToAction("Collection", new { id = id });
            }

            model.SelectedType = section.Type;
            return View(model);
        }

        ModelState.AddModelError("Section ID", "Invalid Section ID");
        return View(model);
    }

View:

@model SectionAddEditVM

@{
    ViewBag.Title = "Edit " + Model.Section.Title + " Information";
}

<h2>
    Edit @Model.Section.Title Information
</h2>

@using (Html.BeginForm(null, null, FormMethod.Post, new { enctype = "multipart/form-data" }))
{
    @Html.AntiForgeryToken();
    @Html.ValidationSummary(false)

    <p>
        @Html.HiddenFor(m => m.Section.ID)
        <label for="Title">Seciton Title:</label> @Html.EditorFor(m => m.Section.Title)
        <br />
        <label for="RouteName">Section Route:</label> @Html.EditorFor(m => m.Section.RouteName)
        <br />
        <label for="Type">Section Type:</label> @Html.DropDownListFor(m => m.Section.Type, new SelectList(Model.Type, "Value", "Text"))
        <br />
        @Html.HiddenFor(m => m.Section.LogoFileID)
        <label for="LogoFile">Logo Image:</label> <input id="LogoFile" name="LogoFile" type="file" />
        <br />
        <label for="Synopsis">Synopsis:</label> @Html.EditorFor(m => m.Section.Synopsis)
        <br />
        <input type="submit" value="Edit Information" />
    </p>
}
Was it helpful?

Solution 2

The solution was to add an ELSE clause and initialize a new, blank, model.

    public ActionResult EditSection(Int16 id = -1)
    {
        Section section = db.Sections.Find(id);

        if (section != null)
        {
            if (section.Type == "Collection")
            {
                RedirectToAction("Collection", new { id = id });
            }

            SectionAddEditVM model = new SectionAddEditVM { Section = section };
            model.SelectedType = section.Type;
            return View(model);
        }
        else
        {
            section = new Section();
            SectionAddEditVM model = new SectionAddEditVM { Section = section };

            ModelState.AddModelError("Section ID", "Invalid Section ID");
            return View(model);
        }
    }

OTHER TIPS

In your controller you're already checking if section is null. So, in case it IS null just return a different view that says: "No section found" or something. Additionally (as suggested here by @Aron) you could return 404 status code with this view, so controller would look something like this:

// find section based on ID
// ... actual code ...
if (section != null)
{
    // process data and assign to model
    return View(model);
}
else
{
    Response.StatusCode = (int) System.Net.HttpStatusCode.NotFound;
    return View("NoSectionFound");
}

Note that by returning a different view, you are not returning a different page. URL is still the same, since it's based on the controller not the view. By rendering a different view you avoid having to complicate the code in the view that normally displays the data.

BTW, avoid giving "too much" information like "Invalid section ID" which can direct potential attacker to where to "poke" next.

I'd also rearrange the code so you assign value to model only when section is found and for the "no section found" view you obviously don't need to pass any model.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top