There are few things I hate more than poorly written code examples that masquerade as useful tutorials/lessons/guides. Every day, readers of these "helpful" guides have no choice but to turn to the ASP.NET forums and Newsgroups seeking help. With a new blog sprouting up every second, I'd be a fool to launch a jihad against the mass-ignorance people seem intent on spewing; but damnit, why can't large
development companies get it right?
Of late I've had my sights set on Macromedia and their
craptastically written .NET material.
most of the .NET code you'll find on Macromedia's site is hugely insecure, impossible to maintain and likely to run slow as hell. Some of the material I'll reference hereafter is written by independent authors that submit their work to Macromedia, but since they all show up on macromedia.com (some of them even featured), it's squarely Macromedia's responsibility.
Let's look at
this example on how to modify the CommandText of a DataSet:
CommandText='<%# ((Request.QueryString["sortOn"] == null || Request.QueryString["sortOn"] == "" || Request.QueryString["sortDir"] == null || Request.QueryString["sortDir"] == "") ? "SELECT EmployeeID, LastName, FirstName, HireDate, Address, City, Region, PostalCode, Country, HomePhone FROM dbo.Employees ORDER BY LastName ASC" : "SELECT EmployeeID, LastName, FirstName, HireDate, Address, City, Region, PostalCode, Country, HomePhone FROM dbo.Employees ORDER BY " + Request.QueryString["sortOn"] + " " + Request.QueryString["sortDir"]) %>
There's actually a note below the code that says "Note that the above code must be on a single line; it cannot be broken into shorter segments over multiple lines." This "single line" is (a) impossible to maintain (b) open to huge SQL injections (c) a slap in the face of N-Tier and OO design.
There are some examples that aren't as bad, but still grate me. For example,
this general tutorial on how to connect flash to .NET has this little gem in it:
void Page_Load( Object sender, EventArgs e)
{
string queryString ;
OleDbConnection connection;
connection = new OleDbConnection( @"Provider=Microsoft.Jet.OleDb.4.0; Data Source=Server.MapPath( "Northwind.mdb" );
connection.Open();
OleDbDataAdapter adapter = new OleDbDataAdapter( queryString, connection );
DataSet dbData = new DataSet();
adapter.Fill( dbData, "Results" );
connection.Close();
}
To credit the author, he at least points out that "it's a good idea to place the database in a private directory that is inaccessible from the web" and makes a small reference to Server.MapPath, but does any one actually think a database path should be coming in from the query string, un-validated?! What irritates me more is the lack of try/finally or using - simply because too many examples exhibit the same lazy oversight. Don't give me any of that editorial crap for online resources either - 99% of the authors that write tutorials like this write code the same way. if (adapter is IDisposable) Console.WriteLine("DISPOSE ME!");
My disgust with Macromedia's lack of groking .NET all began when I tried to familiarize myself with Flash Remoting (the binary protocol used for Flash to communicate with the server-side code). Page 6 of the PDF is a pretty big offender. Although the two examples above are pretty bad, the code written by Macromedia itself is even worse. Here's an example of what I mean:
protected System.Data.OleDb.OleDbConnection sqlConnection;
public DataTable getProductsByCategory(
int catId)
{
string sql = "
SELECT productId, name, description,
categoryId, image, price FROM Product WHERE categoryId = " + catId;
GetConnection();
OleDbDataAdapter da =
new OleDbDataAdapter(sql, sqlConnection);
DataSet ds =
new DataSet();
da.Fill(ds, "
Products");
return ds.Tables["Products"];
}
private void GetConnection()
{
String source1 = "
Provider=Microsoft.Jet.OLEDB.4.0;Data Source=C:\\Inetpub\\wwwroot\\flashremoting\\samples\\db\\sports2000.mdb;";
// get a connection using the connect info that works on this SDK platform sqlConnection =
new OleDbConnection(source1);
try {
sqlConnection =
new OleDbConnection(source1);
}
catch(Exception)
{
}
}
To be efficient, I'll use a list:
- Import a namespace but don't use it (System.Data.OleDb is imported)
- Protected fields
- Hard coded connection string
- Less than meaningful variable names ("source1", "sqlConnection" for OleDbConnection object)
- Perfect example of how to never swallow exceptions
- Not using command parameters
- Creating a dataset just to return a datatable
- Resources never released
Maybe it's just me, but to add insult to injury, the comment "get a connection using the connect info that works on this SDK platform" doesn't even seem to be written in English!
After seeing that code, it didn't surprise me to see Macromedia's actual .NET products lacking as well. They use an HttpModule where an HttpHandler would be far better suited. To get Flash remoting working, you need a physical page called FlashGateway.aspx. This page is empty and has no associated codebehind. Rather, the HttpModule checks to see if that page was hit to do it's thing. It's inefficient and screams of not understanding modules and handlers.
If a company writes documentation that features highly insecure code, what expectations should we have from the products it delivers? If their documentation results in hard-to-maintain code, how stable do we think their offerings will be? Macromedia's development center obviously needs to step it up a notch and bring in some technical editors who know what they are doing.