When Rory was interviewing for his position he liked what he heard. A robust data access layer, and a company policy that dictated that if you get caught writing a non-parameterized query, you're tarred, feathered, and recommended for execution. Their rigidity and adherence to the practices, as well as an interviewer asking all of the right questions regarding secure coding and good design ultimately lead to his acceptence of the position.
Once he was finally in, he cracked open the code, excited to bask in the radiance of beautiful data access layers, secure code, and well-implemented design patterns.
As he skimmed some of the classes, he learned some of the standards. [ClassName]DAL was the data access layer for [ClassName]. Database connections were handled in a singleton, ConnectionMgr.Connection, though, strangely, each consumer had a check to see if the connection was open, and if not, open it.
The query situation was a little odd, since they were stored as a collection of strings in one big class called "Queries." Still, it was reassuring to see parameter references in the strings.
Moving on, he discovered how they did their own custom parameterization:
insertQuery = Queries.InsertForumSignUp; insertQuery = insertQuery.Replace("@Name", "'" + signUpEntity.Name + "'"); insertQuery = insertQuery.Replace("@Email", "'" + signUpEntity.Email + "'"); insertQuery = insertQuery.Replace("@Type", "'" + signUpEntity.Type + "'"); insertQuery = insertQuery.Replace("@ForumsOption", signUpEntity.ForumsOption.ToString());
Really? Really? So if you really want to make Rory have a miserable day, all it takes is turning off JavaScript (to bypass client-side data validation), some basic SQL knowledge, and a bad attitude.
The kicker? This next snippet immediately followed the Replace statements:
#region OdbcParameter //OdbcParameter paramName = new OdbcParameter("?Name", OdbcType.VarChar, 50); //paramName.Value = signUpEntity.Name; //cmd.Parameters.Add(paramName); //OdbcParameter paramEmail = new OdbcParameter("?Email", OdbcType.VarChar, 75); //paramEmail.Value = signUpEntity.Email; //cmd.Parameters.Add(paramEmail); //OdbcParameter paramType = new OdbcParameter("?Type", OdbcType.Char, 5); //paramType.Value = signUpEntity.Type; //cmd.Parameters.Add(paramType); //OdbcParameter paramForumOption = new OdbcParameter("@ForumOption", OdbcType.Int, 1); //paramForumOption.Value = signUpEntity.ForumOption; //cmd.Parameters.Add(paramForumOption); #endregion
So at some point they had done it the right way, but later changed it to the wrong way, but preseved the correct method for posterity, I guess.