"Because 9.975 was just a *little* bit too small," explains our first anonymous helper.

 

Our second anonymous helper tells us "While looking up how to find my banks branch using a blank check, I came across this site that seems to have used AI to write their posts. Didn't expect to learn about git while reading about checks. I included the navbar because its just as bad."

 

Our third anonymous helper snickered "I guess I was just a bit over quota." Nicely done.

 

Our fourth anonymous helper isn't actually anonymous, alas. He signed off as the plausibly-named Vincent R, muttering "I dunno, it's all Greek to me. Or at least it *was* Greek until Firefox thoughtfully translated all the lambdas and mus and sigmas in these probability formulas..."

 

Finally for Friday, the fifth from Dan W. "On my way to the airport, I checked my route on the Trainline app. I think I'll have just enough time to make this connection in Wolverhampton." Walk, don't run.

 

[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.

Charles is supporting a PHP based application. One feature of the application is a standard "Contact Us" form. I'll let Charles take on the introduction:

While it looks fine on the outside, the code is a complete mess. The entire site is built with bad practices, redundant variables, poor validation, insecure cookie checks, and zero focus on maintainability or security. Even the core parts of the platform are a nightmare

We're going to take this one in chunks, because it's big and ugly.

try { if (isset($_POST)) { $name = $_POST['objName']; $lst_name = $_POST['objLstName']; $email = $_POST['objEmail']; $phone = $_POST['objGsm']; $message = $_POST['objMsg']; $verifycode = $_POST['objVerifyCode']; /******************************************************/ $objCmpT = $_POST['objCmpT']; $objCmpS = $_POST['objCmpS']; $objCountry = $_POST['objCountry']; $objCity = $_POST['objCity']; $objName2 = $_POST['objName2']; $objLstName2 = $_POST['objLstName2']; $objQuality = $_POST['objQuality']; $objEmail = $_POST['objEmail']; $objMsg2 = $_POST['objMsg2']; $objVerifyCode2 = $_POST['objVerifyCode2'];

I don't love that there's no structure or class here, to organize these fields, but this isn't bad, per se. We have a bunch of form fields, and we jam them into a bunch of variables. I am going to, with no small degree of willpower, not comment on the hungarian notation present in the field names. Look at me not commenting on it. I'm definitely not commenting on it. Look at me not commenting that some, but not all, of the variables also get the same hungarian prefix.

What's the point of hungarian notation when everything just gets the same thing anyway; like hungarian is always bad, but this is just USELESS

Ahem.

Let's continue with the code.

$ok = 0; $ok2 = 0; $sendTo = "example@example.com"; $golableMSG = ' -First Name & Last Name :' . $name . ' ' . $lst_name . ' -email :' . $email . ' -Phone Number : 0' . $phone . ' -Message : ' . $message; $globaleMSG2 = ' -First Name & Last Name :' . $objName2 . ' ' . $objLstName2 . ' -Email :' . $objEmail . ' -Type of company : ' . $objCmpT . ' -Sector of activity : ' . $objCmpS . ' -Country : ' . $objCountry . ' -City : ' . $objCity . ' -Your position within the company : ' . $objQuality . ' -Message : ' . $objMsg2;

We munge all those form fields into strings. These are clearly going to be the bodies of our messages. Only now I'm noticing that the user had to supply two different names- $name and $objName2. Extra points here, as I believe they meant to name both of these message variables globaleMSG but misspelled the first one, golableMSG.

Well, let's continue.

if (!$name) { $data['msg1'] = '*'; } else { $ok++; $data['msg1'] = ''; } if (!$lst_name) { $data['msg2'] = '*'; } else { $ok++; $data['msg2'] = ''; } if (!$email) { $data['msg3'] = '*'; } else { $ok++; $data['msg3'] = ''; } if ($phone <= 0) { $data['msg4'] = '*'; } else { $ok++; $data['msg4'] = ''; } if (!$message) { $data['msg5'] = '*'; } else { $ok++; $data['msg5'] = ''; } if (!$verifycode) { $data['msg6'] = '*'; } else { $ok++; $data['msg6'] = ''; } /*********************************************************************************/ if (!$objCmpS) { $data['msg7'] = '*'; } else { $ok2++; $data['msg7'] = ''; } if (!$objCountry) { $data['msg8'] = '*'; } else { $ok2++; $data['msg8'] = ''; } if (!$objCity) { $data['msg9'] = '*'; } else { $ok2++; $data['msg9'] = ''; } if (!$objName2) { $data['msg10'] = '*'; } else { $ok2++; $data['msg10'] = ''; } if (!$objLstName2) { $data['msg11'] = '*'; } else { $ok2++; $data['msg11'] = ''; } if (!$objQuality) { $data['msg12'] = '*'; } else { $ok2++; $data['msg12'] = ''; } if (!$objMsg2) { $data['msg13'] = '*'; } else { $ok2++; $data['msg13'] = ''; } if (!$objVerifyCode2) { $data['msg14'] = '*'; } else { $ok2++; $data['msg14'] = ''; }

What… what are we doing here? I worry that what I'm looking at here is some sort of preamble to verification code. But why is it like this? Why?

/********************************************************************************/ if ($ok == 6) { if (preg_match("/^[ a-z,.+!:;()-]+$/", $name)) { $data['msg1_1'] = ''; if (preg_match("/^[ a-z,.+!:;()-]+$/", $lst_name)) { $data['msg2_2'] = ''; $subject = $name . " " . $lst_name; if (filter_var($email, FILTER_VALIDATE_EMAIL)) { $data['msg3_3'] = ''; $from = $email; if (preg_match("/^[6-9][0-9]{8}$/", $phone)) { $data['msg4_4'] = ''; if (intval($verifycode) == intval($_COOKIE['nmbr1']) + intval($_COOKIE['nmbr2'])) { $data['msg6_6'] = ''; $headers = 'From: ' . $from . "\r\n" . 'Reply-To: ' . $sendTo . "\r\n" . 'X-Mailer: PHP/' . phpversion(); mail($sendTo, $subject, $golableMSG, $headers); $data['msgfinal'] = 'Votre Messsage est bien envoyer'; /*$data = array('success' => 'Votre Messsage est bien envoyer', 'postData' => $_POST);*/ } else { $data['msg6_6'] = 'votre resultat est incorrect'; } } else { $data['msg4_4'] = 'Votre Numéro est incorrect'; } } else { $data['msg3_3'] = 'Votre Email est incorrect'; } } else { $data['msg2_2'] = 'Votre Prénom est Incorrect'; } } else { $data['msg1_1'] = 'Votre Nom est Incorrect'; } }

Oh look, it is verification code. And right off the bat, we do things like prohibit "-" in names, which maybe isn't common in the Francophone world, but would be a great disappointment to Jean-Luc. Their verification code system, presumably to prevent spamming messages, is not particularly secure or useful. The real thing I see here, though, is the namespaced keys. Earlier, we set $data['msg1'], and now we're setting $data['msg1_1'] which is a code stench that could kill from a hundred yards.

And don't worry, we do the same thing for the other message we send:

/**************************************************************/ if ($ok2 == 8) { if (preg_match("/^[ a-z,.+!:;()-]+$/", $objName2)) { $data['msg10_10'] = ''; if (preg_match("/^[ a-z,.+!:;()-]+$/", $objLstName2)) { $data['msg11_11'] = ''; $subject2 = $objName2 . " " . $objLstName2; if (intval($objVerifyCode2) == intval($_COOKIE['nmbr3']) + intval($_COOKIE['nmbr4'])) { $from2 = $objEmail; $data['msg14_14'] = ''; $headers2 = 'From: ' . $from2 . "\r\n" . 'Reply-To: ' . $sendTo . "\r\n" . 'X-Mailer: PHP/' . phpversion(); mail($sendTo, $subject2, $globaleMSG2, $headers2); $data['msgfinal'] = 'Votre Messsage est bien envoyer'; } else { $data['msg14_14'] = 'votre resultat est incorrect'; } } else { $data['msg11_11'] = 'Votre Prénom est Incorrect'; } } else { $data['msg10_10'] = 'Votre Nom est Incorrect'; } }

Phew. Hey, remember way back at the top, when we checked to see if the $_POST variable were set? Well, we do have an else clause for that.

} else { throw new \Exception($mot[86]); }

Who doesn't love throwing messages by hard-coded array indexes in your array of possible error messages? Couldn't be bothered with a constant, could we? Nope, message 86 it is.

But don't worry about that exception going uncaught. Remember, this whole thing was inside of a try:

} catch (\Exception $e) { $data['msgfinal'] = "Votre Messsage n'est pas bien envoyer"; /*$data = array('danger' => 'Votre Messsage pas bien envoyer', 'postData' => $_POST);*/ }

Yeah, it didn't matter what message we picked, because we just catch the exception and hard-code out an error message.

Also, I don't speak French, but is "message" supposed to have an extra "s" in it?

Charles writes:

It’s crazy to see such sloppy work on a platform that seems okay at first glance. Honestly, this platform is the holy grail of messy code—it could have its own course on how not to code because of how bad and error-prone it is. There are also even worse scenarios of bad code, but it's too long to share, and honestly, they're too deep and fundamentally ingrained in the system to even begin explaining.

Oh, I'm sure we could explain it. The explanation may be "there was a severe and fatal lack of oxygen in the office, and this is what hypoxia looks like in code," but I'm certain there'd be an explanation.

.comment { border: none; } [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!

Once upon a time, web browsers weren't the one-stop-shop for all kinds of possible content that they are today. Aside from the most basic media types, your browser depended on content plugins to display different media types. Yes, there was an era where, if you wanted to watch a video in a web browser, you may need to have QuickTime or… (shudder) Real Player installed.

As a web developer, you'd need to write code to check which plugins were installed. If they don't have Adobe Acrobat Reader installed, there's no point in serving them up a PDF file- you'll need instead to give them an install link.

Which brings us to Ido's submission. This code is intended to find the Acrobat Reader plugin version.

acrobatVersion: function GetAcrobatVersion() { // Check acrobat is Enabled or not and its version acrobatVersion = 0; if (navigator.plugins && navigator.plugins.length) { for (intLoop = 0; intLoop <= 15; intLoop++) { if (navigator.plugins[intLoop] != -1) { acrobatVersion = parseFloat(navigator.plugins[intLoop].version); isAcrobatInstalled = true; break; } } } else {...} }

So, we start by checking for the navigator.plugins array. This is a wildly outdated thing to do, as the MDN is quite emphatic about, but I'm not going to to get hung up on that- this code is likely old.

But what I do want to pay attention to is that they check navigator.plugins.length. Then they loop across the set of plugins using a for loop. And don't use the length! They bound the loop at 15, arbitrarily. Why? No idea- I suspect it's for the same reason they named the variable intLoop and not i like a normal human.

Then they check to ensure that the entry at plugins[intLoop] is not equal to -1. I'm not sure what the expected behavior was here- if you're accessing an array out of bounds in JavaScript, I'd expect it to return undefined. Perhaps some antique version of Internet Explorer did something differently? Sadly plausible.

Okay, we've found something we believe to be a plugin, because it's not -1, we'll grab the version property off of it and… parseFloat. On a version number. Which ignores the fact that 1.1 and 1.10 are different versions. Version numbers, like phone numbers, are not actually numbers. We don't do arithmetic on them, treat them like text.

That done, we can say isAcrobatInstalled is true- despite the fact that we didn't check to see if this plugin was actually an Acrobat plugin. It could have been Flash. Or QuickTime.

Then we break out of the loop. A loop that, I strongly suspect, would only ever have one iteration, because undefined != -1.

So there we have it: code that doesn't do what it intends to, and even if it did, is doing it the absolute wrong way, and is also epically deprecated.

[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!

Sometimes, there's code so bad you simply know it's unused and never called. Bernard sends us one such method, in Java:

/** * Finds a <code>GroupEntity</code> by group number. * * @param group the group number. * @return the <code>GroupEntity</code> object. */ public static GroupEntity find(String group) { return GroupEntity.find(group); }

This is a static method on the GroupEntity class called find, which calls a static method on the GroupEntity class called find, which calls a static method on the GroupEntity class called find and it goes on and on my friend.

Clearly, this is a mistake. Bernard didn't supply much more context, so perhaps the String was supposed to be turned into some other type, and there's an overload which would break the recursion. Regardless, there was an antediluvian ticket on the backlog requesting that the feature to allow finding groups via a search input that no one had yet worked on.

I'm sure they'll get around to it, once the first call finishes.

.comment { border: none; } [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!

Simon recently found himself working alongside a "very senior" developer- who had a whopping 5 years of experience. This developer was also aggrieved that in recent years, Object Oriented programming had developed a bad reputation. "Functional this, functional that, people really just don't understand how clean and clear objects make your code."

For example, here are a few Java objects which they wrote to power a web scraping tool:

class UrlHolder { private String url; public UrlHolder(String url) { this.url = url; } } class UrlDownloader { private UrlHolder url; public String downloadPage; public UrlDownLoader(String url) { this.url = new UrlHolder(Url); } } class UrlLinkExtractor { private UrlDownloader url; public UrlLinkExtractor(UrlDownloader url) { this.url = url; } public String[] extract() { String page = Url.downloadPage; ... } }

UrlHolder is just a wrapper around string, but also makes that string private and provides no accessors. Anything shoved into an instance of that may as well be thrown into oblivion.

UrlDownloader wraps a UrlHolder, again, as a private member with no accessors. It also has a random public string called downloadPage.

UrlLinkExtractor wraps a UrlDownloader, and at least UrlLinkExtractor has a function- which presumably downloads the page. It uses UrlDownloader#downloadPage- the public string property. It doesn't use the UrlHolder, because of course it couldn't. The entire goal of this code is to pass a string to the extract function.

I guess I don't understand object oriented programming. I thought I did, but after reading this code, I don't.

[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.

...Screens of Death. Photos of failures in kiosk-mode always strike me as akin to the wizard being exposed behind his curtain. Yeah, that shiny thing is after all just some Windows PC on a stick. Here are a few that aren't particularly recent, but they're real.

Jared S. augurs ill: "Seen in downtown Mountain View, CA: In Silicon Valley AI has taken over. There is no past, there is no future, and strangely, even the present is totally buggered. However, you're free to restore the present if you wish."

 

Windows crashed Maurizio De Cecco's party and he is vexé. "Some OS just doesn’t belong in the parisian nightlife," he grumbled. But neither does pulled pork barbecue and yet there it is.

 

Máté cut Windows down cold. "Looks like the glaciers are not the only thing frozen at Matterhorn Glacier Paradise..."

 

Thomas found an installer trying to apply updates "in the Northwestern University's visitor welcome center, right smack in the middle of a nine-screen video display. I can only imagine why they might have iTunes or iCloud installed on their massive embedded display." I certainly can't.

 

Finally, Charles T. found a fast-food failure and was left entirely wordless. And hungry.

 

[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.

ArSo works at a small company. It's the kind of place that has one software developer, and ArSo isn't it. But ArSo is curious about programming, and has enough of a technical background that small tasks should be achievable. After some conversations with management, an arrangement was made: Kurt, their developer, would identify a few tasks that were suitable for a beginner, and would then take some time to mentor ArSo through completing them.

It sounded great, especially because Kurt was going to provide sample code which would give ArSo a head start on getting things done. What better way to learn than by watching a professional at work?

DateTime datTmp; File.Copy(strFileOld, strFileNew); // 2 seconds delay datTmp = DateTime.Now; while (datTmp.Second == DateTime.Now.Second); datTmp = DateTime.Now; while (datTmp.Second == DateTime.Now.Second); File.Delete(strFileOld);

This code copies a file from an old path to a new path, and then deletes the old path after a two second delay. Why is there a delay? I don't know. Why is the delay written like this? I can't possibly explain that.

Check the time at the start of the loop. When the second part of that time stops matching the second part of the current time, we assume one second has passed. This is, of course, inaccurate- if I check the time at 0:00:00.9999 a lot less than a second will pass. This delay is at most one second.

In any case, ArSo has some serious questions about Kurt's mentorship, and writes:

Now I don't know if I should ask for more coding tasks.

Honestly, I think you should ask for more. Like, I think you should just take Kurt's job. You may be a beginner, but honestly, you're likely going to do a better job than this.

[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!

Alicia recently inherited a whole suite of home-grown enterprise applications. Like a lot of these kinds of systems, it needs to do batch processing. She went tracking down a mysterious IllegalStateException only to find this query causing the problem:

select * from data_import where id > 10000

The query itself is fine, but the code calling it checks to see if this query returned any rows- if it did, the code throws the IllegalStateException.

First, of course, this should be a COUNT(*) query- no need to actually return rows here. But also… what? Why do we fail if there are any transactions with an ID greater than 10000? Why on Earth would we care?

Well, the next query it runs is this:

update data_import set id=id+10000

Oh. Oh no. Oh nooooo. Are they… are they using the ID to also represent some state information about the status of the record? It sure seems like it!

The program then starts INSERTing data, using a counter which starts at 1. Once all the new data is added, the program then does:

delete from data_import where id > 10000

All this is done within a single method, with no transactions and no error handling. And yes, this is by design. You see, if anything goes wrong during the inserts, then the old records don't get deleted, so we can see that processing failed and correct it. And since the IDs are sequential and always start at 1, we can easily find which row caused the problem. Who needs logging or any sort of exception handling- just check your IDs.

The underlying reason why this started failing was because the inbound data started trying to add more than 10,000 rows, which meant the INSERTs started failing (since we already had rows there for this). Alicia wanted to fix this and clean up the process, but too many things depended on it working in this broken fashion. Instead, her boss implemented a quick and easy fix: they changed "10000" to "100000".

[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.

Some time ago, poor Keith found himself working on an antique Classic ASP codebase. Classic ASP uses VBScript, which is like VisualBasic 6.0, but worse in most ways. That's not to say that VBScript code is automatically bad, but the language certainly doesn't help you write clean code.

In any case, the previous developer needed to make an 8 element array to store some data. Traditionally, in VBScript, you might declare it like so:

Dim params(8)

That's the easy, obvious way a normal developer might do it.

Keith's co-worker did this instead:

Dim params : params = Split(",,,,,,,", ",")

Yes, this creates an array using the Split function on a string of only commas. 7, to be exact. Which, when split, creates 8 empty substrings.

We make fun of stringly typed data a lot here, but this is an entirely new level of stringly typed initialization.

We can only hope that this code has finally been retired, but given that it was still in use well past the end-of-life for Classic ASP, it may continue to lurk out there, waiting for another hapless developer to stumble into its grasp.

[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!

Ross needed to write software to integrate with a credit card payment gateway. The one his company chose was relatively small, and only served a handful of countries- but it covered the markets they cared about and the transaction fees were cheap. They used XML for data interchange, and while they had no published schema document, they did have some handy-dandy sample code which let you parse their XML messages.

$response = curl_exec($ch); $authecode = fetch_data($response, '<authCode>', '</authCode>'); $responsecode = fetch_data($response, '<responsecode>', '</responsecode>'); $retrunamount = fetch_data($response, '<returnamount>', '</returnamount>'); $trxnnumber = fetch_data($response, '<trxnnumber>', '</trxnnumber>'); $trxnstatus = fetch_data($response, '<trxnstatus>', '</trxnstatus>'); $trxnresponsemessage = fetch_data($response, '<trxnresponsemessage>', '</trxnresponsemessage>');

Well, this looks… worrying. At first glance, I wonder if we're going to have to kneel before Z̸̭͖͔͂̀ā̸̡͖͕͊l̴̜͕͋͌̕g̸͉̳͂͊ȯ̷͙͂̐. What exactly does fetch_data actually do?

function fetch_data($string, $start_tag, $end_tag) { $position = stripos($string, $start_tag); $str = substr($string, $position); $str_second = substr($str, strlen($start_tag)); $second_positon = stripos($str_second, $end_tag); $str_third = substr($str_second, 0, $second_positon); $fetch_data = trim($str_third); return $fetch_data; }

Phew, no regular expressions, just… lots of substrings. This parses the XML document with no sense of the document's structure- it literally just searches for specific tags, grabs whatever is between them, and calls it done. Nested tags? Attributes? Self-closing tags? Forget about it. Since it doesn't enforce that your open and closing tags match, it also lets you grab arbitrary (and invalid) document fragments- fetch_data($response, "<fooTag>", "<barTag>"), for example.

And it's not like this needs to be implemented from scratch- PHP has built-in XML parsing classes. We could argue that by limiting ourselves to a subset of XML (which I can only hope this document does) and doing basic string parsing, we've built a much simpler approach, but I suspect that after doing a big pile of linear searches through the document, we're not really going to see any performance benefits from this version- and maintenance is going to be a nightmare, as it's so fragile and won't work for many very valid XML documents.

It's always amazing when TRWTF is neither PHP nor XML but… whatever this 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!

Amateur physicist B.J. is going on vacation, but he likes to plan things right down to the zeptosecond. "Assume the flight accelerates at a constant speed for the first half of the flight, and decelerates at the same rate for the second half. 1) What speed does the plane need to reach to have that level of time dilation? 2) What is the distance between the airports?"

 

Contrarily, Eddie R. was tired of vacation so got a new job, but right away he's having second thoughts. "Doing my onboarding, but they seem to have trouble with the idea of optional."

 

"Forget UTF-8! Have you heard about the new, hot encoding standard for 2024?!" exclaimed Daniel , kvetching "Well, if you haven't then Gravity Forms co. is going to change your mind: URLEncode everything now! Specially if you need to display some diacritics on your website. Throw away the old, forgotten UTF-8. Be a cool guy, just use that urlencode!"

 

Immediately afterward, Daniel also sent us another good example, this time from Hetzner. He complains "Hetzner says the value is invalid. Of course they won't say what is or isn't allowed. It wasn't the slash character, it was... a character with diacritics! Hetzner is clearly using US-ASCII created in 1960's."

 

Finally this week, we pulled something out of the archive from Boule de Berlin who wrote "Telekom, the biggest German ISP, shows email address validation is hard. They use a regex that limits the TLD part of an email address to 4 chars." Old but timeless.

 

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

Matt needed to add a new field to a form. This simple task was made complicated by the method used to save changes back to the database. Let's see if you can spot what the challenge was:

public int saveQualif(String docClass, String transcomId, String cptyCod, String tradeId, String originalDealId, String codeEvent, String multiDeal, String foNumber, String codeInstrfamily, String terminationDate, String premiumAmount, String premiumCurrency, String notionalAmount, String codeCurrency, String notionalAmount2, String codeCurrency2, String fixedRate, String payout, String maType, String maDate, String isdaZoneCode, String tradeDate, String externalReference, String entityCode, String investigationFileReference, String investigationFileStartDate, String productType, String effectiveDate, String expiryDate, String paymentDate, String settInstrucTyp, String opDirection, String pdfPassword, String extlSysCod, String extlDeaId, String agrDt) throws TechnicalException, DfException

That's 36 parameters right there. This function, internally, creates a data access object which takes just as many parameters in its constructor, and then does a check: if a field is non-null, it updates that field in the database, otherwise it doesn't.

Of course, every single one of those parameters is stringly typed, which makes it super fun. Tracking premiumAmount and terminationDate as strings is certainly never going to lead to problems. I especially like the pdfPassword being stored, which is clearly just the low-security password meant to be used for encrypting a transaction statement or similar: "the last 4 digits of your SSN" or whatever. So I guess it's okay that it's being stored in the clear in the database, but also I still hate it. Do better!

In any case, this function was called twice. Once from the form that Matt was editing, where every parameter was filled in. The second time, it was called like this:

int nbUpdates = incoming.saveQualif(docClass, null, null, null, null, null, multiDeal, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null);

As tempted as Matt was to fix this method and break it up into multiple calls or change the parameters to a set of classes or anything better, he was too concerned about breaking something and spending a lot of time on something which was meant to be a small, fast task. So like everyone who'd come before him, he just slapped in another parameter, tested it, and called it a day.

Refactoring is a problem for tomorrow's developer.

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

There's the potential for endless installments of "programmers not understanding how UUIDs work." Frankly, I think the fact that we represent them as human readable strings is part of the problem; sure, it's readable, but conceals the fact that it's just a large integer.

Which brings us to this snippet, from Capybara James.

if (!StringUtils.hasLength(uuid) || uuid.length() != 36) { throw new RequestParameterNotFoundException(ErrorCodeCostants.UUID_MANDATORY_OR_FORMAT); }

StringUtils.hasLength comes from the Spring library, and it's a simple "is not null or empty" check. So- we're testing to see if a string is null or empty, or isn't exactly 36 characters long. That tells us the input is bad, so we throw a RequestParameterNotFoundException, along with an error code.

So, as already pointed out, a UUID is just a large integer that we render as a 36 character string, and there are better ways to validate a UUID. But this also will accept any 36 character string- as long as you've got 36 characters, we'll call it a UUID. "This is valid, really valid, dumbass" is now a valid UUID.

With that in mind, I also like the bonus of it not distinguishing between whether or not the input was missing or invalid, because that'll make it real easy for users to understand why their input is getting rejected.

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

Since it's election day in the US, many people are thinking about counting today. We frequently discuss counting here, and how to do it wrong, so let's look at some code from RK.

This code may not be counting votes, but whatever it's counting, we're not going to enjoy it:

case LogMode.Row_limit: // row limit excel = 65536 rows if (File.Exists(personalFolder + @"\" + fileName + ".CSV")) { using (StreamReader reader = new StreamReader(personalFolder + @"\" + fileName + ".CSV")) { countRows = reader.ReadToEnd().Split(new char[] { '\n' }).Length; } }

Now, this code is from a rather old application, originally released in 2007. So the comment about Excel's row limit really puts us in a moment in time- Excel 2007 raised the row limit to 1,000,000 rows. But older versions of Excel did cap out at 65,536. And it wasn't the case that everyone just up and switched to Excel 2007 when it came out- transitioning to the new Office file formats was a conversion which took years.

But we're not even reading an Excel file, we're reading a CSV.

I enjoy that we construct the name twice, because that's useful. But the real magic of this one is how we count the rows. Because while Excel can handle 65,536 rows at this time, I don't think this program is going to do a great job of it- because we read the entire file into memory with ReadToEnd, then Split on newlines, then count the length that way.

As you can imagine, in practice, this performed terribly on large files, of which there were many.

Unfortunately for RK, there's one rule about old, legacy code: don't touch it. So despite fixing this being a rather easy task, nobody is working on fixing it, because nobody wants to be the one who touched it last. Instead, management is promising to launch a greenfield replacement project any day now…

[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.

For years, Victoria had a co-worker who "programmed by Google Search"; they didn't understand how anything worked, they simply plugged their problem into Google search and then copy/pasted and edited until they got code that worked. For this developer, I'm sure ChatGPT has been a godsend, but this code predates its wide use. It's pure "Googlesauce".

StringBuffer stringBuffer = new StringBuffer(); stringBuffer.append("SELECT * FROM TABLE1 WHERE COLUMN1 = 1 WITH UR"); String sqlStr = stringBuffer.toString(); ps = getConnection().prepareStatement(sqlStr); ps.setInt(1, code); rs = ps.executeQuery(); while (rs.next()) { count++; }

The core of this WTF isn't anything special- instead of running a SELECT COUNT they run a SELECT and then loop over the results to get the count. But it's all the little details in here which make it fun.

They start by using a StringBuffer to construct their query- not a horrible plan when the query is long, but this is just a single, simple, one-line query. The query contains a WITH clause, but it's in the wrong spot. Then they prepareStatement it, which does nothing, since this query doesn't contain any parameters (and also, isn't syntactically valid). Once it's prepared, they set the non-existent parameter 1 to a value- this operation will throw an exception because there are no parameters in the query.

Finally, they loop across the results to count.

The real WTF is that this code ended up in the code base, somehow. The developer said, "Yes, this seems good, I'll check in this non-functional blob that I definitely don't understand," and then there were no protections in place to keep that from happening. Now it falls to more competent developers, like Victoria, to clean up after this co-worker.

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

"Check out Visual Studio optimizing their rating system to only include the ratings used," shared Fiorenzo R. Imagine the performance gain!

 

"This sounds about right," says Colin A.

 

"Wow! Must snap up some sweet Anker kit with this amazing offer; but less than four days to go!" exclaims Dave L., who then goes on to explain
"The actual WTF is this though. I sent this image to Anker with this email: But only 3days left? I hope this offer continues!
Anker replied: Thank you for your feedback! I understand that you appreciate the savings on the Anker SOLIX PS100 Portable Solar Panel and wish the offer could be extended beyond the current 3-day limit. Your suggestion is valuable and will be considered for future promotions to enhance customer satisfaction. If you have any other requests or need further assistance, please let me know.
I for one welcome our new AI overlords. "

 

Graham F. almost stashed this away for later. "Looks like Dropbox could use a few lessons in how to do Maths! Although maybe their definition of 'almost' differs from mine."

 

Finally Joshua found time to report a brand-new date-handling bug. "Teams is so buggy; this one just takes the cake. I had to check with the unix cal program to make sure I wasn't completely bonkers." For the readers, November 8 this year is supposed to be a Friday. I suppose things could change after the US election.

 

Have a great weekend. Maybe I'll see you next Friday, or maybe all the weekdays will be renamed Thursday. [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!

Horror movies, as of late, have gone to great lengths to solve the key obstacle to horror movies- cell phones. When we live in a world where help is a phone call away, it's hard to imagine the characters not doing that. So screenwriters put them in situations where this is impossible: in Midsommar they isolate them in rural Sweden, in Get Out calling the police is only going to put our protagonist in more danger. But what's possibly more common is making the film a period piece- like the X/Pearl/Maxxxine trilogy, Late Night with the Devil, or Netflix's continuing series of R.L. Stine adaptations.

I bring this up, because today's horror starts in 1993. A Norwegian software company launched its software product to mild acclaim. Like every company, it had its ups and downs, its successes and missteps. On the surface, it was a decent enough place to work.

Over the years, the company tried to stay up to date with technology. In 1993, the major languages one might use for launching a major software product, your options are largely C or Pascal. Languages like Python existed, but weren't widely used or even supported on most systems. But the company stayed in business and needed to update their technology as time passed, which meant the program gradually grew and migrated to new languages.

Which meant, by the time Niklas F joined the company, they were on C#. Even though they'd completely changed languages, the codebase still derived from the original C codebase. And that meant that the codebase had many secrets, dark corners, and places a developer should never look.

Like every good horror movie protagonist, Niklas heard the "don't go in there!" and immediately went in there. And lurking in those shadows was the thing every developer fears the most: homebrew date handling code.

/// <summary> /// /// </summary> /// <param name="dt"></param> /// <returns></returns> public static DateTime LastDayInMonth(DateTime dt) { int day = 30; switch (dt.Month) { case 1: day = 31; break; case 2: if (IsLeapYear(dt)) day = 29; else day = 28; break; case 3: day = 31; break; case 4: day = 30; break; case 5: day = 31; break; case 6: day = 30; break; case 7: day = 31; break; case 8: day = 31; break; case 9: day = 30; break; case 10: day = 31; break; case 11: day = 30; break; case 12: day = 31; break; } return new DateTime(dt.Year, dt.Month, day, 0, 0, 0); } /// <summary> /// /// </summary> /// <param name="dt"></param> /// <returns></returns> public static bool IsLeapYear(DateTime dt) { bool ret = (((dt.Year % 4) == 0) && ((dt.Year % 100) != 0) || ((dt.Year % 400) == 0)); return ret; }

For a nice change of pace, this code isn't incorrect. Even the leap year calculation is actually correct (though my preference would be to just return the expression instead of using a local variable). But that's what makes this horror all the more insidious: there are built-in functions to handle all of this, but this code works and will likely continue to work, just sitting there, like a demon that we've made a pact with. And suddenly we realize this isn't Midsommar but Ari Aster's other hit film, Hereditary, and we're trapped being in a lineage of monsters, and can't escape our inheritance.

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

Once again, we take a look at the traditional "if (boolean) return true; else return false;" pattern. But today's, from RJ, offers us a bonus twist.

public override bool IsValid { get { if (!base.IsValid) return false; return true; } }

As promised, this is a useless conditional. return base.IsValid would do the job just as well. Except, that's the twist, isn't it. base is our superclass. We're overriding a method on our superclass to… just do what the base method does.

This entire function could just be deleted. No one would notice. And yet, it hasn't been. Everyone agrees that it should be, yet it hasn't been. No one's doing it. It just sits there, like a pimple, begging to be popped.

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

Jon recently started a new project. When setting up his dev environment, one of his peers told him, "You can disable verbose logging by setting DEBUG_LOG=false in your config file."

Well, when Jon did that, the verbose logging remained on. When he asked his peers, they were all surprised to see that the flag wasn't turning off debug logging. "Hunh, that used to work. Someone must have changed something…" Everyone had enough new development to do that tracking down a low priority bug fell to Jon. It didn't take long.

const DEBUG_LOG = process.env.DEBUG_LOG || true

According to the blame, the code had been like this for a year, the commit crammed with half a dozen features, was made by a developer who was no longer with the company, and the message was simply "Debugging". Presumably, this was intended to be a temporary change that accidentally got committed and no one noticed or cared.

Jon fixed it, and moved on. There was likely going to be plenty more to find.

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

Quality control is an important business function for any company. When your company is shipping devices with safety concerns, it's even more important. In some industries, a quality control failure is bound to be national headlines.

When the quality control software tool stopped working, everyone panicked. At which point, GRH stepped in.

Now, we've discussed this software and GRH before, but as a quick recap, it was:

written by someone who is no longer employed with the company, as part of a project managed by someone who is no longer at the company, requested by an executive who is also no longer at the company. There are no documented requirements, very few tests, and a lot of "don't touch this, it works".

And this was a quality control tool. So we're already in bad shape. It also had been unmaintained for years- a few of the QC engineers had tried to take it over, but weren't programmers, and it had essentially languished.

Specifically, it was a quality control tool used to oversee the process by about 50 QC engineers. It automates a series of checks by wrapping around third party software tools, in a complex network of "this device gets tested by generating output in program A, feeding it to program B, then combining the streams and sending them to the device, but this device gets tested using programs D, E, and F."

The automated process using the tool has a shockingly low error rate. Without the tool, doing things manually, the error rate climbs to 1-2%. So unless everyone wanted to see terrifying headlines in the Boston Globe about their devices failing, GRH needed to fix the problem.

GRH was given the code, in this case a a zip file on a shared drive. It did not, at the start, even build. After fighting with the project configuration to resolve that, GRH was free to start digging in deeper.

Public Sub connect2PCdb() Dim cPath As String = Path.Combine(strConverterPath, "c.pfx") Dim strCN As String ' JES 12/6/2016: Modify the following line if MySQL server is changed to a different server. A dump file will be needed to re-create teh database in the new server. strCN = "metadata=res://*/Model1.csdl|res://*/Model1.ssdl|res://*/Model1.msl;provider=MySql.Data.MySqlClient;provider connection string='server=REDACTED;user id=REDACTED;database=REDACTED;sslmode=Required;certificatepassword=REDACTED;certificatefile=REDACTED\c.pfx;password=REDACTED'" strCN = Regex.Replace(strCN, "certificatefile=.*?pfx", "certificatefile=" & cPath) pcContext = New Entities(strCN) strCN = "metadata=res://*/Model1.csdl|res://*/Model1.ssdl|res://*/Model1.msl;provider=MySql.Data.MySqlClient;provider connection string='server=REDACTED;user id=REDACTED;persistsecurityinfo=True;database=REDACTED;password=REDACTED'" strCN = Regex.Match(strCN, ".*'(.*)'").Groups(1).Value Try strCN = pcContext.Database.Connection.ConnectionString cnPC.ConnectionString = "server=REDACTED;user id=REDACTED;password=REDACTED;database=REDACTED;" cnPC.Open() Catch ex As Exception End Try End Sub

This is the code which connects to the backend database. The code is in the category of more of a trainwreck than a WTF. It's got a wonderful mix of nonsense in here, though- a hard-coded connection string which includes plaintext passwords, regex munging to modify the string, then hard-coding a string again, only to use regexes to extract a subset of the string. A subset we don't use.

And then, for a bonus, the whole thing has a misleading comment- "modify the following line" if we move to a different server? We have to modify several lines, because we keep copy/pasting the string around.

Oh, and of course, it uses the pattern of "open a database connection at application startup, and just hold that connection forever," which is a great way to strain your database as your userbase grows.

The good news about the hard-coded password is that it got GRH access to the database. With that, it was easy to see what the problem was: the database was full. The system was overly aggressive with logging, the logs went to database tables, the server was an antique with a rather small hard drive, and the database wasn't configured to even use all of that space anyway.

Cleaning up old logs got the engineers working again. GRH kept working on the code, though, cleaning it up and modernizing it. Updating to latest version of the .NET Core framework modified the data access to be far simpler, and got rid of the need for hard-coded connection strings. Still, GRH left the method looking like this:

Public Sub connect2PCdb() 'Dim cPath As String = Path.Combine(strConverterPath, "c.pfx") 'Dim strCN As String ' JES 12/6/2016: Modify the following line if MySQL server is changed to a different server. A dump file will be needed to re-create teh database in the new server. 'strCN = "metadata=res://*/Model1.csdl|res://*/Model1.ssdl|res://*/Model1.msl;provider=MySql.Data.MySqlClient;provider connection string='server=REDACTED;user id=REDACTED;database=REDACTED;sslmode=Required;certificatepassword=REDACTED;certificatefile=REDACTED\c.pfx;password=REDACTED'" 'strCN = Regex.Replace(strCN, "certificatefile=.*?pfx", "certificatefile=" & cPath) 'pcContext = New Entities(strCN) 'strCN = "metadata=res://*/Model1.csdl|res://*/Model1.ssdl|res://*/Model1.msl;provider=MySql.Data.MySqlClient;provider connection string='server=REDACTED;user id=REDACTED;persistsecurityinfo=True;database=REDACTED;password=REDACTED'" 'strCN = Regex.Match(strCN, ".*'(.*)'").Groups(1).Value 'GRH 2021-01-15. Connection information moved to App.Config 'GRH 2021-08-13. EF Core no longer supports App.Config method pcContext = New PcEntities Try ' GRH 2021-08-21 This variable no longer exists in .NET 5 'strCN = pcContext.Database.Connection.ConnectionString ' GRH 2021-08-20 Keeping the connection open causes EF Core to not work 'cnPC.ConnectionString = "server=REDACTED;user id=REDACTED;password=REDACTED;database=REDACTED;SslMode=none" 'cnPC.Open() Catch ex As Exception End Try End Sub

It's now a one-line method, with most of the code commented out, instead of removed. Why on Earth is the method left like that?

GRH explains:

Yes, I could delete the function as it is functionally dead, but I keep it for the same reasons that a hunter mounts a deer's head above her mantle.

[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!