Skip to main content

CodeSOD: The Last Last Name

19 hours 20 minutes ago

Sometimes, you see some code which is perfectly harmless, but illustrates an incredibly dangerous person behind them. The code isn't good, but it isn't bad in any meaningful way, but they were written by a cocaine addled pomeranian behind the controls of a bulldozer: it's full of energy, doesn't know exactly what's going on, and at some point, it's going to hit something important.

Such is the code which Román sends us.

public static function registerUser($name, $lastName, $username, ...) { // 100% unmodified first lines, some comments removed $tsCreation = new DateTime(); $user = new User(); $name = $name; $lastname = $lastName; $username = $username; $user->setUsername($username); $user->setLastname($lastname); $user->setName($name); // And so on. }

This creates a user object and populates its fields. It doesn't use a meaningful constructor, which is its own problem, but that's not why we're here. We're here because for some reason the developer behind this function assigns some of the parameters to themselves. Why? I don't know, but it's clearly the result of some underlying misunderstanding of how things work.

But the real landmine is the $lastname variable- which is an entirely new variable which has slightly different capitalization from $lastName.

And you've all heard this song many times, so sing along with the chorus: "this particular pattern shows up all through the codebase," complete with inconsistent capitalization.

.comment { border: none; } [Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!
Remy Porter

CodeSOD: And Config

1 day 19 hours ago

It's not unusual to store format templates in your application configuration files. I'd argue it's probably a good and wise thing to do. But Phillip inherited a C# application from a developer woh "abandoned" it, and there were some choices in there.

<appSettings> <add key="xxxurl" value="[http://{1}:7777/pls/xxx/p_pristjek?i_type=MK3000{0}i_ean={3}{0}i_style=http://{2}/Content/{0}i_red=http://{2}/start.aspx/]http://{1}:7777/pls/xxx/p_pristjek?i_type=MK3000{0}i_ean={3}{0}i_style=http://{2}/Content/{0}i_red=http://{2}/start.aspx"/> </appSettings>

Okay, I understand that this field contains URLs, but I don't understand much else about what's going on here. It's unreadable, but also, it has some URLs grouped inside of a [] pair, but others which aren't, and why oh why does the {0} sigil keep showing up so much?

Maybe it'll make more sense after we fill in the template?

var url = string.Format(xxxUrl, "&", xxxIp, srvUrl, productCode);

Oh. It's an "&". Because we're constructing a URL query string, which also seems to contain URLs, which I suspect is going to have some escaping issues, but it's for a query string.

At first, I was wondering why they did this, but then I realized: they were avoiding escape characters. By making the ampersand a formatting parameter, they could avoid the need to write &amp; everywhere. Which… I guess this is a solution?

Not a good solution, but… a solution.

I still don't know why the same URL is stored twice in the string, once surrounded by square brackets and once not, and I don't think I want to know. Only bad things can result from knowing that.

[Advertisement] Plan Your .NET 9 Migration with Confidence
Your journey to .NET 9 is more than just one decision.Avoid migration migraines with the advice in this free guide. Download Free Guide Now!
Remy Porter

CodeSOD: It's Not Wrong to Say We're Equal

2 days 19 hours ago

Aaron was debugging some C# code, and while this wasn't the source of the bug, it annoyed him enough to send it to us.

protected override int DoCompare(Item item1, Item item2) { try { DateTime thisDate = ((DateField)item1.Fields["Create Date"]).DateTime; DateTime thatDate = ((DateField)item2.Fields["Create Date"]).DateTime; return thatDate.CompareTo(thisDate); } catch (Exception) { return 0; // Sorry, ran out of budget! } }

Not to be the pedantic code reviewer, but the name of this function is terrible. Also, DoCompare clearly should be static, but this is just pedantry.

Now, there's a lot of implied WTFs hidden in the Item class. They're tracking fields in a dictionary, or maybe a ResultSet, but I don't think it's a ResultSet because they're converting it to a DateField object, which I believe to be a custom type. I don't know what all is in that class, but the whole thing looks like a mess and I suspect that there are huge WTFs under that.

But we're not here to look at implied WTFs. We're here to talk about that exception handler.

It's one of those "swallow every error" exception handlers, which is always a "good" start, and it's the extra helpful kind, which returns a value that is likely incorrect and provides no indication that anything failed.

Now, I suspect it's impossible for anything to have failed- as stated, this seems to be some custom objects and I don't think anything is actively talking to a database in this function (but I don't know that!) so the exception handler likely never triggers.

But hoo boy, does the comment tell us a lot about the codebase. "Sorry, ran out of budget!". Bugs are inevitable, but this is arguably the worst way to end up with a bug in your code: because you simply ran out of money and decided to leave it broken. And ironically, I suspect the code would be less broken if you just let the exception propagate up- if nothing else, you'd know that something failed, instead of incorrectly thinking two dates were the same.

.comment { border: none; } [Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!
Remy Porter

CodeSOD: A Highly Paid Field

3 days 19 hours ago

In ancient times, Rob's employer didn't have its own computer; it rented time on a mid-range computer and ran all its jobs using batch processing in COBOL. And in those ancient times, these stone tools were just fine.

But computing got more and more important, and the costs for renting time kept going up and up, so they eventually bought their own AS/400. And that meant someone needed to migrate all of their COBOL to RPG. And management knew what you do for those kinds of conversions: higher a Highly Paid Consultant.

On one hand, the results weren't great. On the other, the code is still in use, though has been through many updates and modernizations and migrations in that time. Still, the HPC's effects can be felt, like this block, which hasn't been touched since she was last here:

// CHECK FOR VALID FIELD IF FIELD1 <> *BLANKS AND FIELD1 < '1' AND FIELD1 > '5'; BadField1 = *ON; LEAVESR; ENDIF;

This is a validation check on a field (anonymized by Rob), but the key thing I want you to note is that what the field stores are numbers, but it stores those numbers as text- note the quotes. And the greater-than/less-than operators will do lexical comparisons on text, which means '21' < '5' is true.

The goal of this comparison was to require the values to be between 1 and 5. But that's not what it's enforcing. The only good(?) news is that this field also isn't used. There's one screen where users can set the value, but no one has- it's currently blank everywhere- and nothing else in the system references the value. Which raises the question of why it's there at all.

But those kinds of questions are par for the course for the HPC. When they migrated a bunch of reports and the users compared the results with the original versions, the results didn't balance. The HPC's explanation? "The users are changing the data to make me look bad."

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

Error'd: Button, button, who's got the button?

6 days 19 hours ago

Wikipedia describes the (very old) English children's game. I wonder if there's a similar game in Germany. In any case, the Worcester News is definitely confused about how this game is played.

Martin I. explains "This is a cookie acceptance dialog. It seems to struggle with labeling the buttons when the user's browser is not set to English ..."

 

In Dutch, Robert R. is playing a different game. "Duolingo is teaching users more than just languages - apparently web development fundamentals are included when HTML entities leak into the user interface. That's one way to make " " part of your vocabulary!" We wonder why the webdev would want to use a nbsp in this location.

 

Ninja Squirrel shares a flubstitution nugget. "Since I've been waiting a long time for a good deal on a new gaming keyboard and the Logitech Play Days started today, I thought I'd treat myself. I wasn't prepared for what Logitech then treated me to - free gifts and wonderful localization errors in the productive WebShop. What started with a simple “Failed to load resource [Logitech.checkout.Total]” in the order overview ended with this wonderful total failure after the order was placed. What a sight to behold - I love it! XD"

 

David P. imagines that Tesla's web devs are allowed near embedded systems. "If Tesla can't even do dates correctly, imagine how much fun Full Self Driving is." Given how often FSD has been promised imminently, I conclude that date confusion is simply central to the corporate culture. Embrace it.

 

But it's not only Tesla that bungles whens. Neil T. nails another big name. "Has Google's Gemini AI hallucinated a whole new calendar? I'm pretty sure the Gregorian calendar only has 30 days in June."

 

And that's it for this week. Next Friday is definitely not June

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

Classic WTF: NoeTimeToken

1 week ago
Maybe we'll just try and read a book. That's a good way to spend your vacation. This can't possibly go badly! Original --Remy

"Have you had a chance to look at that JIRA ticket yet?"

Marge debated pretending she hadn't seen the Slack message yet—but, if she did, she knew Gary would just walk over to her desk and badger her further. In truth, she didn't want to look at the ticket: it was a low priority ticket, and worse, it only affected a small fraction of one client's customers, meaning it was likely to be some weird edge case bug nobody would ever run into again. Maybe if I ignore it long enough, it'll go away on its own, she thought.

The client was a bookseller with a small but signifigant-to-them online presence; the software they used to sell books, including your standard e-commerce account functionality, was made by Marge's company. The bug was somewhere in the password reset feature: some customers, seemingly at random, were unable to use the password reset link the software emailed out.

Marge pulled up the ticket, looking over the half-hearted triage work that had been done before it landed on her desk to solve. The previous guy had pulled logs and figured out that all the customers who were complaining were using the same ISP based out of Germany. He'd recommended reaching out to them, but had been transferred to another division before he'd gotten around to it.

When Marge realized that the contact information was all in German, she almost gave up then and there. But with the magic of Google Translate, she managed to get in touch with a representative via email. After a bit of back and forth, she noticed this gem in one of his (translated) replies:

We want to display mails in our webmail client as close to the original as possible. Since most mails are HTML formatted, the client supports the full HTTP protocol and can display (almost) all HTML tags. Unfortunately, this means that "evil" JS-Content in such mails can do all kinds of stuff in the browser and therefore on the customer's PC.

To avert this, all mails are processed by a "SafeBrowsing"-module before they are displayed, to recognize and circumvent such manipulations. One of those security measures is the recognition of js-modules that begin with "on...", since that are mostly js functions that are triggered by some event in the browser. Our "countermeasure" is to just replace "on..." with "no..." before the HTML content is sent to the rendering process.

Marge frowned at the answer for a bit, something nagging at her mind. "There's no way," she murmured as she pulled up the access logs. Sure enough, the url for the reset link was something like https://bookseller.com?oneTimeToken=deadbeef ... and the customers in question had accessed https://bookseller.com?noeTimeToken=deadbeef instead.

A few lines of code and it was resolved: a conditional would check for the incorrect query string parameter and copy the token to the correct query string parameter instead. Marge rolled her eyes, merged her change into the release branch, and finally, at long last, closed that annoying low-priority ticket once and for all.

hljs.initHighlightingOnLoad(); code { font-family: Consolas, monospace; } [Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.
Jane Bailey

CodeSOD: Classic WTF: When it's OK to GOTO

1 week 1 day ago
Where did you GOTO on your vacation? Nowhere. GOTO is considered harmful. Original --Remy

Everybody knows that you should never use "goto" statements. Well, except in one or two rare circumstances that you won't come across anyway. But even when you do come across those situations, they're usually "mirage cases" where there's no need to "goto" anyway. Kinda like today's example, written by Jonathan Rockway's colleague. Of course, the irony here is that the author likely tried to use "continue" as his label, but was forced to abbreviate it to "cont" in order to skirt compiler "reserved words" errors.

while( sysmgr->getProcessCount() != 0 ) { // Yes, I realize "goto" statements are considered harmful, // but this is a case where it is OK to use them cont: //inactivation is not guaranteed and may take up to 3 calls sysmgr->CurrentProcess()->TryInactivate(); if( sysmgr->CurrentProcess()->IsActive() ) { Sleep(DEFAULT_TIMEOUT); goto cont; } /* ED: Snip */ //disconnect child processes if( sysmgr->CurrentProcess()->HasChildProcesses() ) { /* ED: Snip */ } /* ED: Snip */ if( sysmgr->CurrentProcess()->IsReusable() ) { sysmgr->ReuseCurrentProcess(); goto cont; } sysmgr->CloseCurrentProcess(); }

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

Classic WTF: The Core Launcher

1 week 2 days ago
As our vacation continues, we might want to maybe play some video games. What could possibly go wrong? Original --Remy

“You R haccking files on my computer~!!!” Charles Carmichael read in a newly-submitted support ticket, “this is illigle and I will sue your whoal compiny. But first I will tell every1 nevar to buy youre stupid game agin.”

The bizarre spelling and vague threats were par for the course. After all, when you market and sell a game to the general public, you can expect a certain percentage of bizarre and vague customer communications. When that game is a popular MMPORG (no, not that one), that percentage tends to hover around the majority.

It took a few days to see the pattern, but the string of emails started to make sense. “Uh, when did your game become spyware?” said one email. “Are you doing this just to force us to play more often?” another customer asked. “I know you have a lot of AI and whatnot, so I think it leaked out. Because now my whole computer wants me to play all the time… like my dog bringing me his chew toy.”

As it turned out, the problem started happening a few days after an update to the core launcher was published. The core launcher was one of those terrifically handy executables that could download all of the assets for any single game that was published, scan them for completeness, replace bad or missing files, and then launch the game itself after the user signed in. It’s a must-have for any modern multiplayer online game.

This core launcher could also patch itself. Updates to this executable were fairly rare, but had to be made whenever a new title launched, as was recently the case. Obviously, a large battery of automated and manual testing is done to ensure that there are no problems after publishing, yet something seemed to have slipped through the cracks… at least for some customers.

After a whole lot of back and forth with customers, Chris was able to compile dozens of detailed process lists, startup program launches, newly installed applications, and firewall usage rules. As he pored over the collected information, one program was always there. It was Interfersoft’s fairly popular anti-virus suite.

It took a solid two days of research, but Chris was finally able to uncover the new “feature” in Interfersoft’s Advanced Firewall Protector that was causing the problems. Like many similar anti-virus suites, when a program wanted to use network services, Interfersoft would pop-up a dialog confirming that the program’s operation was authorized. Behind the scenes, if the user allowed the program, Interfersoft would make a hash of that executable file, and would allow its communications to pass through the firewall every time thereafter.

Users who had this antivirus solution installed had, at one time, allowed the launcher through their firewall. The first time they connected to the game server after the launcher patch was released, their executable would download its patch, apply it to itself, and restart itself. But then of course, the executable hash didn’t match any more, and the program was no longer able to go through the firewall.

Rather than asking users if they wanted to allow the program to connect to the internet, in the new version of Interfersoft’s suite, the anti-virus system would rename the executable and move it. The logic being that, if it was changed after connecting to the internet, it was probably malware.

But what did they name the file? Program.exe. Unless that was already taken, then they would name it Progra~1.exe or Progra~2.exe and so forth. And where did they place this file? Well, in the root directory of C of course!

This naming convention, as it turned out, was a bad idea. Back in the very old, Windows 3 days, Windows did not support long file names. It wasn’t until Windows NT 3.5.1 (and then Windows 95 later) that long file names were supported. Prior to this, there were a lot of limitations on what characters could be part of a filename or directory, one of those being a space.

In fact, any space in a shell command execution was seen to be an argument. This made sense at the time so you could issue a command like this:

C:\DOOM\doom.exe -episode 3

That, of course, would start Doom at episode 3. However, when Microsoft switched to Long File Names, it still had to support this type of invocation. So, the way the windows cmd.exe shell works is simple. You pass it a string like this:

C:\Program Files\id Software\Doom\Doom.exe -nomusic

And it will try to execute “C:\Program” as a file, passing it “Files\id Software\Doom\Doom.exe -nomusic” as argument to that executable. Of course, this program doesn’t exist, so it will then try to execute “C:\Program Files\id”, passing it “Software\Doom\Doom.exe -nomusic” as argument. If this doesn’t exist, it will try to execute “C:\Program Files\id Software\Doom\Doom.exe” passing in “-nomusic” as an argument. It would continue this way until a program existed and started, or until the path was depleted and no program was to be found.

And on top of all this, desktop shortcuts on Windows are mostly just invocations of the shell, with the actual location of the executable you want to start (the path) stored in text inside the shortcut. When you click it, it reads this path, and passes it to the shell to start up the program. And this is why Intersoft’s process of moving files to the root directory was the worst decision they could have made.

Most of the programs installed in Windows at this time were installed to the “Program Files” directory by default. This was a folder in the root (C:\) directory. So when you wanted to launch, for instance, Microsoft Word, the shortcut on your Desktop pointed to “C:\Program Files\Microsoft\Office\Word.exe” or Firefox, which was in “C:\Program Files\Mozilla\Firefox\”. But thanks to Program.exe in the root directory, you ended up doing this:

C:\Program.exe “Files\Microsoft\Office\Word.exe”

and

C:\Program.exe “Files\Mozilla\Firefox\”

So, when users were trying to launch their application – applications which resided in the Program Files directory on their C drive – they were getting the launcher instead.

Chris explained all of this in great detail to Interfersoft, all the while explaining to customers how to fix the problem with the firewall. It helped some, but several hundred customers ended up closing their accounts a direct result of the “hacking”.

A few weeks later, Interfersoft started responding to the issues with their customers. Fortunately (for them), they decided to not use their own auto-update process to deliver a new version of the firewall.

[Advertisement] Plan Your .NET 9 Migration with Confidence
Your journey to .NET 9 is more than just one decision.Avoid migration migraines with the advice in this free guide. Download Free Guide Now!
Alex Papadimoulis

Classic WTF: Take the Bus

1 week 3 days ago
It's summer break time, here at TDWTF, and based on this classic, we shouldn't be traveling by bus. Original --Remy

Rachel started working as a web developer for the local bus company. The job made her feel young, since the buses, the IT infrastructure, and most of their back-office code was older than she was. The bus fare-boxes were cash only, and while you could buy a monthly pass, it was just a little cardboard slip that you showed the driver. Their accounting system ran on a mainframe, their garage management software was a 16-bit DOS application. Email ran on an Exchange 5.5 server.

In charge of all of the computing systems, from the web to DOS, was Virgil, the IT director. Virgil had been hired back when the accounting mainframe was installed, and had nestled into his IT director position like a tick. The bus company, like many such companies in the US, was ostensibly a private company, but chartered and subsidized by the city. This created a system which had all the worst parts of private-sector and public-sector employment merged together, and Virgil was the master of that system.

Rachel getting hired on was one of his rare “losses”, and he wasn’t shy about telling her so.

“I’ve been doing the web page for years,” Virgil said. “It has a hit counter, so you can see how many hits it actually gets- maybe 1 or 2 a week. But management says we need to have someone dedicated to the website.” He grumbled. “Your salary is coming out of my budget, you know.”

That website was a FrontPage 2000 site, and the hit-counter was broken in any browser that didn’t have ActiveX enabled. Rachel easily proved that there was far more traffic than claimed, not that there was a lot. And why should there be? You couldn’t buy a monthly pass online, so the only feature was the ability to download PDFs of the hand-schedules.

With no support, Rachel did her best to push things forward. She redesigned the site to be responsive. She convinced the guy who maintained their bus routes (in a pile of Excel spreadsheets) to give her regular exports of the data, so she could put the schedules online in a usable fashion. Virgil constantly grumbled about wasting money on a website nobody used, but as she made improvements, more people started using it.

Then it was election season. The incumbent mayor had been complaining about the poor service the bus company was offering, the lack of routes, the costs, the schedules. His answer was, “cut their funding”. Management started talking about belt-tightening, Virgil started dropping hints that Rachel was on the chopping block, and she took the hint and started getting resumes out.

A miracle occurred. The incumbent mayor’s campaign went off the rails. He got caught siphoning money from the city to pay for private trips. A few local cops mentioned that they’d been called in to cover-up the mayor’s frequent DUIs. His re-election campaign’s finances show strange discrepancies, and money had come in that couldn’t be tied back to a legitimate contribution. He tried to get a newly built stadium named after himself, which wasn’t illegal, but was in poor taste and was the final straw. He dropped out of the election, paving the way for “Mayor Fred” to take over.

Mayor Fred was a cool Mayor. He wanted to put in bike lanes. He wanted to be called “Mayor Fred”. He wanted to make it easier for food trucks to operate in the city. And while he shared his predecessor’s complaints about the poor service from the bus company, he had a different solution, which he revealed while taking a tour of the bus company’s offices.

“I’m working right now to secure federal grants, private sector funding, to fund a modernization project,” Mayor Fred said, grinning from behind a lectern. “Did you know we’re paying more to keep our old buses on the road for five years than it would cost to buy new buses?” And thus, Mayor Fred made promises. Promises about new buses, promises about top-flight consultants helping them plan better routes, promises about online functionality.

Promises that made Virgil grumble and whine. Promises that the mayor… actually kept.

New buses started to hit the streets. They had GPS and a radio communication system that gave them up-to-the-second location reporting. Rachel got put in charge of putting that data on the web, with a public API, and tying it to their schedules. A group of consultants swung through to help, and when the dust settled, Rachel’s title was suddenly “senior web developer” and she was in charge of a team of 6 people, integrating new functionality to the website.

Virgil made his opinion on this subject clear to her: “You are eating into my budget!”

“Isn’t your budget way larger?” Rachel asked.

“Yes, but there’s so much more to spend it on! We’re a bus company, we should be focused on getting people moving, not giving them pretty websites with maps that tell them where the buses are! And now there’s that new FlashCard project!”

FlashCard was a big project that didn’t involve Rachel very much. Instead of cash fares and cardboard passes, they were going to get an RFID system. You could fill your card at one of the many kiosks around the city, or even online. “Online” of course, put it in Rachel’s domain, but it was mostly a packaged product. Virgil, of all people, had taken over the install and configuration, Rachel just customized the stylesheet so that it looked vaguely like their main site.

Rachel wasn’t only an employee of the bus company, she was also a customer. She was one of the first in line to get a FlashCard. For a few weeks, it was the height of convenience. The stop she usually needed had a kiosk, she just waved her card at the farebox and paid. And then, one day, when her card was mostly empty and she wasn’t anywhere near a kiosk, she decided to try filling her card online.

Thank you for your purchase. Your transaction will be processed within 72 hours.

That was a puzzle. The kiosks completed the transaction instantly. Why on Earth would a website take 3 days to do the same thing? Rachel became more annoyed when she realized she didn’t have enough on her card to catch the bus, and she needed to trudge a few blocks out of her way to refill the card. That’s when it started raining. And then she missed her bus, and had to wait 30 minutes for the next one. Which is when the rain escalated to a downpour. Which made the next bus 20 minutes late.

Wet, cold, and angry, Rachel resolved to figure out what the heck was going on. When she confronted Virgil about it, he said, “That’s just how it works. I’ve got somebody working full time on keeping that system running, and that’s the best they can do.”

Somebody working full time? “Who? What? Do you need help? I’ve done ecommerce before, I can-”

“Oh no, you’ve already got your little website thing,” Virgil said. “I’m not going to let you try and stage a coup over this.”

With an invitation like that, Rachel decided to figure out what was going on. It wasn’t hard to get into the administration features of the FlashCard website. From there, it was easy to see the status of the ecommerce plugin for processing transactions: “Not installed”. In fact, there was no sign at all that the system could even process transactions at all.

The only hint that Rachel caught was the configuration of the log files. They were getting dumped to /dev/lp1. A printer. Next came a game of hide-and-seek- the server running the FlashCard software wasn’t in their tiny data-center, which meant she had to infer its location based on which routers were between her and it. It took a few days of poking around their offices, but she eventually found it in the basement, in an office.

In that office was one man with coke-bottle glasses, an antique continuous feed printer, a red document shredder, and a FlashCard kiosk running in diagnostic mode. “Um… can I help you?” the man asked.

“Maybe? I’m trying to track down how we’re processing credit card transactions for the FlashCard system?”

The printer coughed to life, spilling out a new line. “Well, you’re just in time then. Here’s the process.” He adjusted his glasses and peered at the output from the printer:

TRANSACTION CONFIRMED: f6ba779d22d5;4012888888881881;$25.00

The man then kicked his rolly-chair over to the kiosk. The first number was the FlashCard the transaction was for, the second was the credit card number, and the third was the amount. He punched those into the kiosk’s keypad, and then hit enter.

“When it gets busy, I get real backed up,” he confessed. “But it’s quiet right now.”

Rachel tracked down Virgil, and demanded to know what he thought he was doing.

“What? It’s not like anybody wants to use a website to buy things,” Virgil said. “And if we bought the ecommerce module, the vendor would have charged us $2,000/mo, on top of an additional transaction fee. This is cheaper, and I barely have enough room in my budget as it is!”

[Advertisement] Plan Your .NET 9 Migration with Confidence
Your journey to .NET 9 is more than just one decision.Avoid migration migraines with the advice in this free guide. Download Free Guide Now!
Remy Porter

Error'd: Colophony

1 week 6 days ago

Just a quick note this week: I discovered that many people have been sending in submissions for this column and designating them for CodeSod by mistakes. Consequently, there is an immense backlog of material from which to choose. An abundance of riches! We will be seeing some older items in the future. For today, a collection of colons:

Bill NoLastName , giving away clues to his banking security questions online: "If had known there was a limit, I would have changed my daughter's middle name. I've been caught by this before - my dad has only a middle initial (no middle name)."

 

Gordon F. heard of a greal deal: "This is the first mention of shipping on a hearing aids website. Tough choice."

 

Michael P. underlines a creative choice: "I got an email from a recruiter about a job opening. I'm a little confused about the requirements."

 

Cole T. pretend panics about pennies (and maybe we need an article about false urgency, hm): "Oh no! My $0 in rewards are about to expire!"

 

Finally, bibliophile WeaponizedFun (alsonolastname) humblebrags erudition. It ain't War & Peace, but it's still an ordeal! "After recently finishing The Brothers Karamazov after 33 hours on audio disc, I was a bit surprised to see that Goodreads is listing this as my longest book with only 28 pages. 28 discs, maybe, but I still am questioning their algorithm, because this just looks silly."

 

[Advertisement] Picking up NuGet is easy. Getting good at it takes time. Download our guide to learn the best practice of NuGet for the Enterprise.
Lyle Seaman

CodeSOD: Using the Old Bean

2 weeks ago

If you write a lot of Java, you're going to end up writing a lot of getters and setters. Without debating the merits of loads of getters and setters versus bare properties, ideally, getters and setters are the easiest code to write. Many IDEs will just generate them for you! How can you screw up getters and setters?

Well, Dave found someone who could.

private ReportDatesDao reportDatesDao; @Resource(name = CensusDao.BEAN_NAME) public void setAuditDao(CensusDao censusDao) { this.reportDatesDao = reportDatesDao; }

The function is called setAuditDao, takes a CensusDao input, but manipulates reportDatesDao, because clearly someone copy/pasted and didn't think about what they were doing.

The result, however, is that this just sets this.reportDatesDao equal to itself.

I'm always impressed by code which given the chance to make multiple decisions makes every wrong choice, even if it is just lazy copy/paste.

[Advertisement] Plan Your .NET 9 Migration with Confidence
Your journey to .NET 9 is more than just one decision.Avoid migration migraines with the advice in this free guide. Download Free Guide Now!
Remy Porter

CodeSOD: Stop Being So ####

2 weeks 1 day ago

Many a network admin has turned to the siren song of Perl to help them automate managing their networks. Frank's predecessor is no exception.

They also got a bit combative about people critiquing their Perl code:

# COMPLEX SUBNET MATH # Looking up a value in an array was faster than any mathematical solution. Yes, it's hard coded, but these values won't ever change anyway. Stop being so #### about it. $Subnets = @("0.0.0.0","128.0.0.0","192.0.0.0","224.0.0.0","240.0.0.0","248.0.0.0","252.0.0.0","254.0.0.0","255.0.0.0","255.128.0.0","255.192.0.0","255.224.0.0","255.240.0.0","255.248.0.0","255.252.0.0","255.254.0.0","255.255.0.0","255.255.128.0","255.255.192.0","255.255.224.0","255.255.240.0","255.255.248.0","255.255.252.0","255.255.254.0","255.255.255.0","255.255.255.128","255.255.255.192","255.255.255.224","255.255.255.240","255.255.255.248","255.255.255.252","255.255.255.254","255.255.255.255")

I believe them when they say that the lookup array is faster, but it leaves me wondering: what are they doing where performance matters that much?

I don't actually think this ascends to the level of a WTF, but I do think the defensive comment is funny. Clearly, the original developer was having a time with people complaining about it.

Frank notes that while Perl has a reputation as a "write only language," this particular set of scripts was actually quite easy to read and maintain. So yes, I guess we should stop being so #### about it.

[Advertisement] Plan Your .NET 9 Migration with Confidence
Your journey to .NET 9 is more than just one decision.Avoid migration migraines with the advice in this free guide. Download Free Guide Now!
Remy Porter

CodeSOD: A Second Date

2 weeks 2 days ago

Ah, bad date handling. We've all seen it. We all know it. So when Lorenzo sent us this C# function, we almost ignored it:

private string GetTimeStamp(DateTime param) { string retDate = param.Year.ToString() + "-"; if (param.Month < 10) retDate = retDate + "0" + param.Month.ToString() + "-"; else retDate = retDate + param.Month.ToString() + "-"; if (param.Day < 10) retDate = retDate + "0" + param.Day.ToString() + " "; else retDate = retDate + param.Day.ToString() + " "; if (param.Hour < 10) retDate = retDate + "0" + param.Hour.ToString() + ":"; else retDate = retDate + param.Hour.ToString() + ":"; if (param.Minute < 10) retDate = retDate + "0" + param.Minute.ToString() + ":"; else retDate = retDate + param.Minute.ToString() + ":"; if (param.Second < 10) retDate = retDate + "0" + param.Second.ToString() + "."; else retDate = retDate + param.Second.ToString() + "."; if (param.Millisecond < 10) retDate = retDate + "0" + param.Millisecond.ToString(); else retDate = retDate + param.Millisecond.ToString(); return retDate; }

Most of this function isn't terribly exciting. We've seen this kind of bad code before, but even when we see a repeat like this, there are still special treats in it. Look at the section for handling milliseconds: if the number is less than 10, they pad it with a leading zero. Just the one, though. One leading zero should be enough for everybody.

But that's not the thing that makes this code special. You see, there's another function worth looking at:

private string FileTimeStamp(DateTime param) { string retDate = param.Year.ToString() + "-"; if (param.Month < 10) retDate = retDate + "0" + param.Month.ToString() + "-"; else retDate = retDate + param.Month.ToString() + "-"; if (param.Day < 10) retDate = retDate + "0" + param.Day.ToString() + " "; else retDate = retDate + param.Day.ToString() + " "; if (param.Hour < 10) retDate = retDate + "0" + param.Hour.ToString() + ":"; else retDate = retDate + param.Hour.ToString() + ":"; if (param.Minute < 10) retDate = retDate + "0" + param.Minute.ToString() + ":"; else retDate = retDate + param.Minute.ToString() + ":"; if (param.Second < 10) retDate = retDate + "0" + param.Second.ToString() + "."; else retDate = retDate + param.Second.ToString() + "."; if (param.Millisecond < 10) retDate = retDate + "0" + param.Millisecond.ToString(); else retDate = retDate + param.Millisecond.ToString(); return retDate; }

Not only did they fail to learn the built-in functions for formatting dates, they forgot about the functions they wrote for formatting dates, and just wrote (or realistically, copy/pasted?) the same function twice.

At least both versions have the same bug with milliseconds. I don't know if I could handle it if they were inconsistent about that.

[Advertisement] Picking up NuGet is easy. Getting good at it takes time. Download our guide to learn the best practice of NuGet for the Enterprise.
Remy Porter

CodeSOD: The Firefox Fix

2 weeks 3 days ago

Yitzchak was going through some old web code, and found some still in-use JavaScript to handle compatibility issues with older Firefox versions.

if ($.browser.mozilla && $.browser.version.slice(0, 1) == '1') { … }

What a marvel. Using JQuery, they check which browser is reported- I suspect JQuery is grabbing this from the user-agent string- and then its version. And if the version has a 1 in its first digit, we apply a "fix" for "compatibility".

I guess it's a good thing there will never be more than 9 versions of Firefox. I mean, what version are they on now? Surely the version number doesn't start with a "1", nor has it started with a "1" for some time, right?

[Advertisement] Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.
Remy Porter

Error'd: Squaring the Circle

2 weeks 6 days ago

Time Lord Jason H. has lost control of his calendar. "This is from my credit card company. A major company you have definitely heard of and depending upon the size of the area you live in, they may even have a bank branch near you. I've reloaded the page and clicked the sort button multiple times to order the rows by date in both ascending and descending order. It always ends up the same. May 17th and 18th happened twice, but not in the expected order." I must say that it is more fun when we know who they are.

 

A job hunter with the unlikely appelation full_name suggested titling this "[submission_title]" which seems appropriate.

 

"The browser wars continue to fall out in HTML email," reports Ben S. "Looking at the source code of this email, it was evidently written by & for Microsoft products (including <center> tags!), and the author likely never saw the non-Microsoft version I'm seeing where only a haphazard assortment of the links are styled. But that doesn't explain why it's AN ELEVEN POINT SCALE arranged in a GRID."

 

"The owl knows who you are," sagely stated Jan. "This happens when you follow someone back. I love how I didn't have to anonymize anything in the screenshot."

 

"Location, location, location!" crows Tim K. who is definitely not a Time Lord. "Snarky snippet: Found while cleaning up miscellaneous accounts held by a former employee. By now we all know to expect how these lists are sorted, but what kind of sadist *created* it? Longer explanation: I wasn't sure what screenshot to send with this one, it just makes less and less sense the more I look at it, and no single segment of the list contains all of the treasures it hides. "America" seems to refer to the entire western hemisphere, but from there we either drill down directly to a city, or sometimes to a US state, then a city, or sometimes just to a country. The only context that indicates we're talking about Jamaica the island rather than Jamaica, NY is the timezone listed, assuming we can even trust those. Also, that differentiator only works during DST. There are eight entries for Indiana. There are TEN entries for the Antarctic."

Well.
In this case, there is a perfectly good explanation. TRWTF is time zones, that's all there is to it. These are the official IANA names as recorded in the public TZDB. In other words, this list wasn't concocted by a mere sadist, oh no. This list was cooked up by an entire committee! If you have the courage, you can learn more than you ever wanted to know about time at the IANA time zones website

 

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

CodeSOD: Gridding My Teeth

3 weeks ago

Dan's co-workers like passing around TDWTF stories, mostly because seeing code worse than what they're writing makes them feel less bad about how often they end up hacking things together.

One day, a co-worker told Dan: "Hey, I think I found something for that website with the bad code stories!"

Dan's heart sank. He didn't really want to shame any of his co-workers. Fortunately, the source-control history put the blame squarely on someone who didn't work there any more, so he felt better about submitting it.

This is another ASP .Net page, and this one made heavy use of GridView elements. GridView controls applied the logic of UI controls to generating a table. They had a page which contained six of these controls, defined like this:

<asp:GridView ID="gvTaskMonth1" runat="server" CssClass="leadsGridView" AutoGenerateColumns="False" OnRowDataBound="gvTaskMonth1_RowDataBound"> ... </asp:GridView> <asp:GridView ID="gvTaskMonth2" runat="server" CssClass="leadsGridView" AutoGenerateColumns="False" OnRowDataBound="gvTaskMonth1_RowDataBound"> ... </asp:GridView> <asp:GridView ID="gvTaskMonth3" runat="server" CssClass="leadsGridView" AutoGenerateColumns="False" OnRowDataBound="gvTaskMonth1_RowDataBound"> ... </asp:GridView>

The purpose of this screen was to display a roadmap of coming tasks, broken up by how many months in the future they were. The first thing that leaps out to me is that they all use the same event handler for binding data to the table, which isn't in-and-of-itself a problem, but the naming of it is certainly a recipe for confusion.

Now, to bind these controls to the data, there needed to be some code in the code-behind of this view which handled that. That's where the WTF lurks:

/// <summary> /// Create a roadmap for the selected client /// </summary> private void CreateRoadmap() { for (int i = 1; i < 7; i++) { switch (i) { case 1: if (gvTaskMonth1.Rows.Count > 0) { InsertTasks(gvTaskMonth1, DateTime.Parse(txtDatePeriod1.Text), "1"); } break; case 2: if (gvTaskMonth2.Rows.Count > 0) { InsertTasks(gvTaskMonth2, DateTime.Parse(txtDatePeriod2.Text), "2"); } break; case 3: if (gvTaskMonth3.Rows.Count > 0) { InsertTasks(gvTaskMonth3, DateTime.Parse(txtDatePeriod3.Text), "3"); } break; case 4: if (gvTaskMonth4.Rows.Count > 0) { InsertTasks(gvTaskMonth4, DateTime.Parse(txtDatePeriod4.Text), "4"); } break; case 5: if (gvTaskMonth5.Rows.Count > 0) { InsertTasks(gvTaskMonth5, DateTime.Parse(txtDatePeriod5.Text), "5"); } break; case 6: if (gvTaskMonth6.Rows.Count > 0) { InsertTasks(gvTaskMonth6, DateTime.Parse(txtDatePeriod6.Text), "6"); } break; } } }

Ah, the good old fashioned loop-switch sequence anti-pattern. I understand the motivation: "I want to do the same thing for six different controls, so I should use a loop to not repeat myself," but then couldn't quite figure out how to do that, so they just repeated themselves, but inside of a loop.

The "fix" was to replace all of this with something more compact:

private void CreateRoadmap() { InsertTasks(gvTaskMonth1, DateTime.Parse(txtDatePeriod1.Text), "1"); InsertTasks(gvTaskMonth2, DateTime.Parse(txtDatePeriod2.Text), "2"); InsertTasks(gvTaskMonth3, DateTime.Parse(txtDatePeriod3.Text), "3"); InsertTasks(gvTaskMonth4, DateTime.Parse(txtDatePeriod4.Text), "4"); InsertTasks(gvTaskMonth5, DateTime.Parse(txtDatePeriod5.Text), "5"); InsertTasks(gvTaskMonth6, DateTime.Parse(txtDatePeriod6.Text), "6"); }

That said, I'd recommend not trying to parse date times inside of a text box inside of this method, but that's just me. Bubbling up the inevitable FormatException that this will generate is going to be a giant nuisance. It's likely that they've got a validator somewhere, so it's probably fine- I just don't like it.

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

Credit Card Sins

3 weeks 1 day ago

Our anonymous submitter, whom we'll call Carmen, embarked on her IT career with an up-and-coming firm that developed and managed eCommerce websites for their clients. After her new boss Russell walked her around the small office and introduced her to a handful of coworkers, he led her back to his desk to discuss her first project. Carmen brought her laptop along and sat down across from Russell, poised to take notes.

Russell explained that their newest client, Sharon, taught CPR classes. She wanted her customers to be able to pay and sign up for classes online. She also wanted the ability to charge customers a fee in case they cancelled on her.

"You're gonna build a static site to handle all this," he said.

Carmen nodded along as she typed out notes in a text file.

"Now, Sharon doesn't want to pay more than a few hundred dollars for the site," Russell continued, "so we're not gonna hook up an endpoint to use a service-provided API for payments."

Carmen glanced up from her laptop, perplexed. "How are we gonna do it, then?"

"Via email," Russell replied smoothly. "The customer will enter their CC info into basic form fields. When they click Submit, you're gonna send all that to Sharon's business address, and also CC it to yourself for backup and recovery purposes."

Carmen's jaw dropped. "Just ... straight-up email raw credit card data?"

"Yep!" Russell replied. "Sharon knows to expect the emails."

Her heart racing with panic, Carmen desperately cast about for some way for this to be less awful. "Couldn't ... couldn't we at least encrypt the CC info before we send it to her?"

"She's not paying us for that," Russell dismissed. "This'll be easier to implement, anyway! You can handle it, can't you?"

"Yyyes—"

"Great! Go get started, let me know if you have any more questions."

Carmen had plenty of questions and even more misgivings, but she'd clearly be wasting her time if she tried to bring them up. There was no higher boss to appeal to, no coworkers she knew well enough who could slip an alternate suggestion into Russell's ear on her behalf. She had no choice but to swallow her good intentions and implement it exactly the way Russell wanted it. Carmen set up the copied emails to forward automatically to a special folder so that she'd never have to look at them. She cringed every time a new one came in, reflecting on how lucky Sharon and her customers were that the woman supporting her website had a conscience.

And then one day, a thought came to Carmen that really scared her: in how many places, in how many unbelievable ways, was her sensitive data being treated like this?

Eventually, Carmen moved on to bigger and better things. Her first project most likely rests in the hands of Russell's newest hire. We can only hope it's an honest hire.

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

CodeSOD: The Pirate's Code

3 weeks 2 days ago

We've talked about ASP .Net WebForms in the past. In this style of development, everything was event driven: click a button, and the browser sends an HTTP request to the server which triggers a series of events, including a "Button Click" event, and renders a new page.

When ASP .Net launched, one of the "features" was a lazy repaint in browsers which supported it (aka, Internet Explorer), where you'd click the button, the page would render on the server, download, and then the browser would repaint only the changed areas, making it feel more like a desktop application, albeit a laggy one.

This model didn't translate super naturally to AJAX style calls, where JavaScript updated only portions of the page. The .Net team added some hooks for it- special "AJAX enabled" controls, as well as helper functions, like __doPostBack, in the UI to generate URLs for "postbacks" to trigger server side execution. A postback is just a POST request with .NET specific state data in the body.

All this said, Chris maintains a booking system for a boat rental company. Specifically, he's a developer at a company which the boat rental company hires to maintain their site. The original developer left behind a barnacle covered mess of tangled lines and rotting hull.

Let's start with the view ASPX definition:

<script> function btnSave_Click() { if (someCondition) { //Trimmed for your own sanity //PostBack to Save Data into the Database. javascript:<%#getPostBack()%>; } else { return false; } } </script> <html> <body> <input type="button" value=" Save Booking " id="btnSave" class="button" title="Save [Alt]" onclick="btnSave_Click()" /> </body> </html>

__doPostBack is the .NET method for generating URLs for performing postbacks, and specifically, it populates two request fields: __EVENTTARGET (the ID of the UI element triggering the event) and __EVENTARGUMENT, an arbitrary field for your use. I assume getPostBack() is a helper method which calls that. The code in btnSave_Click is as submitted, and I think our submitter may have mangled it a bit in "trimming", but I can see the goal is to ensure than when the onclick event fires, we perform a "postback" operation with some hard-coded values for __EVENTTARGET and __EVENTELEMENT.

Or maybe it isn't mangled, and this code just doesn't work?

I enjoy that the tool-tip "title" field specifies that it's "[Alt]" text, and that the name of the button includes extra whitespace to ensure that it's padded out to a good rendering size, instead of using CSS.

But we can skip past this into the real meat. How this gets handled on the server side:

Protected Sub Page_Load(ByVal sender As Object, ByVal e As System.EventArgs) Handles Me.Load '// Trimmed more garbage If Page.IsPostBack Then 'Check if save button has been Clicked. Dim eventArg As String = Request("__EVENTARGUMENT") Dim offset As Integer = eventArg.IndexOf("@@@@@") If (offset > -1) Then 'this is an event that we raised. so do whatever you need to here. Save() End If End If End Sub

From this, I conclude that getPostBack populates the __EVENTARGUMENT field with a pile of "@", and we use that to recognize that the save button was clicked. Except, and this is the important thing, if they populated the ID property with btnSave, then ASP .Net would automatically call btnSave_Click. The entire point of the __doPostBack functionality is that it hooks into the event handling pattern and acts just like any other postback, but lets you have JavaScript execute as part of sending the request.

The entire application is a boat with multiple holes in it; it's taking on water and going down, and like a good captain, Chris is absolutely not going down with it and looking for a lifeboat.

Chris writes:

The thing in its entirety is probably one of the biggest WTFs I've ever had to work with.
I've held off submitting because nothing was ever straight forward enough to be understood without posting the entire website.

Honestly, I'm still not sure I understand it, but I do hate it.

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

CodeSOD: A Real POS Report

3 weeks 3 days ago

Eddie's company hired a Highly Paid Consultant to help them retool their systems for a major upgrade. Of course, the HPC needed more and more time, and the project ran later and later and ended up wildly over budget, so the HPC had to be released, and Eddie inherited the code.

What followed was a massive crunch to try and hit absolutely hard delivery dates. Management didn't want their team "rewriting" the expensive code they'd already paid for, they just wanted "quick fixes" to get it live. Obviously, the HPC's code must be better than theirs, right?

After release, a problem appeared in one of their sales related reports. The point-of-sale report was meant to deliver a report about which items were available at any given retail outlet, in addition to sales figures. Because their business dealt in a high volume of seasonal items, every quarter the list of items was expected to change regularly.

The users weren't seeing the new items appear in the report. This didn't make very much sense- it was a report. The data was in the database. The report was driven by a view, also in the database, which clearly was returning the correct values? So the bug must be in the code which generated the report…

If POSItemDesc = "Large Sign" Then grdResults.Columns.FromKey("FColumn12").Header.Caption = "Large Sign" End If If POSItemDesc = "Small Sign" Then grdResults.Columns.FromKey("FColumn12").Header.Caption = "Small Sign" End If If POSItemDesc = "2x2 Hanging Sign" Then grdResults.Columns.FromKey("FColumn12").Header.Caption = "2x2 Hanging Sign" End If If POSItemDesc = "1x1 Sign" Then grdResults.Columns.FromKey("FColumn12").Header.Caption = "1x1 Sign" End If '.........Snipping more of these........ If POSItemDesc = "Light Thief" Then grdResults.Columns.FromKey("FColumn12").Header.Caption = "Light Thief" End If If POSItemDesc = "Door Strike" Then grdResults.Columns.FromKey("FColumn12").Header.Caption = "Door Strike" End If

First, it's worth noting that inside of the results grid display item, the HPC named the field FColumn12, which is such a wonderfully self documenting name, I'm surprised we aren't all using that everywhere. But the more obvious problem is that the list of possible items is hard-coded into the report; items which don't fit one of these if statements don't get displayed.

At no point, did the person writing this see the pattern of "I check if a field equals a string, and then set another field equal to that string," and say, "maybe there's a better way?" At no point, in the testing process, did anyone try this report with a new item?

It was easy enough for Eddie to change the name of the column in the results grid, and replace all this code with a simpler: grdResults.Columns.FromKey("POSItem").Header.Caption = POSItemDesc, which also had the benefit of actually working, but we're all left puzzling over why this happened in the first place. It's not like the HPC was getting paid per line of code. Right? Right?

Of course not- no HPC would willingly be paid based on any metric that has an objective standard, even if the metric is dumb.

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

Error'd: There's No Place Like

3 weeks 6 days ago

... London!
This week, we're showcasing some multiple submissions from two regular participants who fell into the theme. Everybody else is just going to have to wait for their turn next week.

Frist up it's Daniel D. "I wanted to see events for the dates I would be in London. Is Skiddle (the website in question) telling me I should come to London more often?" They're certainly being very generous with their interpretation of dates.

 

But wait, there's more! Daniel follows with a variation: "Skiddle here again - let's choose June 7th to June 14th, but Skiddle knows better and sets the dates to June 6th to June 13th."

 

"I was not aware the Berlin to London route passes through Hawaii (which is Mokulele'shome turf)" chuckles our old friend Michael R. He seems to believe it's an Error'd but I think the real WTF is simply the Byzantine tapestry of partnerships, resellers, rebranding, whitelabeling and masquerades in the air transport biz.

 

"Maybe it's just a Monday morning thing," he reports from the airport.

 

But Monday had everybody troubled, and Michael was already thinking of Friday. "I am so sure I took the Circle Line just last Friday. And the other lines have the option Monday-Friday/Saturday/Sunday." I hope there isn't a subtext here.

 

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.
Lyle Seaman
Checked
50 minutes 43 seconds ago
Curious Perversions in Information Technology
Subscribe to The Daily WTF feed