Alex saw, in the company's codebase, a method called recursive_readdir. It had no comments, but the name seemed pretty clear: it would read directories recursively, presumably enumerating their contents.

Fortunately for Alex, they checked the code before blindly calling the method.

public function recursive_readdir($path) { $handle = opendir($path); while (($file = readdir($handle)) !== false) { if ($file != '.' && $file != '..') { $filepath = $path . '/' . $file; if (is_dir($filepath)) { rmdir($filepath); recursive_readdir($filepath); } else { unlink($filepath); } } } closedir($handle); rmdir($path); }

This is a recursive delete. rmdir requires the target directory to be empty, so this recurses over all the files and subfolders in the directory, deleting them, so that we can delete the directory.

This code is clearly cribbed from comments on the PHP documentation, with a fun difference in that this version is both unclearly named, and also throws an extra rmdir call in the is_dir branch- a potential "optimization" that doesn't actually do anything (it either fails because the directory isn't empty, or we end up calling it twice anyway).

Alex learned to take nothing for granted in this code base.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

In the far-off era of the late-90s, Jens worked for a small software shop that built tools for enterprise customers. It was a small shop, and most of the projects were fairly small- usually enough for one developer to see through to completion.

A co-worker built a VB4 (the latest version available) tool that interfaced with an Oracle database. That co-worker quit, and that meant this tool was Jens's job. The fact that Jens had never touched Visual Basic before meant nothing.

With the original developer gone, Jens had to go back to the customer for some knowledge transfer. "Walk me through how you use the application?"

"The main thing we do is print reports," the user said. They navigated through a few screens worth of menus to the report, and got a preview of it. It was a simple report with five records displayed on each page. The user hit "Print", and then a dialog box appeared: "Print Page 1? [Yes] [No]". The user clicked "Yes". "Print Page 2? [Yes] [No]". The user started clicking "no", since the demo had been done and there was no reason to burn through a bunch of printer paper.

"Wait, is this how this works?" Jens asked, not believing his eyes.

"Yes, it's great because we can decide which pages we want to print," the user said.

"Print Page 57? [Yes] [No]".

With each page, the dialog box took longer and longer to appear, the program apparently bogging down.

Now, the code is long lost, and Jens quickly forgot everything they learned about VB4 once this project was over (fair), so instead of a pure code sample, we have here a little pseudocode to demonstrate the flow:

for k = 1 to runQuery("SELECT MAX(PAGENO) FROM ReportTable WHERE ReportNumber = :?", iRptNmbr) dataset = runQuery("SELECT * FROM ReportTable WHERE ReportNumber = :?", iRptNmbr) for i = 0 to dataset.count - 1 if dataset.pageNo = k then useRecord(dataset) dataset.MoveNext end next if MsgBox("Do you want to print page k?", vbYesNo) = vbYes then print(records) end next

"Print Page 128? [Yes] [No]"

The core thrust is that we query the number of pages each time we run the loop. Then we get all of the rows for the report, and check each row to see if they're supposed to be on the page we're printing. If they are, useRecord stages them for printing. Once they're staged, we ask the user if they should be printed.

"Why doesn't it just give you a page selector, like Word does?" Jens asked.

"The last guy said that wasn't possible."

"Print Page 170? [Yes] [No]"

Jens, ignorant of VB, worried that he stepped on a land-mine and had just promised the customer something the tool didn't support. He walked the statement back and said, "I'll look into it, to see if we can't make it better."

It wasn't hard for Jens to make it better: not re-running the query for each page and not iterating across the rows of previous pages on every page boosted performance.

"Print Page 201? [Yes] [No]"

Adding a word-processor-style page selector wasn't much harder. If not for that change, that poor user might be clicking "No" to this very day.

"Print Page 215? [Yes] [No]"

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!

One would imagine that logging has been largely solved at this point. Simple tasks, like, "Only print this message when we're in debug mode," seem like obvious, well-understood features for any logging library.

"LostLozz offers us a… different approach to this problem.

if ( LOG.isDebugEnabled() ) { try { Integer i = null; i.doubleValue(); } catch ( NullPointerException e ) { LOG.debug(context.getIdentity().getToken() + " stopTime:" + instrPoint.getDescription() + " , " + instrPoint.getDepth(), e); } }

If we're in debug mode, trigger a null pointer exception, and catch it. Then we can log our message, including the exception- presumably because we want the stack trace. Because there's not already a method for doing that (there is).

I really "love" how much code this is to get to a really simple result. And this code doesn't appear in the codebase once, this is a standardized snippet for all logging. Our submitter didn't include any insight into what instrPoint may be, but I suspect it's a tracing object that's only going to make things more complicated. getDescription and getDepth seem to be information about what our execution state is, and since this snippet was widely reused, I suspect it's a property on a common-base class that many objects inherit from, but I'm just guessing. Guessing based on a real solid sense of where things can go wrong, but still a guess.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

Dave's codebase used to have this function in it:

public DateTime GetBeginDate(DateTime dateTime) { return new DateTime(dateTime.Year, dateTime.Month, 01).AddMonths(1); }

I have some objections to the naming here, which could be clearer, but this code is fine, and implements their business rule.

When a customer subscribes, their actual subscription date starts on the first of the following month, for billing purposes. Note that it's passed in a date time, because subscriptions can be set to start in the future, or the past, with the billing date always tied to the first of the following month.

One day, all of this worked fine. After a deployment, subscriptions started to ignore all of that, and always started on the date that someone entered the subscription info.

One of the commits in the release described the change:

Adjusted the begin dates for the subscriptions to the start of the current month instead of the start of the following month so that people who order SVC will have access to the SVC website when the batch closes.

This sounds like a very reasonable business process change. Let's see how they implemented it:

public DateTime GetBeginDate(DateTime dateTime) { return DateTime.Now; }

That is not what the commit claims happens. This just ignores the submitted date and just sets every subscription to start at this very moment. And it doesn't tie to the start of a month, which not only is different from what the commit says, but also throws off their billing system and a bunch of notification modules which all assume subscriptions start on the first day of a month.

The correct change would have been to simply remove the AddMonths call. If you're new here, you might wonder how such an obvious blunder got past testing and code review, and the answer is easy: they didn't do any of those things.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!

Date problems continue again this week as usual, both sublime (Goodreads!) and mundane (a little light time travel). If you want to be frist poster today, you're going to really need that time machine.

Early Bird Dave crowed "Think you're hot for posting the first comment? I posted the zeroth reply to this comment!" You got the worm, Dave.

 

Don M. sympathized for the poor underpaid time traveler here. "I feel sorry for the packer on this order....they've a long ways to travel!" I think he's on his way to get that minusfirsth post.

 

Cardholder Other Dave L. "For Co-Op bank PIN reminder please tell us which card, but whatever you do, for security reason don't tell us which card" This seems like a very minor wtf, their instructions probably should have specified to only send the last 4 and Other Dave used all 16.

 

Diligent Mark W. uncovered an innovative solution to date-picker-drudgery. If you don't like the rules, make new ones! Says Mark, "Goodreads takes the exceedingly lazy way out in their app. Regardless of the year or month, the day of month choice always goes up to 31."

 

Finally this Friday, Peter W. found a classic successerror. "ChituBox can't tell if it succeeded or not." Chitu seems like the glass-half-full sort of android.

 

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

Rui recently pulled an all-nighter on a new contract. The underlying system is… complicated. There's a PHP front end, which also talks directly to the database, as well as a Java backend, which also talks to point-of-sale terminals. The high-level architecture is a bit of a mess.

The actual code architecture is also a mess.

For example, this code lives in the Java portion.

final class Status { static byte [] status; static byte [] normal = {22,18,18,18}; //snip public static boolean equals(byte[] array){ boolean value=true; if(status[0]!=array[0]) value=false; if(status[1]!=array[1]) value=false; if(status[2]!=array[2]) value=false; if(status[3]!=array[3]) value=false; return value; } }

The status information is represented as a string of four integers, with the normal status being the ever descriptive "22,18,18,18". Now, these clearly are code coming from the POS terminal, and clearly we know that there will always be four of them. But boy, it'd be nice if this code represented that more clearly. A for loop in the equals method might be nice, or given that there are four distinct status codes, maybe put them in variables with names?

But that's just the aperitif.

The PHP front end has code that looks like this:

$sql = "select query from table where id=X"; $result = mysql_query($sql); // ... snip few lines of string munging on $result... $result2 = mysql_query($result);

We fetch a field called "query" from the database, mangle it to inject some values, and then execute it as a query itself. You know exactly what's happening here: they're storing database queries in the database (so users can edit them! This always goes well!) and then the front end checks the database to know what queries it should be executing.

Rui is looking forward to the end of this contract.

.comment { border: none; } [Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

Branon's boss, Steve, came storming into his cube. From the look of panic on his face, it was clear that this was a full hair-on-fire emergency.

"Did we change anything this weekend?"

"No," Branon said. "We never deploy on a weekend."

"Well, something must have changed?!"

After a few rounds of this, Steve's panic wore off and he explained a bit more clearly. Every night, their application was supposed to generate a set of nightly reports and emailed them out. These reports went to a number of people in the company, up to and including the CEO. Come Monday morning, the CEO checked his inbox and horror of horror- there was no report!

"And going back through people's inboxes, this seems like it's been a problem for months- nobody seems to have received one for months."

"Why are they just noticing now?" Branon asked.

"That's really not the problem here. Can you investigate why the emails aren't going out?"

Branon put aside his concerns, and agreed to dig through and debug the problem. Given that it involved sending emails, Branon was ready to spend a long time trying to debug whatever was going wrong in the chain. Instead, finding the problem only took about two minutes, and most of that was spent getting coffee.

public void Send() { //TODO: send email here }

This application had been in production over a year. This function had not been modified in that time. So while it's technically true that no one had received a report "for months" (16 months is a number of months), it would probably have been more accurate to say that they had never received a report. Now, given that it had been over a year, you'd think that maybe this report wasn't that important, but now that the CEO had noticed, it was the most important thing at the company. Work on everything else stopped until this was done- mind you, it only took one person a few hours to implement and test the feature, but still- work on everything else stopped.

A few weeks later a new ticket was opened: people felt that the nightly reports were too frequent, and wanted to instead just go to the site to pull the report, which is what they had been doing for the past 16 months.

.comment { border: none; } [Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

Donald was cutting a swathe through a jungle of old Java code, when he found this:

protected void waitForEnd(float time) { // do nothing }

Well, this function sure sounds like it's waiting around to die. This protected method is called from a private method, and you might expect that child classes actually implement real functionality in there, but there were no child classes. This was called in several places, and each time it was passed Float.MAX_VALUE as its input.

Poking at that odd function also lead to this more final method:

public void waitAtEnd() { System.exit(0); }

This function doesn't wait for anything- it just ends the program. Finally and decisively. It is the end.

I know the end of this story: many, many developers have worked on this code base, and many of them hoped to clean up the codebase and make it better. Many of them got lost, never to return. Many ran away screaming.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!

FreeBSDGuy sends us a VB .Net snippet, which layers on a series of mistakes:

If (gLang = "en") Then If (item.Text.Equals("Original")) Then item.Enabled = False End If ElseIf (gLang = "fr") Then If (item.Text.Equals("Originale")) Then item.Enabled = False End If Else If (item.Text.Equals("Original")) Then item.Enabled = False End If End If

The goal of this code is to disable the "original" field, so the user can't edit it. To do this, it checks what language the application is configured to use, and then based on the language, checks for the word "Original" in either English or French.

The first obvious mistake is that we're identifying UI widgets based on the text inside of them, instead of by some actual identifier.

As an aside, this text sure as heck sounds like a label which already doesn't allow editing, so I think they're using the wrong widget here, but I can't be sure.

Then we're hard-coding in our string for comparison, which is already not great, but then we are hard-coding in two languages. It's worth noting that .NET has some pretty robust internationalization features that help you externalize those strings. I suspect this app has a lot of if (gLang = "en") calls scattered around, instead of letting the framework handle it.

But there's one final problem that this code doesn't make clear: they are using more unique identifiers to find this widget, so they don't actually need to do the If (item.Text.Equals("Original")) check. FreeBSDGuy replaced this entire block with a single line:

item.Enabled = False [Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

Inability to properly program dates continued to afflict various websites last week, even though the leap day itself had passed. Maybe we need a new programming language in which it's impossible to forget about timezones, leap years, or Thursday.

Timeless Thomas subtweeted "I'm sure there's a great date-related WTF story behind this tweet" Gosh, I can't imagine what error this could be referring to.

 

Data historian Jonathan babbled "Today, the 1st of March, is the start of a new tax year here and my brother wanted to pull the last years worth of transactions from a financial institution to declare his taxes. Of course the real WTF is that they only allow up to 12 months." I am not able rightly to apprehend the confusion of ideas that could provoke such an error'd.

 

Ancient Matthew S. breathed a big sigh of relief on seeing this result: "Good to know that I'm up to date as of 422 years ago!"

 

Jaroslaw gibed "Looks like a translation mishap... What if I didn't knew English?" Indeed.

 

Hardjay vexillologist Julien casts some shade without telling us where to direct our disdain "I don't think you can have dark mode country flags..." He's not wrong.

 

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!

Today, John sends us a confession. This is his code, which was built to handle ISO 8583 messages. As we'll see from some later comments, John knows this is bad.

The ISO 8583 format is used mostly in financial transaction processing, frequently to talk to ATMs, but is likely to show up somewhere in any transaction you do that isn't pure cash.

One of the things the format can support is bitmaps- not the image format, but the "stuff flags into an integer" format. John wrote his own version of this, in C#. It's a long class, so I'm just going to focus on the highlights.

private readonly bool[] bits;

Look, we don't start great. This isn't an absolute mistake, but if you're working on a data structure that is meant to be manipulated via bitwise operations, just lean into it. And yes, if endianness is an issue, you'll need to think a little harder- but you need to think about that anyway. Use clear method names and documentation to make it readable.

In this developer's defense, the bitmap's max size is 128 bits, which doesn't have a native integral type in C#, but a pair of 64-bits would be easier to understand, at least for me. Maybe I've just been infected by low-level programming brainworms. Fine, we're using an array.

Now, one thing that's important, is that we're using this bitmap to represent multiple things.

public bool IsExtendedBitmap { get { return this.IsFieldSet(1); } }

Note how the 1st bit in this bitmap is the IsExtendedBitmap flag. This controls the length of the total bitmap.

Which, as an aside, they're using IsFieldSet because zero-based indexes are too hard:

public bool IsFieldSet(int field) { return this.bits[field - 1]; }

But things do get worse.

/// <summary> /// Sets a field /// </summary> /// <param name="field"> /// Field to set /// </param> /// <param name="on"> /// Whether or not the field is on /// </param> public void SetField(int field, bool on) { this.bits[field - 1] = on; this.bits[0] = false; for (var i = 64; i <= 127; i++) { if (this.bits[i]) { this.bits[0] = true; break; } } }

I included the comments here because I want to highlight how useless they are. The first line makes sense. Then we set the first bit to false. Which, um, was the IsExtendedBitmap flag. Why? I don't know. Then we iterate across the back half of the bitmap and if there's anything true in there, we set that first bit back to true.

Which, by writing that last paragraph, I've figured out what it's doing: it autodetects whether you're using the higher order bits, and sets the IsExtendedBitmap as appropriate. I'm not sure this is actually correct behavior- what happens if I want to set a higher order bit explicitly to 0?- but I haven't read the spec, so we'll let it slide.

public virtual byte[] ToMsg() { var lengthOfBitmap = this.IsExtendedBitmap ? 16 : 8; var data = new byte[lengthOfBitmap]; for (var i = 0; i < lengthOfBitmap; i++) { for (var j = 0; j < 8; j++) { if (this.bits[i * 8 + j]) { data[i] = (byte)(data[i] | (128 / (int)Math.Pow(2, j))); } } } if (this.formatter is BinaryFormatter) { return data; } IFormatter binaryFormatter = new BinaryFormatter(); var bitmapString = binaryFormatter.GetString(data); return this.formatter.GetBytes(bitmapString); }

Here's our serialization method. Note how here, the length of the bitmap is either 8 or 16, while previously we were checking all the bits from 64 up to see if it was extended. At first glance, this seemed wrong, but then I realized- data is a byte[]- so 16 bytes is indeed 128 bits.

This gives them the challenging problem of addressing individual bits within this data structure, and they clearly don't know how bitwise operations work, so we get the lovely Math.Pow(2, j) in there.

Ugly, for sure. Unclear, definitely. Which only gets worse when we start unpacking.

public int Unpack(byte[] msg, int offset) { // This is a horribly nasty way of doing the bitmaps, but it works // I think... var lengthOfBitmap = this.formatter.GetPackedLength(16); if (this.formatter is BinaryFormatter) { if (msg[offset] >= 128) { lengthOfBitmap += 8; } } else { if (msg[offset] >= 0x38) { lengthOfBitmap += 16; } } var bitmapData = new byte[lengthOfBitmap]; Array.Copy(msg, offset, bitmapData, 0, lengthOfBitmap); if (!(this.formatter is BinaryFormatter)) { IFormatter binaryFormatter = new BinaryFormatter(); var value = this.formatter.GetString(bitmapData); bitmapData = binaryFormatter.GetBytes(value); } // Good luck understanding this. There be dragons below for (var j = 0; j < 8; j++) { this.bits[i * 8 + j] = (bitmapData[i] & (128 / (int)Math.Pow(2, j))) > 0; } return offset + lengthOfBitmap; }

Here, we get our real highlights: the comments. "… but it works… I think…". "Good luck understanding this. There be dragons below."

Now, John wrote this code some time ago. And the thing that I get, when reading this code, is that John was likely somewhat green, and didn't fully understand the problem in front of him or the tools at his disposal to solve it. Further, this was John's independent project, which he was doing to solve a very specific problem- so while the code has problems, I wouldn't heap up too much blame on John for it.

Which, like many other confessional Code Samples-of-the-Day, I'm sharing this because I think it's an interesting learning experience. It's less a "WTF!" and more a, "Oh, man, I see that things went really wrong for you." We all make mistakes, and we all write terrible code from time to time. Credit to John for sharing this mistake.

.comment { border: none; } [Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

Today's submitter identifies themselves as pleaseKillMe, which hey, c'mon buddy. Things aren't that bad. Besides, you shouldn't let the bad code you inherit drive you to depression- it should drive you to revenge.

Today's simple representative line is one that we share because it's not just representative of our submitter's code base, but one that shows up surprisingly often.

SELECT * FROM users WHERE last_name='NULL'

Now, I don't think this particular code impacted Mr. Null, but it certainly could have. That's just a special case of names being hard.

In this application, last_name is a nullable field. They could just store a NULL, but due to data sanitization issues, they stored 'NULL' instead- a string. NULL is not 'NULL', and thus- we've got a lot of 'NULL's that may have been intended to be NULL, but also could be somebody's last name. At this point, we have no way to know.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

It's a nearly universal experience that the era of our youth and early adulthood is where we latch on to for nostalgia. In our 40s, the music we listened to in our 20s is the high point of culture. The movies we watched represent when cinema was good, and everything today sucks.

And, based on the sheer passage of calendar time, we have a generation of adults whose nostalgia has latched onto Flash. I've seen many a thinkpiece lately, waxing rhapsodic about the Flash era of the web. I'd hesitate to project a broad cultural trend from that, but we're roughly about the right time in the technology cycle that I'd expect people to start getting real nostalgic for Flash. And I'll be honest: Flash enabled some interesting projects.

Of course, Flash also gave us Flex, and I'm one of the few people old enough to remember when Oracle tried to put their documentation into a Flex based site from which you could not copy and paste. That only lasted a few months, thankfully, but as someone who was heavily in the Oracle ecosystem at the time, it was a terrible few months.

In any case, long ago, CW inherited a Flash-based application. Now, Flash, like every UI technology, has a concept of "containers"- if you put a bunch of UI widgets inside a container, their positions (default) to being relative to the container. Move the container, and all the contents move too. I think we all find this behavior pretty obvious.

CW's co-worker did not. Here's how they handled moving a bunch of related objects around:

public function updateKeysPosition(e:MouseEvent):void{ if(dragging==1){ theTextField.x=catButtonArray[0].x-100; theTextField.y=catButtonArray[0].y-200; catButtonArray[1].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10; catButtonArray[1].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[2].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth; catButtonArray[2].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[3].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth*2; catButtonArray[3].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[4].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth*3; catButtonArray[4].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[5].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth*4; catButtonArray[5].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[6].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth*5; catButtonArray[6].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[7].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth*6; catButtonArray[7].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[8].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth*7; catButtonArray[8].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[9].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth*8; catButtonArray[9].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[10].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth*9; catButtonArray[10].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[11].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30; catButtonArray[11].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[12].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30+keyWidth; catButtonArray[12].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[13].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30+keyWidth*2; catButtonArray[13].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[14].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30+keyWidth*3; catButtonArray[14].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[15].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30+keyWidth*4; catButtonArray[15].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[16].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30+keyWidth*5; catButtonArray[16].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[17].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30+keyWidth*6; catButtonArray[17].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[18].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30+keyWidth*7; catButtonArray[18].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[19].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30+keyWidth*8; catButtonArray[19].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[20].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+60; catButtonArray[20].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+180; catButtonArray[21].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+60+keyWidth; catButtonArray[21].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+180; catButtonArray[22].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+60+keyWidth*2; catButtonArray[22].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+180; catButtonArray[23].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+60+keyWidth*3; catButtonArray[23].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+180; catButtonArray[24].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+60+keyWidth*4; catButtonArray[24].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+180; catButtonArray[25].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+60+keyWidth*5; catButtonArray[25].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+180; catButtonArray[26].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+60+keyWidth*6; catButtonArray[26].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+180; //SPACE catButtonArray[27].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+228; catButtonArray[27].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+240; //RETURN catButtonArray[28].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+558; catButtonArray[28].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+207; } } [Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

In the great olden times, when Classic ASP was just ASP, there were a surprising number of intranet applications built in it. Since ASP code ran on the server, you frequently needed JavaScript to run on the client side, and so many applications would mix the two- generating JavaScript from ASP. This lead to a lot of home-grown frameworks that were wobbly at best.

Years ago, Melinda inherited one such application from a 3rd party supplier.

<script type='text/javascript' language="JavaScript"> var NoOffFirstLineMenus=3; // Number of first level items function BeforeStart(){return;} function AfterBuild(){return;} function BeforeFirstOpen(){return;} function AfterCloseAll(){return;} // Menu tree <% If Session("SubSystem") = "IndexSearch" Then %> <% If Session("ReturnURL") = "" Then %> Menu1=new Array("Logoff","default.asp","",0,20,100,"","","","","","",-1,-1,-1,"","Logoff"); <% else %> Menu1=new Array("<%=session("returnalt")%>","returnredirect.asp","",0,20,100,"","","","","","",-1,-1,-1,"","Return to Application"); <% end if %> Menu2=new Array("Menu","Menu.asp","",0,20,100,"","","","","","",-1,-1,-1,"","Menu"); Menu3=new Array("Back","","",5,20,40,"","","","","","",-1,-1,-1,"","Back to Previous Pages"); Menu3_1=new Array("<%= Session("sptitle") %>",<% If OWFeatureExcluded(Session("UserID"),"Web Index Search","SYSTEM","","")Then %>"","",0,20,130,"#33FFCC","#33FFCC","#C0C0C0","#C0C0C0","","","","","","",-1,-1,-1,"","<%= Session("sptitle") %>"); <% Else %>"SelectStorage.asp","",0,20,130,"","","","","","",-1,-1,-1,"","<%= Session("sptitle") %>"); <% End If %> Menu3_2=new Array("Indexes","IndexRedirect.asp?<%= Session("ReturnQueryString")%>","",0,20,95,"","","","","","",-1,-1,-1,"","Enter Index Search Criteria"); Menu3_3=new Array("Document List","DocumentList.asp?<%= Session("ReturnQueryString")%>","",0,20,130,"","","","","","",-1,-1,-1,"","Current Document List"); Menu3_4=new Array("Document Detail",<% If OWFeatureExcluded(Session("UserID"),"Web Document Detail",Documents.Fields.Item("StoragePlace").Value,"","") Then %>"","",0,20,135,"#33FFCC","#33FFCC","#C0C0C0","#C0C0C0","","","","","","",-1,-1,-1,"","Document Details"); <% Else %>"DetailPage.asp?CounterKey=<%= Request.QueryString("CounterKey") %>","",0,20,135,"","","","","","",-1,-1,-1,"","Document Details");<% End If %> Menu3_5=new Array("Comments","Annotations.asp?CounterKey=<%= Request.QueryString("CounterKey") %>","",0,20,70,"","","","","","",-1,-1,-1,"","Document Comments"); <% Else %> <% If Session("ReturnURL") = "" Then %> Menu1=new Array("Logoff","default.asp","",0,20,100,"","","","","","",-1,-1,-1,"","Logoff"); <% else %> Menu1=new Array("<%=session("returnalt")%>","returnredirect.asp","",0,20,100,"","","","","","",-1,-1,-1,"","Return to Application"); <% end if %> Menu2=new Array("Menu","Menu.asp","",0,20,100,"","","","","","",-1,-1,-1,"","Menu"); Menu3=new Array("Back","","",3,20,40,"","","","","","",-1,-1,-1,"","Back to Previous Pages"); Menu3_1=new Array("Document List","SearchDocumentList.asp?<%= Session("ReturnQueryString") %>","",0,20,130,"","","","","","",-1,-1,-1,"","Current Document List"); Menu3_2=new Array("Document Detail","DetailPage.asp?CounterKey=<%= Request.QueryString("CounterKey") %>","",0,20,135,"","","","","","",-1,-1,-1,"","Document Details"); Menu3_3=new Array("Comments","Annotations.asp?CounterKey=<%= Request.QueryString("CounterKey") %>","",0,20,70,"","","","","","",-1,-1,-1,"","Document Comments"); <% End If %> </script>

Here, the ASP code just provides some conditionals- we're checking session variables, and based on those we emit slightly different JavaScript. Or sometimes the same JavaScript, just to keep us on our toes.

The real magic is that this isn't the code that actually renders menu items, this is just where they get defined, and instead of using objects in JavaScript, we just use arrays- the label, the URL, the colors, and many other parameters that control the UI elements are just stuffed into an array, unlabeled. And then there are also the extra if statements, embedded right inline in the code, helping to guarantee that you can't actually debug this, because you can't understand what it's actually doing without really sitting down and spending time with it.

Of course, this application is long dead. But for Melinda, the memory lives on.

[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.

Not exactly once, I sincerely hope. That would be tragic.

"Apparently, today's leap day is causing a denial of service error being able to log into our Cemetery Management software due to some bad date calculations," writes Steve D. To be fair, he points out, it doesn't happen often.

 

In all seriousness, unusual as that might be, I do have cemeteries on my mind this week. I recently discovered a web site that has photographs of hundreds of my relatives' graves, and a series of memorials for "Infant Spencer" and "Infant Strickland" and "Infant McHugh", along with another named dozen deceased age 0. Well, it's sobering. Taking a moment here in thanks to Doctors Pasteur, Salk, Jenner, et.al. And now, back to our meagre ration of snark.

Regular Peter G. found a web site that thought Lorem Ipsum was too inaccessible to the modern audience, so they translate it to English. Peter muses "I've cropped out the site identity because it's a smallish company that provides good service and I don't want to embarrass them, but I'm kinda terrified at what a paleo fap pour-over is. Or maybe it's the name of an anarcho-punk fusion group?"

 

"Beat THAT, Kasparov!" crows Orion S.

 

"Insert Disc 2 into your Raspberry Pi" quoth an anonymous poster. "I'm still looking for a way to acquire an official second installation disc for qt for Debian."

 

Finally, Michael P. just couldn't completely ignore this page, could he? "I wanted to unsubscribe to this, but since my email is not placeholderEmail, I guess I should ignore the page." I'm sure he did a yeoman's job of trying.

 

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

Brian was working on landing a contract with a European news agency. Said agency had a large number of intranet applications of varying complexity, all built to support the news business.

Now, they understood that, as a news agency, they had no real internal corporate knowledge of good software development practices, so they did what came naturally: they hired a self-proclaimed "code guru" to built the system.

Said code guru was notoriously explosive. When users came to him with complaints like "your system lost all the research I've been gathering for the past three months!" the guru would shout about how users were doing it wrong, couldn't be trusted to handle the most basic tasks, and "wiping your ass isn't part of my job description."

With a stellar personality like that, what was his PHP code like?

$req000="SELECT idfiche FROM fiche WHERE idevent=".$_GET['id_evt']; $rep000=$db4->query($req000); $nb000=$rep000->numRows(); if ($nb000>0) { while ($row000=$rep000->fetchRow(DB_FETCHMODE_ASSOC)) { $req001="UPDATE fiche SET idevent=NULL WHERE idfiche=".$row000['idfiche']; $rep001=$db4->query($req001); } }

It's common that the first line of a submission is bad. It's rare that the first 7 characters fill me with a sense of dread. $req000. Oh no. Oh noooo. We're talking about those kinds of variables.

We query using $req000 and store the result in $rep000, using $db4 to run the query. My skin is crawling so much from that that I feel like the obvious SQL injection vulnerability using $_GET to write the query is probably not getting enough of my attention. I really hate these variable names though.

We execute our gaping security vulnerability, and check how many rows we got (using $nb000 to store the result). While we have rows, we store each row in $row000, to populate $req001- an update query. We execute this query once for each row, storing the result in $rep001.

Now, the initial SELECT could return up to 4,000 rows. That's not a massive dataset, but as you can imagine, this whole application was running on a potato-powered server stuffed in the network closet. It was slow.

The fix was obvious- you could replace both the SELECT and the UPDATEs with a single query: UPDATE fiche SET idevent=NULL WHERE idevent=?- that's all this code actually does.

Fixing performance wasn't how Brian proved he was the right person for more contract work, though. Once Brian saw the SQL injection, he demonstrated to the boss how a malicious user could easily delete the entire database from the URL bar in their browser. The boss was sufficiently terrified by the prospect- the code guru was politely asked to leave, and Brian was told to please fix this quickly.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

Gabe enjoys it when clients request that he does updates on old software. For Gabe, it's exciting: you never know what you'll discover.

Public Sub AspJavaMessage(ByVal Message As String) System.Web.HttpContext.Current.Response.Write("<SCRIPT LANGUAGE=""JavaScript"">" & vbCrLf) System.Web.HttpContext.Current.Response.Write("alert(""" & Message & """)" & vbCrLf) System.Web.HttpContext.Current.Response.Write("</SCRIPT>") End Sub

This is server-side ASP .Net code.

Let's start with the function name: AspJavaMessage. We already know we're using ASP, or at least I hope we are. We aren't using Java, but JavaScript. I'm not confident the developer behind this is entirely clear on the difference.

Then we do a Response.Write to output some JavaScript, but we need to talk about the Response object a bit. In ASP .Net, you mostly receive your HttpResponse as part of the event that triggered your response. The only reason you'd want to access the HttpResponse through this long System.Web.HttpContext.Current.Response accessor is because you are in a lower-level module which, for some reason, hasn't been passed an HTTP response.

That's a long-winded way of saying, "This is a code smell, and this function likely exists in a layer that shouldn't be tampering with the HTTP response."

Then, of course, we have the ALL CAPS HTML tag, followed by a JavaScript alert() call, aka, the worst way to pop up notifications in a web page.

Ugly, awful, and hinting at far worse choices in the overall application architecture. Gabe certainly unearthed a… delightful treat.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

Let's say you had input in the form of field=value, and you wanted to pick that "value" part off. In C#, you'd likely just use String.Split and call it a day. But you're not RK's co-worker.

public string FilterArg(string value) { bool blAction; if (value.Contains('=')) blAction = false; else blAction = true; string tmpValue = string.Empty; foreach (char t in value) { if (t == '=') { blAction = true; } else if (t != ' ' && blAction == true) { tmpValue += t; } } return tmpValue; }

If the input contains an =, we set blAction to false. Then we iterate across our input, one character at a time. If the character we're on is an =, we set blAction to true. Otherwise, if the character we're on is not a space, and blAction is true, we append the current character to our output.

I opened by suggesting we were going to look at a home-grown split function, because at first glance, that's what this code looks like.

It does the job, but the mix of flags and loops and my inability to read sometimes makes it extra confusing to follow.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

"Personal Mountains" was hearing dire rumors about one of the other developers; rumors about both the quality of their work and their future prospects at the company. Fortunately for Personal Mountains, they never actually had to work with this person.

Unfortunately, that person was fired and 30,000 lines of code were now Personal Mountains' responsibility.

Fortunately, it's not really 30,000 lines of code.

list.DeleteColumn(61); list.DeleteColumn(60); list.DeleteColumn(59); list.DeleteColumn(58); list.DeleteColumn(57); list.DeleteColumn(56); list.DeleteColumn(55); list.DeleteColumn(54); list.DeleteColumn(53); list.DeleteColumn(52); list.DeleteColumn(51); list.DeleteColumn(50); list.DeleteColumn(49); list.DeleteColumn(48); list.DeleteColumn(47); list.DeleteColumn(46); list.DeleteColumn(45); list.DeleteColumn(44); list.DeleteColumn(43); list.DeleteColumn(42); list.DeleteColumn(41); list.DeleteColumn(40); list.DeleteColumn(39); list.DeleteColumn(38); list.DeleteColumn(37); list.DeleteColumn(36); list.DeleteColumn(35); list.DeleteColumn(34); list.DeleteColumn(33); list.DeleteColumn(32); list.DeleteColumn(31); list.DeleteColumn(30); list.DeleteColumn(29); list.DeleteColumn(28); list.DeleteColumn(27); list.DeleteColumn(26); list.DeleteColumn(25); list.DeleteColumn(24); list.DeleteColumn(23); list.DeleteColumn(22); list.DeleteColumn(21); list.DeleteColumn(20); list.DeleteColumn(19); list.DeleteColumn(18); list.DeleteColumn(17); list.DeleteColumn(16); list.DeleteColumn(15); list.DeleteColumn(14); list.DeleteColumn(13); list.DeleteColumn(12); list.DeleteColumn(11); list.DeleteColumn(10); list.DeleteColumn(9); list.DeleteColumn(8); list.DeleteColumn(7); list.DeleteColumn(6); list.DeleteColumn(5); list.DeleteColumn(4); list.DeleteColumn(3); list.DeleteColumn(2); list.DeleteColumn(1); list.DeleteColumn(0);

Comments in the code indicated that this was done for "extreme optimization", which leads me to believe someone heard about loop unrolling and decided to just do that everywhere there was a loop, without regard to whether or not it actually helped performance in any specific case, whether the loop was run frequently enough to justify the optimization, or understanding if the compiler might be more capable at deciding when and where to unroll a loop.

Within a few weeks, Personal Mountains was able to shrink the program from 30,000 lines of code to 10,000, with no measurable impact on its behavior or performance.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

It was an extraordinarily busy week at Error'd HQ. The submission list had an all-time record influx, enough for a couple of special edition columns. Among the list was an unusual PEBKAC. We don't get many of these so it made me chuckle and that's really all it takes to get a submission into the mix.

Headliner Lucio Crusca perseverated "Here's what I found this morning, after late night working yesterday, sitting on my couch, with my Thinkpad on my lap. No, it was not my Debian who error'd. I'm afraid it was me."

 

Eagle-eyed Ken S. called out Wells Fargo: "I see it just fine." Ditto.

 

Logical Mark W. insists "If you press 'Cancel', you are not cancelling; instead, you are cancelling the cancellation. Can we cancel this please?"

 

Peter pondered "Should I try to immediately delete, or is it safer to immediately delete?"

 

No relation to Ken S., Legal Eagle lamented "Due to my poor LTAS scores and borderline illetteracy, it was very hard for me to get into user_SchoolName. There is so much reading!" Hang in there, L. Eagle. With a resume like yours, I'm sure there will be work for you in Florida after graduation. Only the best!

 

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!