We previously discussed some whitespacing choices in a C++ codebase. Tim promised that there were more WTFs lurking in there, and has delivered one.

Let's start with this class constructor:

QBatch_arithExpr::QBatch_arithExpr(QBatch_unOp, const QBatch_snippet &, const QBatch_snippet &);

You'll notice that this takes a parameter of type QBatch_unOp. What is that type? Well, it's an enumerated type describing the kind of operation this arithExpr represents. That is to say, they're not using real inheritance, but instead switching on the QBatch_unOp value to decide which code branch to execute- hand-made, home-grown artisanal inheritance. And while there are legitimate reasons to avoid inheritance, this is a clear case of "is-a" relationships, and it would allow compile-time checking of how you combine your types.

Tim also points out the use of the "repugnant west const", which is maybe a strong way to word it, but definitely using only "east const" makes it a lot easier to understand what the const operator does. It's worth noting that in this example, the second parameters is a const reference (not a reference to a const value).

Now, they are using inheritance, just not in that specific case:

class QBatch_paramExpr : public QBatch_snippet {...};

There's nothing particularly wrong with this, but we're going to use this parameter expression in a moment.

QBatch_arithExpr* Foo(QBatch_snippet *expr) { // snip QBatch_arithExpr *derefExpr = new QBatch_arithExpr(enum_tag1, *(new QBatch_paramExpr(paramId))); assert(derefExpr); return new QBatch_arithExpr(enum_tag2, *expr, *derefExpr); }

Honestly, in C++ code, seeing a pile of "*" operators and raw pointers is a sign that something's gone wrong, and this is no exception.

Let's start with calling the QBatch_arithExpr constructor- we pass it *(new QBatch_paramExpr(paramId)), which is a multilayered "oof". First, the new operator will heap allocate and construct an object, and return a pointer to that object. We then dereference that pointer, and pass the value as a reference to the constructor. This is an automatic memory leak; because we never trap the pointer, we never have the opportunity to release that memory. Remember kids, in C/C++ you need clear ownership semantics and someone needs to be responsible for deallocating all of the allocated memory- every new needs a delete, in this case.

Now, new QBatch_arithExpr(...) will also return a pointer, which we put in derefExpr. We then assert on that pointer, confirming that it isn't null. Which… it can't be. A constructor may fail and throw an exception, but you'll never get a null (now, I'm sure a sufficiently motivated programmer can mix nothrow and -fno-exceptions to get constructors to return null, but that's not happening here, and shouldn't happen anywhere).

Then we dereference that pointer and pass it to QBatch_arithExpr- creating another memory leak. Two memory leaks in three lines of code, where one line is an assert, is fairly impressive.

Elsewhere in the code, shared_pointer objects are used, wit their names aliased to readable types, aka QBatch_arithExpr::Ptr, and if that pattern were followed here, the memory leaks would go away.

As Tim puts it: "Some folks never quite escaped their Java background," and in this case, I think it shows. Objects are allocated with new, but never deleted, as if there's some magical garbage collector which is going to find the unused objects and free them.

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

Marco found this wreck, left behind by a former co-worker:

$("#image_sample").html('<i><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br />No image selected, select an image to see how it looks in the banner!</i>');

This code uses the JQuery library to find an element in the web page with the ID "image_sample", and then replaces its contents with this hard-coded blob of HTML.

I really appreciate the use of self-closing, XHTML style BR tags, which was a fad between 2000 and 2002, but never truly caught on, and was basically forgotten by the time HTML5 dropped. But this developer insisted that self-closing tags were the "correct" way to write HTML.

Pity they didn't put any thought in the "correct" way to add blank space to page beyond line breaks. Or the correct way to populate the DOM that isn't accessing the inner HTML of an element.

At least this was a former co-worker.

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

As oft stated, the "right" way to validate emails is to do a bare minimum sanity check on format, and then send a verification message to the email address the user supplied; it's the only way to ensure that what they gave you isn't just syntactically valid, but is actually usable.

But even that simple approach leaves places to go wrong. Take a look at this code, from Lana.

public function getEmailValidationErrors($data): array { $errors = []; if (isset($data["email"]) && !empty($data["email"])) { if (!str_contains($data["email"], "@")) { $error["email"] = "FORM.CONTACT_DETAILS.ERRORS.NO_AT"; } if (!str_contains($data["email"], ".")) { $error["email"] = "FORM.CONTACT_DETAILS.ERRORS.NO_DOT"; } if (strrpos($data["email"], "@") > strrpos($data["email"], ".")) { $error["email"] = "FORM.CONTACT_DETAILS.ERRORS.NO_TLD"; } } if (isset($data["email1"]) && !empty($data["email1"])) { if (!str_contains($data["email1"], "@")) { $error["email1"] = "FORM.CONTACT_DETAILS.ERRORS.NO_AT"; } if (!str_contains($data["email1"], ".")) { $error["email1"] = "FORM.CONTACT_DETAILS.ERRORS.NO_DOT"; } if (strrpos($data["email1"], "@") > strrpos($data["email1"], ".")) { $error["email1"] = "FORM.CONTACT_DETAILS.ERRORS.NO_TLD"; } } if (isset($data["email2"]) && !empty($data["email2"])) { if (!str_contains($data["email2"], "@")) { $error["email2"] = "FORM.CONTACT_DETAILS.ERRORS.NO_AT"; } if (!str_contains($data["email2"], ".")) { $error["email2"] = "FORM.CONTACT_DETAILS.ERRORS.NO_DOT"; } if (strrpos($data["email2"], "@") > strrpos($data["email2"], ".")) { $error["email2"] = "FORM.CONTACT_DETAILS.ERRORS.NO_TLD"; } } if (isset($data["email3"]) && !empty($data["email3"])) { if (!str_contains($data["email3"], "@")) { $error["email3"] = "FORM.CONTACT_DETAILS.ERRORS.NO_AT"; } if (!str_contains($data["email3"], ".")) { $error["email3"] = "FORM.CONTACT_DETAILS.ERRORS.NO_DOT"; } if (strrpos($data["email3"], "@") > strrpos($data["email3"], ".")) { $error["email3"] = "FORM.CONTACT_DETAILS.ERRORS.NO_TLD"; } } return $errors; }

Let's start with the obvious problem: repetition. This function doesn't validate simply one email, but four, by copy/pasting the same logic multiple times. Lana didn't supply the repeated blocks, just noted that they existed, so let's not pick on the bad names: "email1", etc.- that's just my placeholder. I assume it's different contact types for a customer, or similar.

Now, the other problems range from trivial to comical. First, the PHP function empty returns true if the variable has a zero/falsy value or is not set, which means it implies an isset, making the first branch redundant. That's trivial.

The way the checks get logged into the $error array, they can overwrite each other, meaning if you forget the "@" and the ".", it'll only complain about the ".", but if you forget the ".", it'll complain about not having a valid TLD (the "NO_DOT" error will never be output). That's silly.

Finally, the $errors array is the return value, but the $error array is where we store our errors, meaning this function doesn't return anything in the first place. And that means that it's a email validation function which doesn't do anything at all, which honestly- probably for the best.

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

It's only a day away!

Punctual Robert F. never procrastinates. But I think now would be a good time for a change. He worries that "I better do something quick, before my 31,295 year deadline arrives."

 

Stewart suffers so, saying "Whilst failing to check in for a flight home on the TUI app (one of the largest European travel companies), their Harry Potter invisibility cloak slipped. Perhaps I'll just have to stay on holiday?" You have my permission, just tell the boss I said so.

 

Diligent Dan H. is in no danger of being replaced. Says Dan, "My coworker was having problems getting regular expressions to work in a PowerShell script. She asked Bing's Copilot for help - and was it ever helpful!"

 

PSU alum (I'm guessing) Justin W. was overwhelmed in Happy Valley. "I was just trying to find out when the game started. This is too much date math for my brain to figure out."

 

Finally, bug-loving Pieter caught this classic. "They really started with a blank slate for the newest update. I'm giving them a solid %f for the effort."

 

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

Markus does QA, and this means writing automated tests which wrap around the code written by developers. Mostly this is a "black box" situation, where Markus doesn't look at the code, and instead goes by the interface and the requirements. Sometimes, though, he does look at the code, and wishes he hadn't.

Today's snippet comes from a program which is meant to generate PDF files and then, optionally, email them. There are a few methods we're going to look at, because they invested a surprising amount of code into doing this the wrong way.

protected override void Execute() { int sendMail = this.VerifyParameterValue(ParamSendMail); if (sendMail == -1) return; if (sendMail == 1) mail = true; this.TraceOutput(Properties.Resources.textGetCustomerForAccountStatement); IList<CustomerModel> customers = AccountStatement.GetCustomersForAccountStatement(); if (customers.Count == 0) return; StreamWriter streamWriter = null; if (mail) streamWriter = AccountStatement.CreateAccountStatementLogFile(); CreateAccountStatementDocumentEngine engine = new CreateAccountStatementDocumentEngine(); foreach (CustomerModel customer in customers) { this.TraceOutput(Properties.Resources.textCustomerAccountStatementBegin + customer.DisplayName.ToString()); // Generate the PDF, optionally send an email with the document attached engine.Execute(customer, mail); if (mail) { AccountStatement.WriteToLogFile(customer, streamWriter); this.TraceOutput(Properties.Resources.textLogWriting); } } engine.Dispose(); if (streamWriter != null) streamWriter.Close(); }

Now, this might sound unfair, but right off the bat I'm going to complain about separation of concerns. This function both generates output and emails it (optionally), while handling all of the stream management. Honestly, I think if the developer were simply forced to go back and make this a set of small, cohesive methods, most of the WTFs would vanish. But there's more to say here.

Specifically, let's look at the first few lines, where we VerifyParameterValue. Note that this function clearly returns -1 when it fails, which is a very C-programmer-forced-to-do-OO idiom. But let's look at that method.

private int VerifyParameterValue(string name) { string stringValue = this.GetParam(name, string.Empty); bool isValid = this.VerifyByParameterFormat(name, stringValue); if (!isValid) return -1; int value = -1; try { value = Convert.ToInt32(stringValue); } catch (Exception e) { this.TraceOutput(string.Concat("\n\n\n", e.Message, "\n\n\n")); return -1; } return value; }

We'll come back to the VerifyByParameterFormat but otherwise, this is basically a wrapper around Convert.ToInt32, and could easily be replaced with Int32.TryParse.

Bonus points for spamming the log output with loads of newlines.

Okay, but what is the VerifyByParameterFormat doing?

private bool VerifyByParameterFormat(string name, string value) { string parameter = string.Empty; string message = string.Empty; if (value.Length != 1) { parameter = Properties.Resources.textSendMail; message = string.Format(Properties.Resources.textSendMailNotValid, value); this.TraceOutput(string.Concat("\n\n\n", message, "\n\n\n")); return false; } string numbers = "0123456789"; char[] characters = value.ToCharArray(); for (byte i = 0; i < characters.Length; i++) { if (numbers.IndexOf(characters[i]) < 0) { parameter = Properties.Resources.textSendMail; message = Properties.Resources.textSendMailNotValid; this.TraceOutput(string.Concat("\n\n\n", message, "\n\n\n")); return false; } } return true; }

Oh, it just goes character by character to verify whether or not this is made up of only digits. Which, by the way, means the CLI argument needs to be an integer, and only when that integer is 1 do we send emails. It's a boolean, but worse.

Let's assume, however, that passing numbers is required by the specification. Still, Markus has thoughts:

Handling this command line argument might seem obvious enough. I'd probably do something along the lines of "if (arg == "1") { sendMail = true } else if (arg != "0") { tell the user they're an idiot }. Of course, I'm not a professional programmer, so my solution is way too simple as the attached piece of code will show you.

There are better ways to do it, Markus, but as you've shown us, there are definitely worse ways.

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

Today, we look at a simple bit of bad code. The badness is not that they're using Oracle, though that's always bad. But it's how they're writing this PL/SQL stored function:

FUNCTION CONVERT_STRING_TO_DATE --Public (p_date_string IN Varchar2, p_date_format IN Varchar2 DEFAULT c_date_format) Return Date AS BEGIN If p_date_string Is Null Then Return Null; Else Return To_Date(p_date_string, p_date_format); End If; END; -- FUNCTION CONVERT_STRING_DATE

This code is a wrapper around the to_date function. The to_date function takes a string and a format and returns that format as a date.

This wrapper adds two things, and the first is a null check. If the input string is null, just return null. Except that's exactly how to_date behaves anyway.

The second is that it sets the default format to c_date_format. This, actually, isn't a terrible thing. If you check the docs on the function, you'll see that if you don't supply a format, it defaults to whatever is set in your internationalization settings, and Oracle recommends that you don't rely on that.

On the flip side, this code is used as part of queries, not on processing input, which means that they're storing dates as strings, and relying on the application layer to send them properly formatted strings. So while their to_date wrapper isn't a terrible thing, storing dates as strings definitely is a terrible thing.

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

The end of the quarter was approaching, and dark clouds were gathering in the C-suite. While they were trying to be tight lipped about it, the scuttlebutt was flowing freely. Initech had missed major sales targets, and not just by a few percentage points, but by an order of magnitude.

Heads were going to roll.

Except there was a problem: the master report that had kicked off this tizzy didn't seem to align with the department specific reports. For the C-suite, it was that report that was the document of record; they had been using it for years, and had great confidence in it. But something was wrong.

Enter Jeff. Jeff had been hired to migrate their reports to a new system, and while this particular report had not yet been migrated, Jeff at least had familiarity, and was capable of answering the question: "what was going on?" Were the sales really that far off, and was everyone going to lose their jobs? Or could it possibly be that this ancient and well used report might be wrong?

The core of the query was basically a series of subqueries. Each subquery followed this basic pattern:

SELECT SUM(complex_subquery_A) as subtotal FROM complex_subquery_B

None of this was particularly readable, mind you, and it took some digging just to get the shape of the individual queries understood. But none of the individual queries were the problem; it was the way they got stitched together:

SELECT SUM(subtotal) FROM (SELECT SUM(complex_subquery_A) as subtotal FROM complex_subquery_B UNION SELECT SUM(complex_subquery_C) as subtotal FROM complex_subquery_D UNION SELECT SUM(complex_subquery_E) as subtotal FROM complex_subquery_F);

The full query was filled with a longer chain of unions, but it was easy to understand what went wrong, and demonstrate it to management.

The UNION operator does a set union- which means if there are any duplicate values, only one gets included in the output. So if "Department A" and "Department C" both have $1M in sales for the quarter, the total will just be $1M- not the expected $2M.

The correct version of the query would use UNION ALL, which preserves duplicates.

What stunned Jeff was that this report was old enough to be basically an antique, and this was the kind of business that would burn an entire forest down to find out why a single invoice was off by $0.15. It was sheer luck that this hadn't caused an explosion before- or maybe in the past it had, and someone had just written it off as a "minor glitch"?

Unfortunately for Jeff, because the report was so important it required a huge number of approvals before the "UNION ALL" change could be deployed, which meant he was called upon to manually run a "test" version of the report containing the fix every time a C-suite executive wanted one, until the end of the following quarter, when he could finally integrate the fix.

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

There are things which are true. Regular expressions frequently perform badly. They're hard to read. Email addresses are not actually regular languages, and thus can't truly be validated (in all they're many possible forms) by a pure regex.

These are true. It's also true that a simple regex can get you most of the way there.

Lucas found this in their codebase, for validating emails.

function echeck(str) { var at="@"; var dot="."; var lat=str.indexOf(at); var lstr=str.length; var ldot=str.indexOf(dot); if (str.indexOf(at)==-1){ alert("You must include an accurate email address for a response."); return false; } if (str.indexOf(at)==-1 || str.indexOf(at)==0 || str.indexOf(at)==lstr){ alert("You must include an accurate email address for a response."); return false; } if (str.indexOf(dot)==-1 || str.indexOf(dot)==0 || str.indexOf(dot)==lstr){ alert("You must include an accurate email address for a response."); return false; } if (str.indexOf(at,(lat+1))!=-1){ alert("You must include an accurate email address for a response."); return false; } if (str.substring(lat-1,lat)==dot || str.substring(lat+1,lat+2)==dot){ alert("You must include an accurate email address for a response."); return false; } if (str.indexOf(dot,(lat+2))==-1){ alert("You must include an accurate email address for a response."); return false; } if (str.indexOf(" ")!=-1){ alert("You must include an accurate email address for a response."); return false; } return true; }

It checks that the string contains an "@", and the "@" is not at the beginning or end of the string. Then it does the same check for a ".". Then it checks that there isn't a second "@". Then it checks that there are at least two non-"@" characters before the ".". Then it checks that there's at least one "." after the "@". Then it checks that there are no spaces.

Like a regex, I don't think this covers the entire space of valid and invalid email addresses, but that's just because the email address spec is complicated. It likely qualifies as "good enough", on that front. But it's the most awkward way to express that series of tests, especially since they create variables which might be useful, but never use them, thus calling str.indexOf many, many times. The awkwardness becomes more obvious with the way it outputs the same error message in multiple branches. Outputs them using alert I might add, which is the kind of choice that should send someone to the Special Hell™.

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

The title of this week's column is making me hungry.

To start off our WTFreitag, Reinier B. complains "I did not specify my gender since it's completely irrelevant when ordering a skateboard for my daughter. That does not mean it is correct to address me as Dear Not specified." I wonder (sincerely) if there is a common German-language personal letter greeting for "Dear somebody of unknown gender". I don't think there is one for English. "To Whom It May Concern" is probably the best we can do.

 

"A coworker shared this phishing email he got," reported Adam R. "Even the scammers have trouble with their email templating variable names too." (It's there at the end).

 

Mathematically-minded Marcel V. thinks these numbers don't figure. "The update process of my Overwatch game left me wondering. I am quite certain my computer does not have 18 Exabytes of harddisk space to reclaim. However, if bytes were meant, then how is it trying to reclaim the last .65 bytes??"

 

Big Spender Jeff W. humblebrags "If you have to ask, you can't afford it."

 

Finishing up for the week, Bruce R. signs off with "This dialog from CNN seems a little overdone."

 

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

Kirill writes:

I've worked in this small company for a year, and on a daily basis I've come across things that make my eyes sink back into their sockets in fear, but mostly I've been too busy fixing them to post anything. It being my last day however, here's a classic

We'll take this one in parts. First, every element of the UI the user can navigate to is marked with an enum, defined thus:

enum UiItem { SectionA, SectionB, SectionC,...SectionG }

These names are not anonymized, so already I hate it. But it's the next enum that starts my skin crawling:

enum SecurityUiItem { SectionA = UiItem.SectionA, SectionB = UiItem.SectionB, ... SectionG = UiItem.SectionG }

A SecurityUiItem is a different type, but the values are identical to UiItem.

These enums are used when trying to evaluate role-based permissions for access, and that code looks like this:

if ((currentAccess.ContainsKey(SecurityUiItem.SectionA) && currentAccess[SecurityUiItem.SectionA] != AccessLevel.NoAccess)) return UiItem.SectionA; else if (!currentAccess.ContainsKey(SecurityUiItem.SectionB) || (currentAccess.ContainsKey(SecurityUiItem.SectionB) && currentAccess[SecurityUiItem.SectionB] != AccessLevel.NoAccess)) return UiItem.SectionB; else if (!currentAccess.ContainsKey(SecurityUiItem.SectionC) || (currentAccess.ContainsKey(SecurityUiItem.SectionC) && currentAccess[SecurityUiItem.SectionC] != AccessLevel.NoAccess)) return UiItem.SectionC; ..... else if (!currentAccess.ContainsKey(SecurityUiItem.SectionG) || (currentAccess.ContainsKey(SecurityUiItem.SectionG) && currentAccess[SecurityUiItem.SectionG] != AccessLevel.NoAccess)) return UiItem.SectionG; else return UiItem.Unknown;

Honestly, I don't hate the idea of having one data type representing the actual UI objects and a separate data type which represents permissions, and having a function which can map between these two things. But this is a perfect example of a good idea executed poorly.

I also have to wonder about the fall-through pattern. If I have access to SectionA, I only seem to get SectionA out of this function. Are these permissions hierarchical? I have no idea, but I suspect there's a WTF underpinning this whole thing.

Congratulations on Kirill's last day.

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

The CEO of Delia's company retired. They were an old hand in the industry, the kind of person who worked their way up and had an engineering background, and while the staff loved them, the shareholders were less than pleased, because the company was profitable, but not obscenely so. So the next CEO was a McKinsey-approved MBA who had one mission: cut costs.

Out went the senior devs, and much of the managers. Anyone who was product or customer focused followed quickly behind. What remained were a few managers handpicked by the new CEO and a slew of junior engineers- and Pierre.

Pierre was a contractor who followed the new CEO around from company to company. Pierre was there to ensure that nobody wasted any time on engineering that didn't directly impact features. Tests? Wastes of time. Module boundaries? Just slow you down. Move fast and break things, and don't worry about fixing anything because that's your successors' problem.

So let's take a look at how Pierre wrote code. This block of PHP code was simply copy/pasted everywhere it needed to be used, across multiple applications.

foreach($alphaCheck as $checkField) { if($paramsArray[$checkField['field']]) { if($paramsArray[$checkField['field']]) { $invalidChars = false; $checkValue = trim(strtoupper($paramsArray[$checkField['field']])); $allowedChars = explode('|',"A|B|C|D|E|F|G|H|I|J|K|L|M|N|O|P|Q|R|S|T|U|V|W|X|Y|Z|-| |.|'"); for($i=0; $i<strlen($checkValue); $i++) { if(!in_array($checkValue[$i],$allowedChars)) { $invalidChars = true; } } if($invalidChars) { $errorMsgs[] = $checkField['name'] . ' contains invalid characters'; } } } }

This isn't the worst approach to this problem I've seen in PHP, but the fact that this is just a copy/pasted blob, and worse- the $allowedChars may vary a bit in each place it's copy/pasted is what makes it terrible.

Don't worry. The new CEO only stayed for 18 months, got a huge bonus thanks to all the cost-cutting, and then left, taking Pierre along to the next company.

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

Python's "batteries included" approach means that a lot of common tasks have high-level convenience functions for them. For example, if you want to read all the lines from a file into an array (list, in Python), you could do something like:

with open(filename) as f: lines = f.readlines()

Easy peasy. Of course, because it's so easy, there are other options.

For example, you can just convert the file directly to a list: lines = list(f). Or you can iterate across the file directly, e.g.:

with open(filename) as f: for line in f: # do stuff

Of course, that's fine for plain old text files. But we frequently use text files which are structured in some fashion, like a CSV file. No worries, though, as Python has a csv library built in, which makes it easy to handle these files too; especially useful because "writing a CSV parser yourself" is one of those tasks that sounds easy until you hit the first edge case, and then you realize you've made a terrible mistake.

Now, it's important to note that CSV usually is expressed as a "comma separated values" file, but the initialism is actually "character separated values". And, as Sally's co-worker realized, newlines are characters, and thus every text file is technically a CSV file.

foo = list(csv.reader(someFile, delimiter="\n")) .comment { border: none; } [Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!

Most of us, when generating a UUID, will reach for a library to do it. Even a UUIDv4, which is just a random number, presents challenges: doing randomness correctly is hard, and certain bits within the UUID are reserved for metadata about what kind of UUID we're generating.

But Gretchen's co-worker didn't reach for a library. What they did reach for was… regular expressions?

function uuidv4() { return "xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx".replace(/[xy]/g, function (c) { var r = (Math.random() * 16) | 0, v = c == "x" ? r : (r & 0x3) | 0x8; return v.toString(16); }); }

At a glance, this appears to be a riff on common answers on Stack Overflow. I won't pick on this code for not using crypto.randomUUID, the browser function for doing this, as that function only started showing up in browsers in 2021. But using a format string and filling it with random data instead of generating your 128-bits as a Uint8Buffer is less forgivable.

This solution to generating UUIDs makes a common mistake: confusing the representation of the data with the reality of the data. A UUID is 128-bits of numerical data, with a few bits reserved for identification (annoyingly, how many bits are reserved depends on which format we're talking about). We render it as the dash-separated-hex-string, but it is not a dash-separated-hex-string.

In the end, this code does work. Awkwardly and inefficiently and with a high probability of collisions due to bad randomness, but it works. I just hate it.

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

We like trains here at Error'd, and you all seem to like trains too. That must be the main reason we get so many submissions about broken information systems.

"Pass," said Jozsef . I think that train might have crashed already.

 

An anonymous subscriber shared an epic tale some time ago. They explained thus. "(I couldn't capture in the photo, but the next station after Duivendrecht was showing the time of 09:24+1.) We know Europe has pretty good trains, and even some high-speed lines. But this was the first time I boarded a time-traveling train. At first I was annoyed to be 47 minutes late. I thought I could easily walk from Amsterdam Centraal to Muiderpoort in less than the 53 minutes that this train would take. But I was relieved to know the trip to the further stations was going to be quicker, and I would arrive there even before arriving at the earlier stations."
I think the explanation here is that this train is currently expected to arrive at Muiderport around 10:01. But it's still projected to arrive at the following stop at 9:46, and more surprisingly at the successive stops at 9:35 and 9:25.

 

Railfan Richard B. recently shared "Points failure on the West Coast Main Line has disrupted the linear nature of time."

 

and quite some time ago, he also sent us this snap, singing "That train that's bound for glory? It runs through here."

 

An unrelated David B. wonders "When is the next train? We don't know, it's running incognito."

 

And finally, courageous Ivan got sideways underground. "Copenhagen subway system may have fully automated trains, but their informational screens need a little manual help every now and then."

 

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

There are real advantages to taking a functional programming approach to expressing problems. Well, some problems, anyway.

Kevin sends us this example of elegant, beautiful functional code in C#:

//create a range of dates List<DateTime> dates = Enumerable.Range (0, 1 + settings.EndDate.Subtract (settings.BeginDate).Days).Select (offset => settings.BeginDate.AddDays(offset)).ToList(); foreach (DateTime procDate in dates) { /*.snip.*/ }

If you're not sure what this code does, it's okay- Kevin rewrote it and "ruined" it:

DateTime procDate = settings.BeginDate; while(procDate <= settings.EndDate) { /*.snip.*/ procDate= procDate.AddDays(1); }

The goal of this code is simply to do something for every day within a range of dates. These two approaches vary a bit in terms of readability though.

I guess the loop in the functional version isn't mutating anything, I suppose. But honestly, I'm surprised that this didn't take the extra step of using the .ForEach function (which takes a lambda and applies it to each parameter). Heck, with that approach, they could have done this whole thing in a single statement.

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

Greg was fighting with an academic CMS, and discovered that a file called write_helper.js was included on every page. It contained this single function:

function document_write(s) { document.write(s); }

Now, setting aside the fact that document.write is one of those "you probably shouldn't use this" functions, and is deprecated, one has to wonder what the point of this function is. Did someone really not like object-oriented style code? Did someone break the "." on their keyboard and just wanted to not have to copy/paste existing "."s?

It's the kind of function you expect to see that someone wrote but that isn't invoked anywhere, and you'd almost be correct. This function, in a file included on every page, is called once and only once.

More like the wrong helper, if we're being honest.

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

Gretchen saw this line in the front-end code for their website and freaked out:

let bucket = new AWS.S3({ params: { Bucket: 'initech-logos' } });

This appeared to be creating an object to interact with an Amazon S3 bucket on the client side. Which implied that tokens for interacting with S3 were available to anyone with a web browser.

Fortunately, Gretchen quickly realized that this line was commented out. They were not hosting publicly available admin credentials on their website anymore.

.comment { border: none; }

They used to, however, and the comments in the code made this a bit more clear:

// inside an angular component: uploadImage(): void { const uniqueName = `${this.utils.generateUUID()}_${this.encrDecSrvc.getObject(AppConstants.companyID)}_${this.file.name}` /*; @note: Disable usage of aws credential, transfer flow to the backend. @note; @disable-aws-credential */ /*; AWS.config.region = 'us-east-1' let bucket = new AWS.S3({ params: { Bucket: 'initech-billinglogos' } }); */ const bucket = ( AWSBucketMask ); const params = { Bucket: 'initech-logos', Key: 'userprofilepic/' + uniqueName, ACL: "public-read", Body: this.file }; const self = this; bucket.upload( params, function (err, data) { if (err) { console.log("error while saving file on s3 server", err); return; } self.isImageUrl = true; self.imageUrl = data.Location; self.myProfileForm.controls['ProfilePic'].setValue(self.imageUrl); self.encrDecSrvc.addObject(AppConstants.imageUrl, self.imageUrl); self.initechAPISrvc.fireImageView(true); self.saveProfileData(); self.fileUpload.clear() }, self.APISrvc ); }

Boy, this makes me wonder what that AWSBucketMask object is, and what its upload function does.

export class AWSBucketMask { public static async upload( option, callback, service ){ const fileReader = new FileReader( ); fileReader.onloadend = ( ( ) => { const dataURI = ( `${ fileReader.result }` ); const [ entityType, mimeType, baseType, data ] = ( dataURI.split( /[\:\;\,]/ ) ); option.ContentEncoding = baseType; option.ContentType = mimeType; option.Body = data; service.awsBucketMaskUpload( option ) .subscribe( function( responseData ){ callback( null, responseData.data ); }, function( error ){ callback( error ); } ); } ); fileReader.readAsDataURL( option.Body ); } public static async deleteObject( option, callback, service ){ service.awsBucketMaskDeleteObject( option ) .subscribe( function( responseData ){ callback( null, responseData ); }, function( error ){ callback( error ); } ); } public static async deleteObjects( option, callback, service ){ service.awsBucketMaskDeleteObjects( option ) .subscribe( function( responseData ){ callback( null, responseData ); }, function( error ){ callback( error ); } ); } public static async getSignedUrl( namespace, option, callback, service ){ service.awsBucketMaskGetSignedUrl( namespace, option ) .subscribe( function( responseData ){ callback(null, responseData.data); }, function( error ){ callback( error ); } ); } }

The important thing to notice here is that each of the methods here invokes a web service service.awsBucketMaskUpload, for example. Given that nothing actually checks their return values and it's all handled through callback hell, this is a clear example of async pollution- methods being marked async without understanding what async is supposed to do.

But that's not the real WTF. You may notice that these calls back to the webservice are pretty thin. You see, here's the problem: originally, they just bundled the S3 into the client-side, so the client-side code could do basically anything it wanted to in S3. Adding a service to "mask" that behavior would have potentially meant doing a lot of refactoring, so instead they made the service just a dumb proxy. Anything you want to do on S3, the service does for you. It does no authentication. It does no authorization. It runs with the admin keys, so if you can imagine a request you want to send it, you can send it that request. But at least the client doesn't have access to the admin keys any more.

This is an accounting application, so some of the things stored in S3 are confidential financial information.

Gretchen writes:

We have to take cybersecurity courses every 3 months, but it seems like this has no effect on the capabilities of my fellow coworkers.

You can lead a programmer to education, but you can't make them think.

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

One thing I've learned by going through our reader submissions over the years is that WTFs never start with just one mistake. They're a compounding sequence of systemic failures. When we have a "bad boss" story, where an incompetent bully puts an equally incompetent sycophant in charge of a project, it's never just about the bad boss- it's about the system that put the bad boss in that position. For every "Brillant" programmer, there's a whole slew of checkpoints which should have stopped them before they went too far.

With all that in mind, today we're doing a news roundup about the worst boss of them all, the avatar of Dunning-Kruger, Elon Musk. Because over the past month, a lot has happened, and there are enough software and IT related WTFs that I need to talk about them.

For those who haven't been paying attention, President Trump assembled a new task force called the "Department of Government Efficiency", aka "DOGE". Like all terrible organizations, its mandate is unclear, its scope is unspecified, and its power to execute is unbounded.

Now, before we get into it, we have to talk about the name. Like so much of Musk's persona, it's an unfunny joke. In this case, just a reference to Dogecoin, a meme currency based on a meme image that Musk has "invested" in. This is part of a pattern of unfunny jokes, like strolling around Twitter headquarters with a sink, or getting your product lines to spell S3XY. This has nothing to do with the news roundup, I just suspect that Musk's super-villain origin story was getting booed off the stage at a standup open-mic night and then he got roasted by the emcee. Everything else he's ever done has been an attempt to convince the world that he's cool and popular and funny.

On of the core activities at DOGE is to be a "woodchipper", as Musk puts it. Agencies Musk doesn't like are just turned off, like USAID.

The United States Agency for International Development handles all of the US foreign aid. Now, there's certainly room for debate over how, why, and how much aid the US provides abroad, and that's a great discussion that I wouldn't have here. But there's a very practical consideration beyond the "should/should not" debate: people currently depend on it.

Farmers in the US depend on USAID purchasing excess crops to stabilize food prices. Abroad, people will die without the support they've been receiving.

Even if you think aid should be ended entirely, simply turning off the machine while people are using it will cause massive harm. But none of this should come as a surprise, because Musk loves to promote his "algorithm".

Calling it an "algorithm" is just a way to make it sound smarter than it is; what Musk's "algorithm" really is is a 5-step plan of bumper-sticker business speak that ranges from fatuous to incompetent, and not even the fawning coverage in the article I linked can truly disguise it.

For example, step 1 is "question every requirement", which is obvious- of course, if you're trying to make this more efficient, you should question the requirements. As a sub-head on that, though, Musk says that requirements should be traceable directly to individuals, not departments. On one hand, this could be good for accountability, but on the other, any sufficiently complex system is going to have requirements that have to be built through collaboration, where any individual claiming the requirement is really just doing so to be a point of accountability.

Step 2, also has a blindingly obvious label: "delete any part of the process you can". Oh, very good, why didn't I think of that! But Musk has a "unique" way of figuring out what parts of the process can be deleted: "You may have to add them back later. In fact, if you do not end up adding back at least 10 percent of them, then you didn’t delete enough."

Or, to put it less charitably: break things, and then unbreak them when you realize what you broke, if you do.

We can see how this plays out in practice, because Musk played this game when he took over Twitter. And sure, it's revenue has collapsed, but we don't care about that here. What we care about are stupid IT stories, like the new owner renting a U-Haul and hiring a bunch of gig workers to manually decommission an expensive data center. Among the parts of the process Musk deleted were:

• Shutting down the servers in an orderly fashion
• Using the proper tools to uninstall the server racks
• Protecting the flooring which wasn't designed to roll 2,500lb server racks
• Not wiping the hard drives which contained user data and proprietary information
• Not securing that valuable data with anything more than a set of Home Depot padlocks and Apple AirTags

And, shockingly, despite thinking this was successful in the moment, the resulting instability caused by just ripping a datacenter out led Musk to admit this was a mistake.

So let's take a look at how this plays out with DOGE. One of the major efforts was taking over the Treasury Department's IT systems. These are systems which handle $5 trillion in payments every year. And who do we put in change? Some random wet-behind-the-ears dev with a history of racist posts on the Internet.

Ostensibly, they were there to "audit" payments, so was their access read only? Did they have admin access? Were they actually given write access? Could they change code? Nobody is entirely certain. Even if it was only read-only, there are plenty of questions about what kind of security risk that constitutes, which means forensic analysis to understand the breach, which is being called the largest data breach in history.

Part of the goal was to just stop payments, following the Muskian "Break things first, and unbreak them if it was a mistake," optimization strategy. Stop paying people, and if you find out you needed to pay them, then start paying them again. Step 2 of the "algorithm".

Speaking of payments, many people in the US depend on payments from the Social Security Administration. This organization, founded in 1935 as part of the New Deal, handles all sorts of benefits, including retirement benefits. According to Musk, it's absolutely riddled with fraud.

What are his arguments? Well, for starters, he worries that SSNs are not de-duplicated- that is, the same SSN could appear multiple times in the database.

Social Security Administration has, since the 1940s, been trying to argue against using SSNs as identifiers for any purpose other than Social Security. They have a history page which is a delightful read as a "we can't say the Executive Orders and laws passed which expanded the use of SSNs into domains where they shouldn't have been used was a bad idea, but we can be real salty about it," document. It's passive-aggression perfected. But you and I already know you should never expect SSNs to be a key.

Also, assuming the SSA systems are mainframe systems, using flat file databases, we would expect a large degree of denormalization. Seeing "unique" keys repeated in the dataset is normal.

On the same subject, Musk has decided that people over 150 years old are collecting Social Security benefits. Now, one could assume that this kind of erroneous data pattern is fraud, or we could wonder if there's an underlying reason to the pattern.

Now, I've seen a lot of discussion on the Internet about this being an epoch related thing, which is certainly possible, but I think the idea that it's related to ISO8601 is obviously false- ISO8601 is just a string representation of dates, and also was standardized well after COBOL and well after SSA started computerizing. Because the number 150 was used, some folks have noted that would be 1875, and have suspected that the date of the Metre Convention is the epoch.

I can't find any evidence that any of this is true, mind you, but we're also reacting to a tweet by a notorious moron, and I have to wonder: did he maybe round off 5 years? Because 1870 is exactly 65 years before 1935- the year Social Security started, and 65 years is the retirement age where you can start collecting Social Security. Thus, the oldest date which the SSA would ever care about was 1870. Though, there's another completely un-epoch related reason why you could have Social Security accounts well older than 150 years: your benefits can continue to be paid out to your spouse and dependents after your death. If an 80 year old marries a 20 year old, and dies the next day, that 20 year old could collect benefits on that account.

The key point I'm making is that "FRAUUUDDDD!!1111!!!" is really not the correct reaction to a system you don't understand. And while there may be many better ways to handle dates within the SSA's software, the system predates computers and has needed to maintain its ability to pay benefits for 90 years. While you could certainly make improvements, what you can't do is take a big "algorithm" Number 2 all over it.

Which, with that in mind, the idea that these people are trying to get access to a whole slew of confidential taxpayer information is I'm sure going to go *great.

There are so, so many more things that could be discussed here, but let's close with the DOGE website. Given that DOGE operates by walking into government agencies and threatening to call Elon, there are some serious concerns over transparency. Who is doing what, when, why and with what authority? The solution? Repost a bunch of tweets to a website with a .gov domain name.

Which, you'd think that spinning up a website that's just that would be easy. Trivially easy. "Security issues" shouldn't even be part of the conversation. But in actuality, the database was unsecured and anyone could modify the site.

In the end, the hacked website is really just Elon Musk's "algorithm" improved: instead of breaking things that are already working, you just start with a broken website.

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

Because we still have the NWS, I learned that "A winter storm will continue to bring areas of heavy snow and ice from the Great Lakes through New England today into tonight." I'm staying put, and apparently so is Dave L.'s delivery driver.

Dave L. imagines the thoughts of this driver who clearly turned around and headed straight home. "Oh, d'ya mean I've got to take these parcels somewhere!? in this weather!? I can't just bring them back?"

 

Infoscavenger Andrew G. shared a found object. "I got this from https://bsky.app/profile/jesseberney.com/post/3lhyiubtay22r and immediately thought of you. Sadly I expect you're going to get more of these in the coming days/weeks." I guess they already fired all people who knew how to use Mail Merge.

 

Bruce R. "I saw this ad in my Android weather app. They think they know all about you, but they got this completely wrong: I don't live in {city}, I live in [city]!" Right next to Mr. [EmployeeLastName].

 

"I've got a vintage k8s cluster if anyone's interested," reports Mike S. "I just installed docker desktop. Apparently it ships with the zeroeth version of kubernetes."

 

Finally for this week, special character Vitra has sent us an elliptical statement about her uni application. "Of course, putting characters in my personal statement is simply a bad idea - it's best to let the Universities have the thrill of mystery! (From UCAS, the main university application thing in the UK too!) "

 

Keep warm. [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!

Abdoullah sends us this little blob of C#, which maybe isn't a full-on WTF, but certainly made me chuckle.

if (file!= null) { if (file.name.StartsWith(userName)) { if (file.name.StartsWith(userName)) { url = string.Format(FILE_LINK, file.itemId, file.name); break; } } }

Are you sure the file name starts with the user name? Are you really sure?

This code somehow slipped by code review, helped perhaps by the fact that the author was the senior-most team member and everyone assumed they were immune to these kinds of mistakes.

No one is immune.

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