The Daily WTF

I've had the misfortune of working in places which did source-control via comments. Like one place which required that, with each section of code changed, you needed to add a comment with your name, the ticket number, and the reason the change was made. You know, the kind of thing you can just get from your source control service.

In their defense, that policy was invented for mainframe developers and then extended to everyone else, and their source control system was in Visual Source Safe. VSS was a) terrible, and b) a perennial destroyer of history, so maybe they weren't entirely wrong and VSS was the real WTF. I still hated it.

In any case, Alice's team uses more modern source control than that, which is why she's able to explain to us the story of this function:

public function calculateMassGrossPay(array $employees, Payroll $payroll): array { // it shouldn't enter here, but if it does by any change, do nth return []; }

Once upon a time, this function actually contained logic, a big pile of fairly complicated logic. Eventually, a different method was created which streamlined the functionality, but had a different signature and logic. All the callers were updated to use that method instead- by commenting out the line which called this one. This function had a comment added to the top: // it shouldn't enter here.

Then, the body of this function got commented out, and the return was turned into an empty array. The comment was expanded to what you see above. Then, eventually, the commented-out callers were all deleted. Years after that, the commented out body of this function was also deleted, leaving behind the skeleton you see here.

This function is not referenced anywhere else, not even in a comment. It's truly impossible for code to "enter here".

Alice writes: "Version control by commented out code does not work very well."

Indeed, it does not.

.comment { border: none; } [Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.

Henrik H's employer thought they could save money by hiring offshore, and save even more money by hiring offshore junior developers, and save even more money by basically not supervising them at all.

Henrik sends us just one representative line:

if (System.DateTime.Now.AddDays(-365) <= f.ReleaseDate) // 365 days means one year

I appreciate the comment, that certainly "helps" explain the magic number. There's of course, just one little problem: It's wrong. I mean, ~75% of the time, it works every time, but it happily disregards leap years. Which may or may not be a problem in this case, but if they got so far as learning about the AddDays method, they were inches from using AddYears.

I guess it's true what they say: you can lead a dev to docs, but you can't make them think.

[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 utility closet Ellis had inherited and lived with for 17 years had been a cesspool of hazards to life and limb, a collection of tangible WTFs that had everyone asking an uncaring god, "What were they thinking?"

Every contractor who'd ever had to perform any amount of work in there had come away appalled. Many had even called over their buddies to come and see the stunning mess for themselves:

• All of the electrical components, dating from the 1980s, were scarily underpowered for what they were supposed to be powering.
• To get to the circuit breaker box—which was unlabeled, of course—one had to contort themselves around a water heater almost as tall as Ellis herself.
• As the house had no basement, the utility closet was on the first floor in an open house plan. A serious failure with said water heater would've sent 40 gallons (150 liters) of scalding-hot tsunami surging through the living room and kitchen.
• The furnace's return air vent had been screwed into crumbling drywall, and only prayers held it in place. Should it have fallen off, it would never have been replaceable. And Ellis' cat would've darted right in there for the adventure of a lifetime.
• To replace the furnace filter, Ellis had to put on work gloves, unscrew a sharp sheet-metal panel from the side of the furnace, pull the old filter out from behind a brick (the only thing holding it in place), manipulate the filter around a mess of water and natural gas pipes to get it out, thread the new filter in the same way, and then secure it in place with the brick before screwing the panel back on. Ellis always pretended to be an art thief in a museum, slipping priceless paintings around security-system lasers.
• Between the water tank, furnace, water conditioning unit, fiber optical network terminal, and router, there was barely room to breathe, much less enough air to power ignition for the gas appliances. Some genius had solved this by cutting random holes in several walls to admit air from outside. One of these holes was at floor-level. Once, Ellis opened the closet door to find a huge puddle on the floor, making her fear her hot water heater was leaking. As it turned out, a power-washing service had come over earlier that day. When they'd power-washed the exterior of her home, some of that water shot straight through one of those holes she hadn't known about, giving her utility closet a bonus bath.
• If air intake was a problem, venting the appliances' exhaust was an even worse issue. The sheet-metal vents had calcified and rusted over time. If left unaddressed, holes could've formed that would've leaked carbon monoxide into Ellis' house.

Considering all the above, plus the fact that the furnace and air conditioner were coming up on 20 years of service, Ellis couldn't put off corrective action any longer. Last week, over a span of 3 days, contractors came in to exorcise the demons:

• Upgrading electricals that hadn't already been dealt with.
• Replacing the hot water tank with a wall-mounted tankless heater.
• Replacing the furnace and AC with a heat pump and backup furnace, controlled by a new thermostat.
• Creating new pipes for intake and venting (no more reliance on indoor air for ignition).
• Replacing the furnace return air vent with a sturdier one.
• Putting a special hinged door on the side of the furnace, allowing the filter to be replaced in a matter of seconds (RIP furnace brick).

With that much work to be done, there were bound to be hiccups. For instance, when the Internet router was moved, an outage occurred: for no good reason, the optical network terminal refused to talk to Ellis' Wifi router after powering back up. A technician came out a couple days later, reset the Internet router, and everything was fine again.

All in all, it was an amazing and welcome transformation. As each new update came online, Ellis was gratefully satisfied. It seemed as though the demons were finally gone.

Unbeknownst to them all, there was one last vengeful spirit to quell, one final WTF that it was hell-bent on doling out.

It was late Friday afternoon. Despite the installers' best efforts, the new thermostat still wasn't communicating with the new heat pump. Given the timing, they couldn't contact the company rep to troubleshoot. However, the thermostat was properly communicating with the furnace. And so, Ellis was left with the furnace for the weekend. She was told not to mess with the thermostat at all except to adjust the set point as desired. They would follow back up with her on Monday.

For Ellis, that was perfectly fine. With the historically cold winter they'd been enduring in her neck of the woods, heat was all she cared about. She asked whom to contact in case of any issues, and was told to call the main number. With all that squared away, she looked forward to a couple of quiet, stress-free days before diving back into HVAC troubleshooting.

Everything was fine, until it wasn't. Around 11AM on Saturday, Ellis noticed that the thermostat displayed the word "Heating" while the furnace wasn't actually running. Maybe it was about to turn on? 15 minutes went by, then half an hour. Nothing had changed except for the temperature in her house steadily decreasing.

Panic set in at the thought of losing heat in her home indefinitely. That fell on top of a psyche that was already stressed out and emotionally exhausted from the last several days' effort. Struggling for calm, Ellis first tried to call that main number line for help as directed. She noticed right away that it wasn't a real person on the other end asking for her personal information, but an AI agent. The agent informed her that the on-call technician had no availablity that weekend. It would pencil her in for a service appointment on Monday. How did that sound?

"Not good enough!" Ellis cried. "I wanna speak to a representative!"

"I understand!" replied the blithe chatbot. "Hold on, let me transfer you!"

For a moment, Ellis was buoyed with hope. She'd gotten past the automated system. Soon, she'd be talking with a live person who might even be able to walk her through troubleshooting over the phone.

The new agent answered. Ellis began pouring her heart out—then stopped dead when she realized it was another AI agent, this time with a male voice instead of a female one. This one proceeded through nearly the same spiel as the first. It also scheduled her for a Monday service appointment even though the other chatbot had already claimed to have done so.

This was the first time an AI had ever pulled such a trick on Ellis. It was not a good time for it. Ellis hung up and called the only other person she could think to contact: her sales rep. When he didn't answer, she left a voicemail and texts: no heat all weekend was unacceptable. She would really appreciate a call back.

While playing the horrible waiting game, Ellis tried to think about what she could do to fix this. They had told her not to mess with the thermostat. Well, from what she could see, the thermostat was sending a signal to the furnace that the furnace wasn't responding to for whatever reason. It was time to look at the docs. Fortunately, the new furnace's manual was resting right on top of it. She spread it open on her kitchen table.

OK, Ellis thought, this newfangled furnace has an LED display which displays status codes. Her old furnace had lacked such a thing. Lemme find that.

Inside her newly remodeled utility closet, she located the blinking display, knelt, and spied the code: 1dL. Looking that up in the doc's troubleshooting section, she found ... Normal Operation. No action.

The furnace was OK, then? Now what?

Aside from documentation, another thing Ellis knew pretty well was tech support. She decided to break out the ol' turn-it-off-and-on-again. She shut off power to both the furnace and thermostat, waited a few minutes, then switched everything back on, crossing her fingers.

No change. The indoor temperature kept dropping.

Her phone rang: the sales rep. He connected her with the on-call technician for that weekend, who fortunately was able to arrive at her house within the hour.

One tiny thermostat adjustment later, and Ellis was enjoying a warm house once more.

What had happened?

This is where an understanding of heat pumps comes into play. In this configuration, the heat pump is used for cooling and for heating, unless the outside temperature gets very cold. At that point, the furnace kicks in, which is more efficient. (Technology Connections has some cool videos about this if you're curious.)

Everything had been running fine for Ellis while the temperatures had remained below freezing. The problem came when, for the first time in approximately 12 years, the temperature rose above 40F (4C). At that point, the new thermostat decided, without telling Ellis, I'm gonna tell the HEAT PUMP to heat the joint!

... which couldn't do anything just then.

Workaround: the on-call technician switched the thermostat to an emergency heat mode that used the furnace no matter what.

Ellis had been told not to goof around with the thermostat. Even if she had, as a heat pump neophyte, she wouldn't have known to go looking for such a setting. She might've dug it up in a manual. Someone could've walked her through it over the phone. Oh, well. There is heat again, which is all that matters.

They will attempt to bring the heat pump online soon. We shall see if the story ends here, or if this becomes The WTF That Wouldn't Die.

P.S. When Ellis explained the AI answering service's deceptive behavior, she was told that the agent had been universally complained about ever since they switched to it. Fed up, they told Ellis they're getting rid of it. She feels pretty chuffed about more people seeing the light concerning garbage AI that creates far more problems than it solves.

ul { list-style-type:disc; line-height:150%; margin-left: 3em; } [Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.

...sent us five wtfs. And so on anon.

Item the first, an anon is "definitely not qualified" for this job. "These years of experience requirements are getting ridiculous."

 

Item the second unearthed by a farmanon has a loco logo. "After reading about the high quality spam emails which are indistinguishable from the company's emails, I got one from the spammer just starting his first day."

 

In thrid place, anon has only good things to say: "I'm liking their newsletter recommendations so far."

 

"A choice so noice, they gave it twoice," quipped somebody.

 

And foinally, a tdwtfer asks "I've seen this mixmastered calendering on several web sites. Is there an OSS package that is doing this? Or is it a Wordpress plugin?" I have a sneaking suspicion I posted this before. Call me on it.

 

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

Antonio has an acquaintance has been seeking career advancement by proactively hunting down and fixing bugs. For example, in one project they were working on, there was a bug where it would incorrectly use MiB for storage sizes instead of MB, and vice-versa.

We can set aside conspiracy theories about HDD and RAM manufacturers lying to us about sizes by using MiB in marketing. It isn't relevant, and besides, its not like anyone can afford RAM anymore, with crazy datacenter buildouts. Regardless, which size to use, the base 1024 or base 1000, was configurable by the user, so obviously there was a bug handling that flag. Said acquaintance dug through, and found this:

const baseValue = useSI ? 1000 : 1024;

I know I have a "reputation" when it comes to hating ternaries, but this is a perfectly fine block of code. It is also correct: if you're using SI notation, you should do base 1000.

Now, given that this code is correct, you or I might say, "Well, I guess that isn't the bug, it must be somewhere else." Not this intrepid developer, who decided that they could fix it.

// const baseValue = useSI ? 1000 : 1024; baseValue = 1024 if (useSI === false) { baseValue = 1000; } if (useSI === true) { baseValue = 1024; }

It's rather amazing to see a single, correct line, replaced with ten incorrect lines, and I'm counting commenting out the correct line as one of them.

First, this doesn't correctly declare baseValue, which JavaScript is pretty forgiving about, but it also discards constness. Of course, you have to discard constness now that you've gotten rid of the ternary.

Then, our if statement compares a boolean value against a boolean literal, instead of simply if(!useSI). We don't use an else, despite an else being absolutely correct. Or actually, since we defaulted baseValue, we don't even need an else!

But of course, all of that is just glitter on a child's hand-made holiday card. The glue holding it all together is that this code just flips the logic. If we're not using SI, we set baseValue to 1000, and if we are using SI, we set it to 1024. This is wrong. This is the opposite of what the code says we should do, what words mean, and how units work.

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

While I'm not hugely fond of ORMs (I'd argue that relations and objects don't map neatly to each other, and any ORM is going to be a very leaky abstraction for all but trivial cases), that's not because I love writing SQL. I'm a big fan of query-builder tools; describe your query programatically, and have an API that generates the required SQL as a result. This cuts down on developer error, and also hopefully handles all the weird little dialects that every database has.

For example, did you know Postgres has an @> operator? It's a contains operation, which returns true if an array, range, or JSON dictionary contains your search term. Basically, an advanced "in" operation.

Gretchen's team is using the Knex library, which doesn't have a built-in method for constructing those kinds of queries. But that's fine, because it does offer a whereRaw method, which allows you to supply raw SQL. The nice thing about this is that you can still parameterize your query, and Knex will handle all the fun things, like transforming an array into a string.

Or you could just not use that, and write the code yourself:

exports.buildArrayString = jsArray => { // postgres has some different array syntax // [1,2] => '{1,2}' let arrayString = '{'; for(let i = 0; i < jsArray.length; i++) { arrayString += jsArray[i]; if(i + 1 < jsArray.length) { arrayString += ',' } } arrayString += '}'; return arrayString; }

There's the string munging we know and love. This constructs a Postgres array, which is wrapped in curly braces.

Also, little pro-tip for generating comma separated code, and this is just a real tiny optimization: before the loop append item zero, start the loop with item 1, and then you can unconditionally prepend a comma, removing any conditional logic from your loop. That's not a WTF, but I've seen so much otherwise good code make that mistake I figured I'd bring it up.

exports.buildArrayContainsQuery = (key, values) => { // TODO: do we need to do input safety checks here? // console.log("buildArrayContainsQuery"); // build the postgres 'contains' query to compare arrays // ex: to fetch files by the list of tags //WORKS: //select * from files where _tags @> '{2}'; //query.whereRaw('_tags @> ?', '{2}') let returnQueryParams = []; returnQueryParams.push(`${key} @> ?`); returnQueryParams.push(exports.buildArrayString(values)); // console.log(returnQueryParams); return returnQueryParams; }

And here's where it's used. "do we need input safety checks here?" is never a comment I like to see as a TODO. That said, because we are still using Knex's parameter handling, I'd hope it handles escaping correctly so that the answer to this question is "no". If the answer is "yes" for some reason, I'd stop using this library!

That said, all of this code becomes superfluous, especially when you read the comments in this function. I could just directly run query.whereRaw('_tags @> ?', myArray); I don't need to munge the string myself. I don't need to write a function which returns an array of parameters that I have to split back up to pass to the query I want to call.

Here's the worst part of all of this: these functions exist in a file called sqlUtils.js, which is just a pile of badly re-invented wheels, and the only thing they have in common is that they're vaguely related to database operations.

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

Arguably, the worst moment for date times was the shift from Julian to Gregorian calendars. The upgrade took a long time, too, as some countries were using the Julian calendar over 300 years from the official changeover, famously featured in the likely aprochryphal story about Russia arriving late for the Olympics.

At least that change didn't involve adding any extra months, unlike some of the Julian reforms, which involved adding multiple "intercalary months" to get the year back in sync after missing a pile of leap years.

Speaking of adding months, Will J sends us this "calendar" enum:

enum Calendar { April = 0, August = 1, December = 2, February = 3, Friday = 4, January = 5, July = 6, June = 7, March = 8, May = 9, Monday = 10, November = 11, October = 12, PublicHoliday = 13, Saturday = 14, Sunday = 15, September = 16, Thursday = 17, Tuesday = 18, Wednesday = 19 }

Honestly, the weather in PublicHoliday is usually a bit too cold for my tastes. A little later into the spring, like Saturday, is usually a nicer month.

Will offers the hypothesis that some clever developer was trying to optimize compile times: obviously, emitting code for one enum has to be more efficient than emitting code for many enums. I think it more likely that someone just wanted to shove all the calendar stuff into one bucket.

Will further adds:

One of my colleagues points out that the only thing wrong with this enum is that September should be before Sunday.

Yes, arguably, since this enum clearly was meant to be sorted in alphabetical order, but that raises the question of: should it really?

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

A good C programmer can write C in any language, especially C++. A bad C programmer can do the same, and a bad C programmer will do all sorts of terrifying things in the process.

Gaetan works with a terrible C programmer.

Let's say, for example, you wanted to see if an index existed in an array, and return its value- or return a sentinel value. What you definitely shouldn't do is this:

double Module::GetModuleOutput(int numero) { double MAX = 1e+255 ; if (this->s.sorties+numero ) return this->s.sorties[numero]; else return MAX ; }

sorties is an array. In C, you may frequently do some pointer arithmetic operations, which is why sorties+numero is a valid operation. If we want to be pedantic, *(my_array+my_index) is the same thing as my_array[my_index]. Which, it's worth noting, both of those operations de-reference an array, which means you better hope that you haven't read off the end of the array.

Which is what I suspect their if statement is trying to check against. They're ensuring that this->s.sorties+numero is not a non-zero/false value. Which, if s.sorties is uninitialized and numero is zero, that check will work. Otherwise, that check is useless and does nothing to ensure you haven't read off the end of the array.

Which, Gaetan confirms. This code works "because in practice, GetModuleOutput is called for numero == 0 first. It never de-references off the end of the array, not because of defensive programming, but because it just never comes up in actual execution.

Regardless, if everything is null, we return 1e+255, which is not a meaningful value, and should be treated like a sentinel for "no real value". None of the calling code does that, however, but also, it turns out not to matter.

This pattern is used everywhere there is arrays, except the handful of places where this pattern is not used.

Then there's this one:

if(nb_type_intervalle<1) { } else if((tab_intervalle=(double*)malloc(nb_lignes_trouvees*nb_type_intervalle*2 \ *sizeof(double)))==NULL) return(ERREUR_MALLOC);

First, I can't say I love the condition here. It's confusing to have an empty if clause. if (nb_type_intervalle>=1) strikes me as more readable.

But readability is boring. If we're in the else clause, we attempt a malloc. While using malloc in C++ isn't automatically wrong, it probably is. C++ has its own allocation methods that are better at handling things like sizes of datatypes. This code allocates memory for a large pile of doubles, and stores a pointer to that memory in tab_intervalle. We do all this inside of an if statement, so we can then check that the resulting pointer is not NULL; if it is, the malloc failed and we return an error code.

The most frustrating thing about this code is that it works. It's not going to blow up in surprising ways. I never love doing the "assignment and check" all in one statement, but I've seen it enough times that I'd have to admit it's idiomatic- to C style programming. But that bit of code golf coupled with the pointlessly inverted condition that puts our main logic in the else just grates against me.

Again, that pattern of the inverted conditional and the assignment and check is used everywhere in the code.

Gaetan leaves us with the following:

Not a world-class WTF. The code works, but is a pain in the ass to inspect and document

In some ways, that's the worst situation to be in: it's not bad enough to require real work to fix it, but it's bad enough to be frustrating at every turn.

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

"No browser is the best browser," opines Michael R. sarcastically as per usual for tdwtf. "Thank you for suggesting a browser. FWIW: neither latest Chrome, Safari, Firefox, Opera work. Maybe I should undust my Netscape."

 

An anonymous dessert lover ruminates "The icing on the cake is that it's for HR where names can be quite important. Just goes to show that not even SAP can do SAP."

 

Another anonymous dessert lover (because honestly, who isn't) cheers "2024 is back again."

 

Thrice capitalled B.J.H. capitulates. "I guess I'm not cleared to know what topic I subscribed to."

 

Jeopardy fan Jeremy P. digs a quick quiz.

It's from Britannica.com. I thought "TV remote control" because it would effectively turn off the TV. The correct answer is toaster.

 To understand what went wrong, the previous correct answer was "blunderbuss".

Apparently this is a test for clairvoyance, which will have come in handy.

For a bonus buzz, Jeremy sent in another.

This time it's "guess the novel from the picture". There was a subtle clue in this one.

You're a monster, Jeremy. 

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

It's always good to think through how any given database operation behaves inside of a transaction. For example, Faroguy inherited a Ruby codebase which was mostly db.execute("SOME SQL") without any transactions at all. This caused all sorts of problems with half-finished operations polluting the database.

Imagine Faroguy's excitement upon discovering a function called db_trans getting called in a few places. Well, one place, but that's better than none at all. This clearly must mean that at least one operation was running inside of a transaction, right?

def self.db_trans(db,stmt) db.execute(stmt) end # self.db_trans

Oh.

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

Goodhart's Law states that when a measure becomes a target, it ceases to be a good measure. Or, more to the point: you get what you measure.

If, for example, you measure code coverage, you are going to get code coverage. It doesn't mean the tests will be any good, it just means that you'll write tests that exercise different blocks of code.

For example, Capybara James sends us this unit test:

@MockitoSettings class CentralizedLoggerTest { @InjectMocks private CentralizedLogger centralizedLogger; @Test void logAround() throws Throwable { centralizedLogger = new CentralizedLogger(); MethodSignature signature = mock(MethodSignature.class); ProceedingJoinPoint joinPoint = mock(ProceedingJoinPoint.class); when(joinPoint.getSignature()).thenReturn(signature); centralizedLogger.logAround(joinPoint); Assertions.assertTrue(true); } }

It doesn't really matter what the mocks are, or what gets instantiated, or honestly, anything that's happening here. The assertion is the beginning and ending.

James writes:

The only requirement was sonar coverage to push the code to production. There is no other purpose.

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

Jon supports some software that's been around long enough that the first versions of the software ran on, and I quote, "homegrown OS". They've long since migrated to Linux, and in the process much of their software remained the same. Many of the libraries that make up their application haven't been touched in decades. Because of this, they don't really think too much about how they version libraries; when they deploy they always deploy the file as mylib.so.1.0. Their RPM post-install scriptlet does an ldconfig after each deployment to get the symlinks updated.

For those not deep into Linux library management, a brief translation: shared libraries in Linux are .so files. ldconfig is a library manager, which finds the "correct" versions of the libraries you have installed and creates symbolic links to standard locations, so that applications which depend on those libraries can load them.

In any case, Jon's team's solution worked until it didn't. They deployed a new version of the software, yum reported success, but the associated services refused to start. This was bad, because this happened in production. It didn't happen in test. They couldn't replicate it anywhere else, actually. So they built a new version of one of the impacted libraries, one with debug symbols enabled, and copied that over. They manually updated the symlinks, instead of using ldconfig, and launched the service.

The good news: it worked.

The bad news: it worked, but the only difference was that the library was built with debug symbols. The functionality was exactly the same.

Well, that was the only difference other than the symlink.

Fortunately, a "before" listing of the library files was captured before the debug version was installed, a standard practice by their site-reliability-engineers. They do this any time they try and debug in production, so that they can quickly revert to the previous state. And in this previous version, someone noticed that mylib.so was a symlink pointing to mylib.so.1.0.bkup_20190221.

Once again, creating a backup file is a standard practice for their SREs. Apparently, way back in 2019 someone was doing some debugging. They backed up the original library file, but never deleted the backup. And for some reason, ldconfig had been choosing the backup file when scanning for the "correct" version of libraries. Why?

Here, Jon does a lot of research for us. It turns out, if you start with the man pages, you don't get a answer- but you do get a warning:

ldconfig will look only at files that are named lib*.so* (for regular shared objects) or ld-.so (for the dynamic loader itself). Other files will be ignored. Also, ldconfig expects a certain pat‐
tern to how the symbolic links are set up, like this example, where the middle file (libfoo.so.1 here) is the SONAME for the library:

libfoo.so -> libfoo.so.1 -> libfoo.so.1.12

Failure to follow this pattern may result in compatibility issues after an upgrade.

Well, they followed the pattern, and they found compatibility issues. But what exactly is going on here? Jon did the work of digging straight into the ldconfig source to find out the root cause.

The version detecting algorithm starts by looking directly at filenames. While the man page warns about a convention, ldconfig doesn't validate names against this convention (which is probably the correct decision). Insetad, to find which filename has the highest version number, it scans through two filenames until finds numeric values in both of them, then does some pretty manual numeric parsing:

int _dl_cache_libcmp(const char *p1, const char *p2) { while (*p1 != '\0') { if (*p1 >= '0' && *p1 <= '9') { if (*p2 >= '0' && *p2 <= '9') { /* Must compare this numerically. */ int val1; int val2; val1 = *p1++ - '0'; val2 = *p2++ - '0'; while (*p1 >= '0' && *p1 <= '9') val1 = val1 * 10 + *p1++ - '0'; while (*p2 >= '0' && *p2 <= '9') val2 = val2 * 10 + *p2++ - '0'; if (val1 != val2) return val1 - val2; } else return 1; } else if (*p2 >= '0' && *p2 <= '9') return -1; else if (*p1 != *p2) return *p1 - *p2; else { ++p1; ++p2; } } return *p1 - *p2; }

NB: this is the version of ldconfig at the time Jon submitted this, and the version that they're using. I haven't dug through to check if this is still true in the latest version. That's an exercise for the reader.

While we have not hit the end of the first string, check if the character in that string is numeric. If it is, check if the character in the second string is numeric. If it is, keep scanning through characters, and for as long as they're numeric, keep parsing them into numbers. If the numbers aren't the same, we return the difference between them.

If the first string contains numbers at this point, but the second string doesn't, return 1. If the second string contains numbers but not the first, return -1. Otherwise, increment our pointers and go to the next character. If we reach the end of the string without finding numeric characters, return the difference between these two characters.

Also, correct me if I'm wrong, but it seems like a malicious set of filenames could cause buffer overruns here.

Now, I'll be honest, I don't have the fortitude to suggest that ldconfig is TRWTF here. It's a venerable piece of software that's solving an extremely hard problem. But boy, DLL Hell is an unending struggle and this particular solution certainly isn't helping. I'm honestly not entirely certain I'd say that there was a true WTF here, just an unfortunate confluence of people doing their best and ending up laying landmines for others.

But here's the fun conclusion: the 2019 version of the library actually had been updated. They'd deployed several new versions between 2019 and 2024, when things finally blew up. The actual deployed software kept using the backup file from 2019, and while it may have caused hard-to-notice and harder-to-diagnose bugs, it didn't cause any crashes until 2024.

.comment { border: none; } [Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.

Gretchen wanted to, in development, disable password authentication. Just for a minute, while she was testing things. That's when she found this approach to handling authentication.

passport.authenticate('local', { session: true }, async (err, user) => { if (err) { res.send({ success: false, message: 'Error authenticating user.' }) } else if (!user) { User.query() .where({ username: req.body.username }) .first() .then(targetUser => { if (targetUser) { const hash = User.hashPassword( targetUser.password_salt, req.body.password ) if (hash === targetUser.password_hash) { res.send({ success: false, message: 'Incorrect username or password.', }) } else { res.send({ success: false, message: 'Incorrect username or password.', }) } } else { res.send({ success: false, message: 'Incorrect username or password.', }) } }) .catch(err => { res.send({ success: false, message: 'Internal server error' }) }) } else if (user.firstLogin) { //...... } })(req, res, next);

passport.authenticate invokes its callback after attempting to authenticate. Now, normally, this is called as middleware on a route defined on the webserver- that is to say, you don't call it from within your application code, but as part of your routing configuration. That's not the case here, where this blob is inside of a controller.

That's weird, but let's just trace through this code. We attempt to authenticate. When the process completes, our callback function executes. If authentication failed, there's an error, so we'll send an error message. Then, if the user object isn't populated, we attempt to look up the user. If we find a user with that user name, we then hash their password and check if the hash matches. If it does, we send an error message. If it doesn't, we send an error message. If we didn't find the user, we send an error message. If anything goes wrong, we send an error message.

Wait a second, back up: if the user exists and their password matches, we send an error message?

I'll let Gretchen explain a bit more:

passport.authenticate returns an error if the authentication failed and a user object, if it succeeded. We check this immediately: if error is set, return an error message. But then, we check if the user does not exist (aka: the authentication failed).

Yes, the reason user would be null is because the authentication failed. So the error would be set. So that entire branch about validating the user won't happen: either the authentication worked and we know who the user is, or it failed, in which case we'd have an error. There's no reasonable world where there's no error but also no user object.

So yes, if authentication failed, but you manually re-run the authentication and it succeeds for some reason, yeah, you probably should still return an error. But I don't know if it's "Incorrect username or password". It probably should be "Invalid reality, please restart the universe and see if the problem persists."

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

...everything looks like a hammer.

"Where is this plane?" wondered erffrfez (hope I spelled that right), explaining "I was on a flight across Aus, and noticed that the back of seat display doesn't seem to know exactly where the plane is. There are two places where 'distance to destination' is displayed. They never matched and the difference varied through the flight." I have a suspicion this is related to the January 20 WTF.

 

Roadtripping Adam R. rued "I'm studying the time tables for the buses in the Italian alps, and I was a little dismayed to find that 19 hours will have elapsed between the 11:05 departure and 12:05 arrival. "

 

Also Adam R. in the airport shared "After my inbound flight to MAD was cancelled, AA couldn't decide if my connecting flight was arriving or departing, so it decided that it was undefineding."

 

Meanwhile B.J.H. "Earlier the monitor had accurate information about a departed flight, but they were able to fix that by bringing in a technician to get the display ready for the flight to Godot."

 

Also B.J.H. pleaded "Won't someone help LATAM airlines find the missing key so we can leave." Apparently they found it eventually.

 

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

Sandra from InitAg (previously) tries to keep the team's code quality up. The team she's on uses CI, code reviews, linting and type checking, and most important: hiring qualified people. Overall, the team's been successful recently. Recently.

The company got its start doing data-science, which meant much of the initial code was written by brilliant PhDs who didn't know the first thing about writing software. Most of that code has been retired, but it is impossible to dispatch all of it.

Which brings us to Stan. Stan was a one-man dev-team/sysadmin for a mission critical piece of software. No one else worked on it, no one else looked at it, but that was "okay" because Stan was happy to work evenings and weekends without anyone even suggesting it. Stan loved his work, perhaps a little too much. And as brilliant as Stan was, when it came to software engineering, he was at best "brillant".

Which brings us to a file called utils/file_io.py. As you might gather from the full name there, this is a "utility" module stuffed with "file and IO" related functions. Let's look at a few:

def del_rw(action, name, exc): """ See https://stackoverflow.com/questions/21261132/shutil-rmtree-to-remove-readonly-files Is used by shutil.rmtree to remove read only files as well """ os.chmod(name, stat.S_IWRITE) os.remove(name)

This, I think, is a case of "maybe this shouldn't be a function?" Given that it's only two lines, it's more clear what you're doing if you just include the lines. Also, it takes three parameters and uses one of them.

def safe_rmtree(remove_dir, minimum_parent_dir, min_parent_parts=2): """ Recursively removes all files in a directory that should contain the min parent dir :param remove_dir: :param minimum_parent_dir: :param min_parent_parts: nr of folders that the parent should contain to avoid removing eg C: :return: """ if not os.path.exists(remove_dir): print(f"WARNING in safe_rmtree: {remove_dir} does not exist, nothing is removed") return remove_path = pathlib.Path(remove_dir) minimum_parent_path = pathlib.Path(minimum_parent_dir) if not minimum_parent_path.is_dir(): raise AssertionError("Min parent dir not a valid dir {}".format(minimum_parent_dir)) if len(minimum_parent_path.parts) <= min_parent_parts: raise AssertionError( "Unsafe removal of {}: minimum parent dir is too short {}".format( remove_dir, minimum_parent_dir ) ) if minimum_parent_path in remove_path.parents: print("REMOVE {}".format(remove_dir)) shutil.rmtree(remove_dir, onerror=del_rw) # use del_rw to also remove read only files else: raise AssertionError( "Unsafe removal of {}: does not contain minimum parent dir {}".format( remove_dir, minimum_parent_dir ) ) time.sleep(0.1)

I definitely don't like this. We do at least use all of our parameters. While the documentation doesn't really make it clear what they all do, this will remove a directory if and only if the directory is contained under minimum_parent_dir, and the minimum_parent_dir is at least min_parent_parts levels deep.

I understand, broadly, why you want some checks around a potentially destructive operation, but I have to wonder about why this is the set of checks we added.

Also, why the tenth of a second sleep at the end? shutil isn't doing things on background threads, it just manipulates the file-system via syscalls.

Now, what's notable about this one is that we're using the pathlib.Path API for interacting with file paths. This is the correct way to do things in Python, which is why this method is just funny:

def file_name_from_path(path, extension=False): """ Get the name of a file (without the extension) given a path :param path: :param extension: default false, but if true fn will include extension :return: a string with the file name (optional extension) """ basename = ntpath.basename(path) if extension is True: return basename else: return os.path.splitext(basename)[0]

This reinvents methods that are already in pathlib.Path. I also like that the documentation contradicts itself: this returns the name of the file (without the extension) except when it also returns the extension.

def create_dir(new_dir, remove_data=False, verbose=False): """ Create a new directory. NOTE: this function only creates one new folder, the root folder must already exist. :param new_dir: directory to create :param remove_data: if yes, remove existing data in folder. :param verbose: :return: a string containing the new directory """ if remove_data and os.path.exists(new_dir): shutil.rmtree(new_dir) time.sleep(0.1) if os.path.exists(new_dir): if verbose: print("Output directory {} already exists".format(new_dir)) else: os.makedirs(new_dir) return new_dir

This function creates a new directory and possibly deletes the old one. Not "safely", though, but hey, that random sleep is back.

def dirs_in_dir(data_dir, contains=""): """ Get the paths of all directories in a directory containing a set of characters, the basename :param data_dir: directory for which to list all paths with a base name :param contains: characters that should be in the name of the directory :return: a list of all directory paths """ if not os.path.exists(data_dir): print("Warning in get_dirs_in_dir: directory {} does not exist".format(data_dir)) return dir_paths = [ os.path.join(data_dir, d) for d in os.listdir(data_dir) if (os.path.isdir(os.path.join(data_dir, d)) and contains.lower() in d.lower()) ] return dir_paths

This is another reinvention of functionality that already exists in pathlib.Path. I'd say, "And they already know about that!" but they clearly don't; they copied code off StackOverflow and didn't read it.

How about writing to a log file in possibly the slowest way possible?

def write_to_log(text_string, file_path, level="INFO", code="I_NOTDEFINED", entry="PROJ"): print(f"Log: {text_string}", flush=True) if not os.path.exists(file_path): file = open(file_path, "w") file.close() lookup_dict = error_lookup_dict responsible = lookup_dict[code]["responsible"] with open(file_path, "a") as file: dt_string = datetime.datetime.now().strftime("%Y-%m-%d %H:%M:%S") log_text = "{}; {}; {}; {}; {}; {}".format( dt_string, entry, level, responsible, code, text_string + "\n" ) file.write(log_text)

Most logging frameworks open a file when you start, and keep the handle open until you're done logging. This one not only checks if it exists, but reopens it again and again for every write. And while it's at it, it also prints what it's logging. And Python has a built-in logging framework that definitely handles all these features.

Finally, what file_io library would be complete without a way to call shell commands?

def run_command_window(cmd, log_path=None): std_output = [] updated_env = {**os.environ, "PYTHONPATH": str(config.repos_dir)} with Popen( cmd, stdout=PIPE, bufsize=1, universal_newlines=True, shell=sys.platform == "linux", env=updated_env, ) as p: for line in p.stdout: print(line, end="", flush=True) # process line here std_output.append(line) if log_path: with open(log_path, "a") as log: log.write(line) return p.returncode, std_output

Broadly speaking, if you're trying to automate things via Python, you might come up with something very much like this. But I draw your attention to their updated_env which populates a PYTHONPATH variable: they're calling Python code by shelling out and launching a new interpreter.

Sandra writes:

This is one of the better files in the project.
Thankfully, this one has a happy ending: management is well aware that this project is legacy garbage, and has authorized a full-scale rework to make it maintainable. Whether the guy doing the work escapes with his sanity intact is another question

Honestly, as "I don't actually know Python but I'm very smart," code goes, this isn't so bad. I mean, it's terrible, but it's clearly written. Even the parts that shouldn't have been written are clean and easy to understand. That, I suppose, just makes it worse, doesn't it. Picking through the code that needs to be thrown away will be harder than one expects because you can't just go, "kill it with fire," you have to think about what you're throwing away.

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