Nov122009

Why Controllers should have a default constructor?

    I am still in the journey of exploring MVC source code.  Yesterday, I wrote about controllers and why they need “Controller” suffix. Today, I will try to find out in the code, why does a controller need a default constructor. First of all, what is default constructor?
You can check wikipedia for its complete definition; but basically a default constructor is a constructor that has no parameters. If there is no other constructors in the code, compiler puts a default constructor for you in the code. You don’t believe me? Here is a simple class:

public class Person
    {
        public string FirstName { get; set; }
        public string LastName { get; set; }
    }


This class does not have any type of constructors, so the compiler put a default constructor for us. compile the project, check the assembly with either reflector or ildasm tool. You will see the default constructor in your compiled code (besides some other stuff):

image 
.ctor() is the default constructor that the compiler added for us, when we did not supply any constructor. So what happens when we add a constructor that takes a parameter but no default constructor:

public class Person
    {
        public string FirstName { get; set; }
        public string LastName { get; set; }

        public Person(string FirstName)
        {
            this.FirstName = FirstName;
        }
    }

We now have a constructor that takes the FirstName as a parameter, and we don’t have a default constructor. This time if you check the assembly again, you will find:

image

You see that, the constructor .ctor does take 1 parameter, and there is no default constructor for us. This means when you want to create a new instance of Person, you can NOT use parameter less constructor such as: Person p = new Person();  You have to call it with a parameter.
So far, this is basic C#, where is MVC section? Here you are:

When a request comes into the MVC, we saw that the framework finds the controller first. Inside the DefaultControllerFactory (if you downloaded the source code, it is in DefaultControllerFactory.cs line # 83), an instance of the controller is being created to process the request. Here is the code from the controller factory:

 protected internal virtual IController GetControllerInstance
(Type controllerType) { // some code is here... try { return (IController)Activator.CreateInstance(controllerType); } // some other code is here...

Activator.CreateInstance is used to create an instance of the controller. Let’s see MSDN for Activator.CreateInstance:

“CreateInstance(Type)    Creates an instance of the specified type using that type's default constructor.” So Activator.CreateInstance(controllerType) will create an instance of the specified controller using the controller’s default constructor. This is the main reason controllers should have a constructor, as DefaultControllerFactory creates the controller by using its default constructor.  Luckily if we don’t have a default constructor, the framework will throw an exception at runtime when it is trying to generate a new instance (I wish we could have compile time error).  The error you will have is very self explanatory.

image 

The framework is even showing us the exact location that throws the exception. So we need to supply a default constructor if we have defined constructors with parameters.
Why would you have a constructor in your controller that takes parameter if the framework does not call it? For 2 simple reasons:

  1. Unit Testing: You want to unit test your controller, and by supplying some values in the constructor you can swap the dependencies with some other implementations that you have control of.
  2. You want to do inversion of control. Basically if you have a class, and your class depends on some other entities (such as your controller needs a class to communicate with database, or a logger class to log the errors), it shouldn’t be a task of your controller to create an instance of this classes that it depends. The classes  should be instantiated and given to your controller ready to use.

 

Let’s see an example again, assume you have a DatabaseLogger class which derives from ILogger interface and sends the logged message to the database:

public interface ILogger
{
    void SendMessage(string Message);
}

public class DatabaseLogger: ILogger
{
    public void SendMessage(string Message)
   {
      // some code to write the message to the database


Our HomeController needs the DatabaseLogger class to send some messages. One way  of doing this is to put the instantiation inside the default controller such as:

public class HomeController:Controller
{
    private ILogger logger;

    public HomeController():base(new DatabaseLogger())
    {     
    }

    public HomeController(ILogger logger)
    {
        this.logger = logger;
    }
}

There are 2 constructors in this controller. The second one that has a parameter, sets the controller’s dependency to the passed in logger interface. The default constructor is creating an instance of DatabaseLogger and passing this to the second constructor. With this technique here we can unit test our controller as the second constructor gives us the ability to swap the ILogger with an object that we can control.  However, if you look at the default constructor, the controller is creating the class that it has the dependency on. It should be passed the depedency ready to use. In this simple example maybe the benefit is not obvious however think that DatabaseLogger() needs some construction parameters that will setup the database connection string, and maybe to read the database connection string, you have some other dependencies such as a configuration file reader. Now your homecontroller has to supply all these settings to create the DatabaseLogger. This is too much now, this is where you need to use dependency injection such as Castle Windsor, StructureMap, Ninject etc…

This will be another story :)



Tags: ,

E-mail | Permalink | Trackback | Post RSSRSS comment feed 2 Responses

Nov112009

Why Controllers should have a “Controller” Suffix?

   If you are programming with ASP.NET MVC, you know that the controller’s should have a suffix of “Controller” such as “HomeController”. You may also know that this is how DefaultControllerFactory works.
Let’s look into the technical details. I will skip some unrelated facts, but we will check the MVC source code, and modify it a little. Please go to CodePlex and download the MVC source code.
When a request comes into IIS (Microsoft’s web server application), IIS first checks the request, then the MVCRouteHandler is called (assuming this is an MVC request). The MVCRouteHandler creates an instance of  MvcHandler by passing the request context. You can check out the code in the MVCRouteHandler.cs file.

protected virtual IHttpHandler GetHttpHandler(RequestContext requestContext) {
            return new MvcHandler(requestContext);
        }


After creating the instance, ProcessRequest (from MVCHandler) is invoked and HttpContext is passed as the parameter. The ProcessRequest function is below:

 protected internal virtual void ProcessRequest(HttpContextBase httpContext) {
            AddVersionHeader(httpContext);

            // Get the controller type
            string controllerName = RequestContext.RouteData.
GetRequiredString("controller"); // Instantiate the controller and call Execute IControllerFactory factory = ControllerBuilder.
GetControllerFactory(); IController controller = factory.CreateController(RequestContext,
controllerName);
.... some other calls


As you can see, this function calls the GetControllerFactory() method, to get the ControllerFactory. If you haven’t registered a new ControllerFactory, this function returns the DefaultControllerFactory. The next call in ProcessRequest function is CreateController.  So let’s see what is happening in the DefaultControllerFactory’s CreateController function.

public virtual IController CreateController(RequestContext requestContext, 
string controllerName) { if (requestContext == null) { throw new ArgumentNullException("requestContext"); } if (String.IsNullOrEmpty(controllerName)) { throw new ArgumentException(
MvcResources.Common_NullOrEmpty, "controllerName"); } RequestContext = requestContext; Type controllerType = GetControllerType(controllerName); IController controller = GetControllerInstance(controllerType); return controller; }


This function after checking requestContext, and controllerName against null values, calls GetControllerType. Here is what GetControllerType does:

protected internal virtual Type GetControllerType(string controllerName) {
            if (String.IsNullOrEmpty(controllerName)) {
                throw new ArgumentException(MvcResources.Common_NullOrEmpty,
"controllerName"); } // first search in the current route's namespace collection object routeNamespacesObj; Type match; match = GetControllerTypeWithinNamespaces
(controllerName, nsHash); if (match != null) { return match; } } }


This is not the whole function but a piece of it.

Again after checking some values against null, it is calling the main function which is GetControllerTypeWithinNamespaces.  I am sure you are already bored from one function call to another function call :) however, I will be writing more about the MVC infrastructure, so it is good to show them. So one more time, let’s look at the GetControllerTypeWithinNamespace function.

The first thing this function does is to call: ControllerTypeCache.EnsureInitialized(BuildManager). This will load all the controllers and cache them, so that the next time a controller is needed, the framework doesn’t have to find and load the controllers. This is a simple yet pretty important function.

 public void EnsureInitialized(IBuildManager buildManager) {
     if (_cache == null) {
          lock (_lockObj) {
            if (_cache == null) {
              List<Type> controllerTypes = GetAllControllerTypes(buildManager);
              var groupedByName = controllerTypes.GroupBy(           
      t => t.Name.Substring(0, t.Name.Length - "Controller".Length),
                            StringComparer.OrdinalIgnoreCase);
       _cache = groupedByName.ToDictionary(
                            g => g.Key,
                            g => g.ToLookup(t => t.Namespace ?? 
String.Empty, StringComparer.OrdinalIgnoreCase), StringComparer.OrdinalIgnoreCase); } } } }


As you can see this function caches the controllers after loading them. The _cache variable that is used in this function is a Dictionary<string,ILookup<string,type>>. So basically the framework loads the controllers into a cache. The function that loads the controllers is GetAllControllerTypes. It loads all the assemblies, iterating through them by calling a function: IsControllerType. This is why, this needs to be cached, as you don’t want to load all the assemblies and check if they are controllers. So this IsControllerType function is the reason why we have to add suffix “Controller” to our controllers:

 internal static bool IsControllerType(Type t) {
            return
                t != null &&
                t.IsPublic &&
                t.Name.EndsWith("Controller", 
                StringComparison.OrdinalIgnoreCase) &&
                !t.IsAbstract &&
                typeof(IController).IsAssignableFrom(t);
        }


This function is checking if the passed in controller, has a suffix of “Controller” or not. If it does, it returns true, and GetAllControllerTypes adds this controller to the list, and returns the list of controllers.

The call goes back to EnsureInitialized which strips out the Controller suffix, and adds it into the cache. Let’s assume we want our controllers to have suffix of “Volkan” ; what do we need to do?

First you have to change the IsControllerType so that it checks .EndsWith(“Volkan”) not “Controller”. This will let the <controllerName>Volkan controllers to be added to the list. But this is not enough, the next thing we have to do is, go back to EnsureInitialized and change the stripping part from t.Name.Length - "Controller".Length to t.Name.Length - "Volkan".Length .

Finally you have to put the “Volkan” suffix to your controllers, and now this becomes a valid controller:

      public class HomeVolkan : Controller{

 

Enjoy your time debugging the MVC code!



Tags: ,

E-mail | Permalink | Trackback | Post RSSRSS comment feed 1 Responses

Nov052009

Sql injection & Sanitizing Database & Triggers

    In one of the web applications we have at my work, there is a sql injection problem. Unfortunately the web application is very old, programmed with classic asp, the company does not support it anymore. We have the source code for the web application as it is classic asp however; the code is not documented, and the hit points to the database is all over, so I can’t just simply go to one location in the source, and fix it. 
   The first thing I have to do after we are hit, is to stop the attacks, so after reporting the Ip address of the attacker to the related departments, I knew I couldn’t fix the problem in the code; so I added a trigger to the database, that will check the inserted data; and if found a <script command simply rollback. Here is a sample trigger that does this:

   1:  CREATE  TRIGGER [dbo].[checkNewsForScriptandIFrameAttach]
   2:  ON [dbo].[news]
   3:  AFTER INSERT, UPDATE
   4:  AS
   5:  IF @@ROWCOUNT=0
   6:  begin
   7:      RETURN
   8:  END
   9:  declare @headline varchar(350)
  10:   
  11:  set @headline = (select headline from inserted)
  12:   
  13:  IF(CHARINDEX('<iframe',@headline))>0
  14:  BEGIN
  15:      ROLLBACK TRANSACTION
  16:  END
  17:   
  18:  IF(CHARINDEX('<script',@headline))>0
  19:  BEGIN
  20:      ROLLBACK TRANSACTION
  21:  END

This is a trigger that will run after the insert and update sql commands on the database. What it does is, to get the column value from the inserted table (this is a special table that sql server uses, before it actually writes the value to the actual table), and checks if the inserted column has <iframe or <script tags. If there is any, then rollback the transaction and don’t write the new values to the actual table but wipe them out.

So the next step is to sanitize the table that is already hit, I could use TSQL for this, but I decided to use LINQ as I can log the operations and do some other things while I am sanitizing the table. I wrote this small console application:

   1:      NewsDBDataContext db = new NewsDBDataContext();
   2:   
   3:      var scriptCode = "<script src=http://www.somefckrcode></script>";
   4:   
   5:      var news = db.News;
   6:      foreach (var news in news
   7:      {
   8:         news.headline = news.headline.Replace(scriptCode,String.Empty);
   9:         db.SubmitChanges();
  10:         //some log operations here
  11:      }

All this code does is to replace the script tag with empty string. Of cource I can query the database, and bring in only the records that has the scriptCode value injected; however in my case all the rows has it, so I simply get all the records, and iterate through all of them.

I ran this application, and got the error from sql “Value will be truncated….”; it wasn’t a good error message; as it wasn’t really telling me anything. But quickly I figured out that the problem was the trigger, the trigger was on, and my code was checking the script and updating it, however trigger was throwing an error; I disabled my trigger, then ran the application, everything worked perfectly fine, and enabled the trigger back again.

One more note on this, the number of rows that  I am updating is around 9000; the update took around 5 minutes; when I replace the headline.Replace with a regular expression replace, the code took less than a minute :)



Tags:

E-mail | Permalink | Trackback | Post RSSRSS comment feed 0 Responses