Archive for the ‘CodeSOD’ Category

« Older Entries

CodeSOD: Thank You for Enabling JavaScript!

Wednesday, December 9th, 2009

Clean data makes me smile.

No really! When I have a finite number of brain cycles to dedicate to some process that receives user data, it makes me quite the happy guy knowing that it has been pre-scrubbed for such nasties as newline characters, the occasional ☺, or worse, the dreaded ಠ_ಠ.

Brion sent in this function which is supposed to prevent users from entering unclean text into a "thank you note" text area on a professional recognition site.  Now, before bashing the function for it's curious name, I must say that it does indeed work at filtering out a bunch of characters that the original developer thought would cause an issue.

function numbersonly(myfield, e, dec)
{
  var key;
  var keychar;

  if (window.event)
    key = window.event.keyCode;
  else if (e)
    key = e.which;
  else
    return true;
  keychar = String.fromCharCode(key);

  // control keys
  if ((key==null) || (key==0) || (key==8) ||
      (key==9) || (key==13) || (key==27) )
    return true;

  // numbers
  else if ((("0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZa" +
             "bcdefghijklmnopqrstuvwxyz.,-/?!$ ").indexOf(keychar) > -1))
    return true;

  // decimal point jump
  else if (dec && (keychar == "."))
  {
    myfield.form.elements[dec].focus();
    return false;
  }
  else
    return false;
}

Thank goodness everybody always runs JavaScript.


Posted in CodeSOD | Comments Off

CodeSOD: A Spacy Problem

Monday, December 7th, 2009

Like most of his past jobs, Don R started at a new company with high hopes and low expectations. And also like many of his past jobs, his dreams were quickly whisked away. This time, it happened on his first support ticket.

"This'll be an easy one," his programmer-turned-manager boss said, "this used to happen quite a bit, and I've fixed it a few times. Sometimes we'll get descriptions with extra spaces, and when we send a datafeed to the processing company, it can mess up their systems. But I'm sure you'll be able to figure it out, it's in the clean-up routines."

Don's boss was right, it was pretty easy to find. The code in question looked as follows....

// pull double spaces
itemDesc = 
    stringReplace(stringReplace(
    stringReplace(stringReplace(
    stringReplace(stringReplace(
    stringReplace(stringReplace(
    stringReplace(stringReplace(
    stringReplace(stringReplace(
    stringReplace(stringReplace(
    stringReplace(stringReplace(
    stringReplace(stringReplace(
    stringReplace(stringReplace(
    stringReplace(stringReplace(
    stringReplace(stringReplace(
      itemDesc, 
    "  ", " "),"  ", " "),
    "  ", " "),"  ", " "),
    "  ", " "),"  ", " "),
    "  ", " "),"  ", " "),
    "  ", " "),"  ", " "),
    "  ", " "),"  ", " "),
    "  ", " "),"  ", " "),
    "  ", " "),"  ", " "),
    "  ", " "),"  ", " "),
    "  ", " "),"  ", " "),
    "  ", " "),"  ", " "),
    "  ", " "),"  ", " ");

Curious as to why he chose twenty four replacements, Don took a peek in source control at the previus version...

// pull double spaces
itemDesc = 
    stringReplace(stringReplace(
    stringReplace(stringReplace(
    stringReplace(stringReplace(
    stringReplace(stringReplace(
      itemDesc, 
    "  ", " "),"  ", " "),
    "  ", " "),"  ", " "),
    "  ", " "),"  ", " "),
    "  ", " "),"  ", " ");

And then he went to the version before that...

// pull double spaces
itemDesc = 
    stringReplace(
    stringReplace(
    stringReplace(
    stringReplace(
      itemDesc, 
    "  ", " "),
    "  ", " "),
    "  ", " "),
    "  ", " ");

A pattern was starting to emerge. He went back another version....

// pull double spaces
itemDesc = stringReplace(stringReplace(
    stringReplace(itemDesc, "  ", " "),
    "  ", " "),
    "  ", " ");

And another version...

// pull double spaces
itemDesc = stringReplace(
    stringReplace(itemDesc, "  ", " "),
    "  ", " ");

Until finally, he reached the original....

// pull double spaces
itemDesc = stringReplace(itemDesc, "  ", " ");

As tempted as Don was to simply add in another bunch of stringReplace calls just for fun, he replaced the whole thing with this.

// pull double spaces
regexReplace(itemDesc, "[ ][ ]+", " ");


Posted in CodeSOD | Comments Off

CodeSOD: The “Who Knows?” Code

Wednesday, December 2nd, 2009

When Vladimir first received his orders, the blood drained from his face. He was to take the legacy VB5 application, a system used daily by scores of users daily, and uplift it to C++.

After reviewing a few parts of the application, things began looking hopeless, however there was some good news! Vladimir had a department full of developers at his disposal and most of them had a hand in the creating and maintenance of the application. 

After finding the following snippet of code, he printed it out and proceeded to grill every developer who was shown to have touched the source file in he past.

“No idea, go try Henrik,” said Edvard.

“Sorry, you might want to check in with Viktor, he wrote the reconciliation part of the system,” suggested Henrik.

“Nope, did you talk to Simon?” quizzed Viktor.

Simon lead to another developer, who suggested someone else and, skip ahead right on back to Edvard, the developer he had asked in the first place.

Facing a dead end and needing to move on with his life, Vladimir took the cavalier Oh, just screw it approach and pasted the code into the C++ app hoping that some day, it wouldn't come down to him having to explain what the code did.

If m(1) = 0 Then GoTo i2 
  For i1 = 1 To m(1) 
i2: If m(2) = 0 Then GoTo i3 
   For i2 = 1 To m(2) 
      If i2 = i1 Then Exit For 
i3: If m(3) = 0 Then GoTo i4 
      For i3 = 1 To m(3) 
        If i3 = i1 Or i3 = i2 Then Exit For 
i4:   If m(4) = 0 Then GoTo i5 
        For i4 = 1 To m(4) 
          If i4 = i1 Or i4 = i2 Or i4 = i3 Then Exit For 
i5:     If m(5) = 0 Then GoTo i6 
          For i5 = 1 To m(5) 
            If i5 = i1 Or i5 = i2 Or i5 = i3 Or i5 = i4 Then Exit For 
i6:       If m(6) = 0 Then GoTo i7 
            For i6 = 1 To m(6) 
              If i6 = i1 Or i6 = i2 Or i6 = i3 Or i6 = i4 Or _
	         i6 = i5 Then Exit For 
i7:         If m(7) = 0 Then GoTo i8 
              For i7 = 1 To m(7) 
                If i7 = i1 Or i7 = i2 Or i7 = i3 Or i7 = i4 Or _
		   i7 = i5 Or i7 = i6 Then Exit For 
i8:           If m(8) = 0 Then GoTo i9 
                For i8 = 1 To m(8) 
                  If i8 = i1 Or i8 = i2 Or i8 = i3 Or i8 = i4 Or _
		     i8 = i5 Or i8 = i6 Or i8 = i7 Then Exit For 
i9:             If m(9) = 0 Then GoTo i10 
                  For i9 = 1 To m(9) 
                    If i9 = i1 Or i9 = i2 Or i9 = i3 Or i9 = i4 Or _
		       i9 = i5 Or i9 = i6 Or i9 = i7 Or i9 = i8 Then Exit For
i10:              If m(10) = 0 Then GoTo i11 
                    For i10 = 1 To m(10) 
                      If i10 = i1 Or i10 = i2 Or i10 = i3 Or i10 = i4 Or _
		         i10 = i5 Or i10 = i6 Or i10 = i7 Or i10 = i8 Or _
			 i10 = i9 Then Exit For
i11:                If m(11) = 0 Then GoTo i12 
                      For i11 = 1 To m(11) 
                        If i11 = i1 Or i11 = i2 Or i11 = i3 Or i11 = i4 Or _
			   i11 = i5 Or i11 = i6 Or i11 = i7 Or i11 = i8 Or _
			   i11 = i9 Or i11 = i10 Then Exit For
i12:                  If m(12) = 0 Then GoTo i13 
                        For i12 = 1 To m(12) 
                          If i12 = i1 Or i12 = i2 Or i12 = i3 Or i12 = i4 Or _
			     i12 = i5 Or i12 = i6 Or i12 = i7 Or i12 = i8 Or _
			     i12 = i9 Or i12 = i10 Or i12 = i11 Then Exit For
i13:                    If m(13) = 0 Then GoTo i14 
                          For i13 = 1 To m(13) 
                            If i13 = i1 Or i13 = i2 Or i13 = i3 Or i13 = i4 Or _
			       i13 = i5 Or i13 = i6 Or i13 = i7 Or i13 = i8 Or _
			       i13 = i9 Or i13 = i10 Or i13 = i11 Or _
			       i13 = i12 Then Exit For
i14:                      If m(14) = 0 Then GoTo i15 
                            For i14 = 1 To m(14) 
                              If i14 = i1 Or i14 = i2 Or i14 = i3 Or _
			         i14 = i4 Or i14 = i5 Or i14 = i6 Or _
				 i14 = i7 Or i14 = i8 Or i14 = i9 Or _
				 i14 = i10 Or i14 = i11 Or _
				 i14 = i12 Or i14 = i13 Then Exit For
i15:                        If m(15) = 0 Then GoTo i16 
                                For i15 = 1 To m(15) 
                              If i15 = i1 Or i15 = i2 Or i15 = i3 Or _
			         i15 = i4 Or i15 = i5 Or i15 = i6 Or _
				 i15 = i7 Or i15 = i8 Or i15 = i9 Or _
				 i15 = i10 Or i15 = i11 Or i15 = i12 _
				 Or i15 = i13 Or i15 = i14 Then Exit For
i16:                          If m(16) = 0 Then GoTo i17 
                                  For i16 = 1 To m(16) 
                                If i16 = i1 Or i16 = i2 Or i16 = i3 Or _
				   i16 = i4 Or i16 = i5 Or i16 = i6 Or _
				   i16 = i7 Or i16 = i8 Or i16 = i9 Or _
				   i16 = i10 Or i16 = i11 Or i16 = i12 _
				   Or i16 = i13 Or i16 = i14 Or i16 = i15 Then Exit For
i17:                                 
                            Combination(kom + 1) = Format(i16, "00") & _
			      Format(i15, "00") & Format(i14, "00") & _
			      Format(i13, "00") & Format(i12, "00") & _
			      Format(i11, "00") & Format(i10, "00") & _
			      Format(i9, "00") & Format(i8, "00") & _
			      Format(i7, "00") & Format(i6, "00") & _
			      Format(i5, "00") & Format(i4, "00") & _
			      Format(i3, "00") & Format(i2, "00") & _
			      Format(i1, "00")
                            kom = kom + 1 
                                Next i16 
n16:                           If m(15) = 0 Then GoTo n15 
                              Next i15 
n15:                         If m(14) = 0 Then GoTo n14 
                            Next i14 
n14:                       If m(13) = 0 Then GoTo n13 
                          Next i13 
n13:                     If m(12) = 0 Then GoTo n12 
                        Next i12 
n12:                   If m(11) = 0 Then GoTo n11 
                      Next i11 
n11:                 If m(10) = 0 Then GoTo n10 
                    Next i10 
n10:               If m(9) = 0 Then GoTo n9 
                  Next i9 
n9:              If m(8) = 0 Then GoTo n8 
                Next i8 
n8:            If m(7) = 0 Then GoTo n7 
              Next i7 
n7:          If m(6) = 0 Then GoTo n6 
            Next i6 
n6:        If m(5) = 0 Then GoTo n5 
          Next i5 
n5:      If m(4) = 0 Then GoTo n4 
        Next i4 
n4:    If m(3) = 0 Then GoTo n3 
      Next i3 
n3:  If m(2) = 0 Then GoTo n2 
    Next i2 
n2: If m(1) = 0 Then GoTo n1 
  Next i1 
n1:


Posted in CodeSOD | Comments Off

CodeSOD: Pretty Basic Validation

Monday, November 30th, 2009

"For reasons beyond my comprehension," Kristof writes, "one of my coworkers has managed to keep his job after more than eighteen months of messing about. His latest project was to build an import feature in the admin module of our web application. The idea behind the feature was that the administrators could upload a tab-delimited text file containing a list of products, and the application would insert or update the products in the database."

"Of course, the import feature required some pretty basic validation," Kristof continued. "Is it actually a text file? Is it tab delimited? Are the columns correct? And so on."

"To solve this, my colleague figured the best way was to verify that the uploaded file's name had the correct extension of .txt. It's a decent first step that one would normally code as follows."

if (System.IO.Path.GetExtension(fileName).ToLower() == "txt")
{
    // The extension is OK. Proceed with the rest of the validation
}
else
{
    // Incorrect extension. Show error message.
}

"My colleague, on the other hand, came up with this."

string InvalidExtensions = ".exe;.dll;.com;.bat;.ini;.sys;.aspx;"
   + ".asp;.php;.htw;.ida;.idq;.asp;.cer;.cdx;.asa;.htr;.idc;.shtm;"
   + ".shtml;.stm;.printer;.asax;.ascx;.ashx;.asmx;.aspx;.axd;.rem;"
   + ".soap;.config;.cs;.csproj;.vb;.vbproj;.webinfo;.licx;.resx;"
   + ".resources;.vsdisco;";

if (!InvalidExtensions.Contains(fileName.ToString().Substring(fileName.ToString().LastIndexOf("."))))
{
    // The extension is OK. Proceed with the rest of the validation
}
else
{
    // Incorrect extension. Show error message.
}

"That's right," Kristof wrote. "He put a very limited list of invalid extensions in a string, and then made sure that the extension of the uploaded file was not in that list."

He added, "when I asked my colleague what would happen if someone uploaded a .doc file or a .pdf file, he replied 'Oh yeah, you're right... I should add those to the list as well!' I was stupefied."


Posted in CodeSOD | Comments Off

CodeSOD: A Random PHP Script

Wednesday, November 25th, 2009

Some time ago when Michael was tasked with updating some of code on an old website, one file on the server caught his eye.

Amid an ocean of static HTML files, it turned out that there was exactly one PHP script. Not sure what to expect, he was surprised when he discovered that the script was entirely comprised of vanilla HTML save for one line.

<script type="text/javascript"> 
  // <![CDATA[                
  var so = new SWFObject(
      "../theswf.swf?rand=<?php echo rand(100,999); ?>", 
      "theswf", "100%", "100%", "8", "#FFFFFF");

  so.addParam("menu", "false"); 
  so.addParam("base", "../"); 
  if( so.write('flashcontent') ) 
                        { 
    var macmousewheel = new SWFMacMouseWheel( so ); 
  } 
// ]]></script>

That’s right – the only reason this particular page was rendered via a script was to use PHP’s rand() function, which was obviously superior to Javascript's Math.random() 


Posted in CodeSOD | Comments Off

« Older Entries