The Daily WTF

Subscribe to The Daily WTF feed
Curious Perversions in Information Technology
Updated: 2 hours 13 min ago

Error'd: Past Imperfect

Fri, 2024-04-05 08:30

A twitchy anonymous reporter pointed out that our form validation code is flaky. He's not wrong. But at least it can report time without needing emoticons! :-3

That same anon sent us the following, explaining "Folks at Twitch are very brave. So brave, they wrote their own time math."

 

Secret Agent Sjoerd reports "There was no train two minutes ago so I presume I should have caught it in an alternate universe." Denver is a key nexus in the multiverse, according to Myka.

 

Chilly Dallin H. is a tiny bit heated about ambiguous abbreviations - or at least about the software that interprets them with inadequate context. "With a range that short, I'd hate to take one of the older generation planes." At least it might be visible!

 

Phoney François P. "I was running a return of a phone through a big company website that shall not be named. Thankfully, they processed my order on April 1st 2024, or 2024年4月1日 in Japanese. There is a slight delay though as it shows 14月2024, which should be the 14th month of 2024. Dates are hard. Date formatting is complicated. For international date formatting, please come back later."

 

At some time in the past, the original Adam R. encountered a time slip. We're just getting to see it even now. "GitHub must be operating on a different calendar than the Gregorian. Comments made just 4 weeks ago [today is 2023-02-07] are being displayed as made last year."

 

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

CodeSOD: A Valid Applicant

Thu, 2024-04-04 08:30

In the late 90s into the early 2000s, there was an entire industry spun up to get businesses and governments off their mainframe systems from the 60s and onto something modern. "Modern", in that era, usually meant Java. I attended vendor presentations, for example, that promised that you could take your mainframe, slap a SOAP webservice on it, and then gradually migrate modules off the mainframe and into Java Enterprise Edition. In the intervening years, I have seen exactly 0 successful migrations like this- usually they just end up trying that for a few years and then biting the bullet and doing a ground-up rewrite.

That's is the situation ML was in: a state government wanted to replace their COBOL mainframe monster with a "maintainable" J2EE/WebSphere based application. Gone would be the 3270 dumb terminals, and here would be desktop PCs running web browsers.

ML's team did the initial design work, which the state was very happy with. But the actual development work gave the state sticker shock, so they opted to take the design from ML's company and ship it out to a lowest-bidder offshore vendor to actually do the development work.

This, by the way, was another popular mindset in the early 00s: you could design your software as a set of UML diagrams and then hand them off to the cheapest coder you could hire, and voila, you'd have working software (and someday soon, you'd be able to just generate the code and wouldn't need the programmer in the first place! ANY DAY NOW).

Now, this code is old, and predates generics in Java, so the use of ArrayLists isn't the WTF. But the programmer's approach to polymorphism is.

public class Applicant extends Person { // ... [snip] ... } . . . public class ApplicantValidator { public void validateApplicantList(List listOfApplicants) throws ValidationException { // ... [snip] ... // Create a List of Person to validate List listOfPersons = new ArrayList(); Iterator i = listOfApplicants.iterator(); while (i.hasNext()) { Applicant a = (Applicant) i.next(); Person p = (Person) a; listOfPersons.add(p); } PersonValidator.getInstance().validatePersonList(listOfPersons); // ... [snip] ... } // ... [snip] ... }

Here you see an Applicant is a subclass of Person. We also see an ApplicantValidator class, which needs to verify that the applicant objects are valid- and to do that, they need to be treated as valid Person objects.

To do this, we iterate across our list of applications (which, it's worth noting, are being treated as Objects since we don't have generics), cast them to Applicants, then cast the Applicant variable to Person, then create a list of Persons- which again, absent generics, is just a list of Objects. Then we pass that list of persons into validatePersonList.

All of this is unnecessary, and demonstrates a lack of understanding about the language in use. This block could be written more clearly as: PersonValidator.getInstance().validatePersonList(listOfApplicants);

This gives us the same result with significantly less effort.

While much of the code coming from the offshore team was actually solid, it contained so much nonsense like this, so many misundertandings of the design, and so many bugs, that the state kept coming back to ML's company to address the issues. Between paying the offshore team to do the work, and then ML's team to fix the work, the entire project cost much more than if they had hired ML's team in the first place.

But the developers still billed at a lower rate than ML's team, which meant the managers responsible still got to brag about cost savings, even as they overran the project budget. "Imagine how much it would have cost if we hadn't gone with the cheaper labor?"

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

CodeSOD: Gotta Catch 'Em All

Wed, 2024-04-03 08:30

It's good to handle any exception that could be raised in some useful way. Frequently, this means that you need to take advantage of the catch block's ability to filter by type so you can do something different in each case. Or you could do what Adam's co-worker did.

try { /* ... some important code ... */ } catch (OutOfMemoryException exception) { Global.Insert("App.GetSettings;", exception.Message); } catch (OverflowException exception) { Global.Insert("App.GetSettings;", exception.Message); } catch (InvalidCastException exception) { Global.Insert("App.GetSettings;", exception.Message); } catch (NullReferenceException exception) { Global.Insert("App.GetSettings;", exception.Message); } catch (IndexOutOfRangeException exception) { Global.Insert("App.GetSettings;", exception.Message); } catch (ArgumentException exception) { Global.Insert("App.GetSettings;", exception.Message); } catch (InvalidOperationException exception) { Global.Insert("App.GetSettings;", exception.Message); } catch (XmlException exception) { Global.Insert("App.GetSettings;", exception.Message); } catch (IOException exception) { Global.Insert("App.GetSettings;", exception.Message); } catch (NotSupportedException exception) { Global.Insert("App.GetSettings;", exception.Message); } catch (Exception exception) { Global.Insert("App.GetSettings;", exception.Message); }

Well, I guess that if they ever need to add different code paths, they're halfway there.

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

CodeSOD: Exceptional Feeds

Tue, 2024-04-02 08:30

Joe sends us some Visual Basic .NET exception handling. Let's see if you can spot what's wrong?

Catch ex1 As Exception ' return the cursor Me.Cursor = Cursors.Default ' tell a story MessageBox.Show(ex1.Message) Return End Try

This code catches the generic exception, meddles with the cursor a bit, and then pops up a message box to alert the user to something that went wrong. I don't love putting the raw exception in the message box, but this is hardly a WTF, is it?

Catch ex2 As Exception ' snitch MessageBox.Show(ex2.ToString(), "RSS Feed Initialization Failure") End Try

Elsewhere in the application. Okay, I also don't love the exN naming convention either, but where's the WTF?

Well, the fact that they're initializing an RSS feed is a hint- this isn't an RSS reader client, it's an RSS serving web app. This runs on the server side, and any message boxes that get popped up aren't going to the end user.

Now, I haven't seen this precise thing done in VB .Net, only in Classic ASP, where you could absolutely open message boxes on the web server. I'd hope that in ASP .Net, something would stop you from doing that. I'd hope.

Otherwise, I've found the worst logging system you could make.

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

Taking Up Space

Mon, 2024-04-01 08:30

April Fool's day is a day where websites lie to you or create complex pranks. We've generally avoided the former, but have done a few of the former, but we also like to just use April Fool's as a chance to change things up.

So today, we're going to do something different. We're going to talk about my Day Job. Specifically, we're going to talk about a tool I use in my day job: cFS.

cFS is a NASA-designed architecture for designing spaceflight applications. It's open source, and designed to be accessible. A lot of the missions NASA launches use cFS, which gives it a lovely proven track record. And it was designed and built by people much smarter than me. Which doesn't mean it's free of WTFs.

The Big Picture

cFS is a C framework for spaceflight, designed to run on real-time OSes, though fully capable of running on Linux (with or without a realtime kernel), and even Windows. It has three core modules- a Core Flight Executive (cFE) (which provides services around task management, and cross-task communication), the OS Abstraction Layer (helping your code be portable across OSes), and a Platform Support Package (low-level support for board-connected hardware). Its core concept is that you build "apps", and the whole thing has a pitch about an app store. We'll come back to that. What exactly is an app in cFS?

Well, at their core, "apps" are just Actors. They're a block of code with its own internal state, that interacts with other modules via message passing, but basically runs as its own thread (or a realtime task, or whatever your OS appropriate abstraction is).

These applications are wired together by a cFS feature called the "Core Flight Executive Software Bus" (cFE Software Bus, or just Software Bus), which handles managing subscriptions and routing. Under the hood, this leverages an OS-level message queue abstraction. Since each "app" has its own internal memory, and only reacts to messages (or emits messages for others to react to), we avoid pretty much all of the main pitfalls of concurrency.

This all feeds into the single responsibility principle, giving each "app" one job to do. And while we're throwing around buzzwords, it also grants us encapsulation (each "app" has its own memory space, unshared), and helps us design around interfaces- "apps" emit and receive certain messages, which defines their interface. It's almost like full object oriented programming in C, or something like how the BeamVM languages (Erlang, Elixir) work.

The other benefit of this is that we can have reusable apps which provide common functionality that every mission needs. For example, the app DS (Data Storage) logs any messages that cross the software bus. LC (Limit Checker) allows you to configure expected ranges for telemetry (like, for example, the temperature you expect a sensor to report), and raise alerts if it falls out of range. There's SCH (Scheduler) which sends commands to apps to wake them up so they can do useful work (also making it easy to sleep apps indefinitely and minimize power consumption).

All in all, cFS constitutes a robust, well-tested framework for designing spaceflight applications.

Even NASA annoys me

This is TDWTF, however, so none of this is all sunshine and roses. cFS is not the prettiest framework, and the developer experience may ah… leave a lot to be desired. It's always undergoing constant improvement, which is good, but still has its pain points.

Speaking of constant improvement, let's talk about versioning. cFS is the core flight software framework which hosts your apps (via the cFE), and cFS is getting new versions. The apps themselves also get new versions. The people writing the apps and the people writing cFS are not always coordinating on this, which means that when cFS adds a breaking change to their API, you get to play the "which version of cFS and App X play nice together". And since everyone has different practices around tagging releases, you often have to walk through commits to find the last version of the app that was compatible with your version of cFS, and see things like releases tagged "compatible with Draco rc2 (mostly)". The goal of "grab apps from an App Store and they just work" is definitely not actually happening.

Or, this, from the current cFS readme:

Compatible list of cFS apps
The following applications have been tested against this release:
TBD

Messages in cFS are represented by structs. Which means when apps want to send each other messages, they need the same struct definitions. This is just a pain to manage- getting agreement about which app should own which message, who needs the definition, and how we get the definition over to them is just a huge mess. It's such a huge mess that newer versions of cFS have switched to using "Electronic Data Sheets"- XML files which describe the structs, which doesn't really solve the problem but adds XML to the mix. At least EDS makes it easy to share definitions with non-C applications (popular ground software is written in Python or Java).

Messages also have to have a unique "Message ID", but the MID is not just an arbitrary unique number. It secretly encodes important information, like whether this message is a command (an instruction to take action) or telemetry (data being output), and if you pick a bad MID, everything breaks. Also, keeping MID definitions unique across many different apps who don't know any of the other apps exist is a huge problem. The general solution that folks use is bolting on some sort of CSV file and code generator that handles this.

Those MIDs also don't exist outside of cFS- they're a unique-to-cFS abstraction. cFS, behind the scenes, converts them to different parts of the "space packet header", which is the primary packet format for the SpaceWire networking protocol. This means that in realistic deployments where your cFS module needs to talk to components not running cFS- your MID also represents key header fields for the SpaceWire network. It's incredibly overloaded and the conversions are hidden behind C macros that you can't easily debug.

But my biggest gripe is the build tooling. Everyone at work knows they can send me climbing the walls by just whispering "cFS builds" in my ear. It's a nightmare (that, I believe has gotten better in newer versions, but due to the whole "no synchronization between app and cFE versions" problem, we're not using a new version). It starts with make, which calls CMake, which also calls make, but also calls CMake again in a way that doesn't let variables propagate down to other layers. cFS doesn't provide any targets you link against, but instead requires that any apps you want to use be inserted into the cFS source tree directly, which makes it incredibly difficult to build just parts of cFS for unit testing.

Oh, speaking of unit testing- cFS provides mocks of all of its internal functions; mocks which always return an error code. This is intentional, to encourage developers to test their failure paths in code, but I also like to test our success path too.

Summary

Any tool you use on a regular basis is going to be a tool that you're intimately familiar with; the good parts frequently vanish into the background and the pain points are the things that you notice, day in, day out. That's definitely how I feel after working with cFS for two years.

I think, at its core, the programming concepts it brings to doing low-level, embedded C, are good. It's certainly better than needing to write this architecture myself. And for all its warts, it's been designed and developed by people who are extremely safety conscious and expect you to be too. It's been used on many missions, from hobbyist cube sats to Mars rovers, and that proven track record gives you a good degree of confidence that your mission will also be safe using it.

And since it is Open Source, you can try it out yourself. The cFS-101 guide gives you a good starting point, complete with a downloadable VM that walks you through building a cFS application and communicating with it from simulated ground software. It's a very different way to approach C programming (and makes it easier to comply with C standards, like MISRA), and honestly, the Actor-oriented mindset is a good attitude to bring to many different architectural problems.

Peregrine

If you were following space news at all, you may already know that our Peregrine lander failed. I can't really say anything about that until the formal review has released its findings, but all indications are that it was very much a hardware problem involving valves and high pressure tanks. But I can say that most of the avionics on it were connected to some sort of cFS instance (there were several cFS nodes).

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

Error'd: Good Enough Friday

Fri, 2024-03-29 07:30

We've got some of the rarer classic Error'd types today: events from the dawn of time, weird definitions of space, and this absolutely astonishing choice of cancel/confirm button text.

Perplexed Stewart found this and it's got me completely befuddled as well! "Puzzled over this classic type of Error'd for ages. I really have no clue whether I should press Yes or No."

 

I have a feeling we've seen errors like this before, but it bears repeating. Samuel H. bemoans the awful irony. "While updating Adobe Reader: Adobe Crash Processor quit unexpectedly [a.k.a. crashed]."

 

Cosmopolitan Jan B. might be looking for a courier to carry something abroad. "I found an eBay listing that seemed too good to be true, but had no bids at all! The item even ships worldwide, except they have a very narrow definition of what worldwide means."

 

Super-Patriot Chris A. proves Tennessee will take second place to nobody when it comes to distrusting dirty furriners. Especially the ones in Kentucky. "The best country to block is one's own. That way, you KNOW no foreigners can read your public documents!"

 

Finally, old-timer Bruce R. has a system that appears to have been directly inspired by Aristotle. "I know Windows has some old code in it, but this is ridiculous."

 

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

CodeSOD: Sorts of Dates

Thu, 2024-03-28 07:30

We've seen loads of bad date handling, but as always, there's new ways to be surprised by the bizarre inventions people come up with. Today, Tim sends us some bad date sorting, in PHP.

// Function to sort follow-ups by Date function cmp($a, $b) { return strcmp(strtotime($a["date"]), strtotime($b["date"])); } // Sort the follow-ups by Date usort($data, "cmp");

The cmp function rests in the global namespace, which is a nice way to ensure future confusion- it's got a very specific job, but has a very generic name. And the job it does is… an interesting approach.

The "date" field in our records is a string. It's a string formatted in YYYY-MM-DD HH:MM:SS, and this is a guarantee of the inputs- which we'll get to in a moment. So the first thing that's worth noting is that the strings are already sortable, and nothing about this function needs to exist.

But being useless isn't the end of it. We convert the string time into a Unix timestamp with strtotime, which gives us an integer- also trivially sortable. But then we run that through strcmp, which converts the integer back into a string, so we can do a string comparison on it.

Elsewhere in the code, we use usort, passing it the wonderfully named $data variable, and then applying cmp to sort it.

Unrelated to this code, but a PHP weirdness, we pass the callable cmp as a string to the usort function to apply a sort. Every time I write a PHP article, I learn a new horror of the language, and "strings as callable objects" is definitely horrifying.

Now, a moment ago, I said that we knew the format of the inputs. That's a bold claim, especially for such a generically named function, but it's important: this function is used to sort the results of a database query. That's how we know the format of the dates- the input comes directly from a query.

A query that could easily be modified to include an ORDER BY clause, making this whole thing useless.

And in fact, someone had made that modification to the query, meaning that the data was already sorted before being passed to the usort function, which did its piles of conversions to sort it back into the same order all over again.

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

CodeSOD: Never Retire

Wed, 2024-03-27 07:30

We all know that 2038 is going to be a big year. In a mere 14 years, a bunch of devices are going to have problems.

Less known is the Y2030 problem, which is what Ventsislav fighting to protect us from.

//POPULATE YEAR DROP DOWN LISTS for (int year = 2000; year <= 2030; year++) { DYearDropDownList.Items.Add(year.ToString()); WYearDropDownList.Items.Add(year.ToString()); MYearDropDownList.Items.Add(year.ToString()); } //SELECT THE CURRENT YEAR string strCurrentYear = DateTime.Now.Year.ToString(); for (int i = 0; i < DYearDropDownList.Items.Count; i++) { if (DYearDropDownList.Items[i].Text == strCurrentYear) { DYearDropDownList.SelectedIndex = i; WYearDropDownList.SelectedIndex = i; MYearDropDownList.SelectedIndex = i; break; } }

Okay, likely less critical than Y2038, but this code, as you might guess, started its life in the year 2000. Clearly, no one thought it'd still be in use this far out, yet… it is.

It's also worth noting that the drop down list object in .NET has a SelectedValue property, so the //SELECT THE CURRENT YEAR section is unnecessary, and could be replaced by a one-liner.

With six years to go, do you think this application is going to be replaced, or is the year for loop just going to change to year <= 2031 and be a manual change for the rest of the application's lifetime?

I mean, they could also change it so it always goes to currentYear or currentYear + 1 or whatever, but do we really think that's a viable option?

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

CodeSOD: Exceptional String Comparisons

Tue, 2024-03-26 07:30

As a general rule, I will actually prefer code that is verbose and clear over code that is concise but makes me think. I really don't like to think if I don't have to.

Of course, there's the class of WTF code that is verbose, unclear and also really bad, which Thomas sends us today:

Private Shared Function Mailid_compare(ByVal queryEmail As String, ByVal FnsEmail As String) As Boolean Try Dim str1 As String = queryEmail Dim str2 As String = FnsEmail If String.Compare(str1, str2) = 0 Then Return True Else Return False End If Catch ex As Exception End Try End Function

This VB .Net function could easily be replaced with String.Compare(queryEmail, FnsEmail) = 0. Of course, that'd be a little unclear, and since we only care about equality, we could just use String.Equals(queryEmail, FnsEmail)- which is honestly clearer than having a method called Mailid_compare, which doesn't actually tell me anything useful about what it does.

Speaking of not doing anything useful, there are a few other pieces of bloat in this function.

First, we plop our input parameters into the variables str1 and str2, which does a great job of making what's happening here less clear. Then we have the traditional "use an if statement to return either true or false".

But the real special magic in this one is the Try/Catch. This is a pretty bog standard useless exception handler. No operation in this function throws an exception- String.Compare will even happily accept null references. Even if somehow an exception was thrown, we wouldn't do anything about it. As a bonus, we'd return a null in that case, throwing downstream code into a bad state.

What's notable in this case, is that every function was implemented this way. Every function had a Try/Catch that frequently did nothing, or rarely (usually when they copy/pasted from StackOverflow) printed out the error message, but otherwise just let the program continue.

And that's the real WTF: a codebase polluted with so many do-nothing exception handlers that exceptions become absolutely worthless. Errors in the program let it continue, and the users experience bizarre, inconsistent states as the application fails silently.

Or, to put it another way: this is the .NET equivalent of classic VB's On Error Resume Next, which is exactly the kind of terrible idea it sounds like.

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

CodeSOD: Contains a Substring

Mon, 2024-03-25 07:30

One of the perks of open source software is that it means that large companies can and will patch it for their needs. Which means we can see what a particular large electronics vendor did with a video player application.

For example, they needed to see if the URL pointed to a stream protected by WideVine, Vudu, or Netflix. They can do this by checking if the filename contains a certain substring. Let's see how they accomplished this…

int get_special_protocol_type(char *filename, char *name) { int type = 0; int fWidevine = 0; int j; char proto_str[2800] = {'\0', }; if (!strcmp("http", name)) { strcpy(proto_str, filename); for(j=0;proto_str[j] != '\0';j++) { if(proto_str[j] == '=') { j++; if(proto_str[j] == 'W') { j++; if(proto_str[j] == 'V') { type = Widevine_PROTOCOL; } } } } if (type == 0) { for(j=0;proto_str[j] != '\0';j++) { if(proto_str[j] == '=') { j++; if(proto_str[j] == 'V') { j++; if(proto_str[j] == 'U') { j++; if(proto_str[j] == 'D') { j++; if(proto_str[j] == 'U') { type = VUDU_PROTOCOL; } } } } } } } if (type == 0) { for(j=0;proto_str[j] != '\0';j++) { if(proto_str[j] == '=') { j++; if(proto_str[j] == 'N') { j++; if(proto_str[j] == 'F') { j++; if(proto_str[j] == 'L') { j++; if(proto_str[j] == 'X') { type = Netflix_PROTOCOL; } } } } } } } } return type; }

For starters, there's been a lot of discussion around the importance of memory safe languages lately. I would argue that in C/C++ it's not actually hard to write memory safe code, it's just very easy not to. And this is an example- everything in here is a buffer overrun waiting to happen. The core problem is that we're passing pure pointers to char, and relying on the strings being properly null terminated. So we're using the old, unsafe string functions to never checking against the bounds of proto_str to make sure we haven't run off the edge. A malicious input could easily run off the end of the string.

But also, let's talk about that string comparison. They don't even just loop across a pair of strings character by character, they use this bizarre set of nested ifs with incrementing loop variables. Given that they use strcmp, I think we can safely assume the C standard library exists for their target, which means strnstr was right there.

It's also worth noting that, since this is a read-only operation, the strcpy is not necessary, though we're in a rough place since they're passing a pointer to char and not including the size, which gets us back to the whole "unsafe string operations" problem.

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

Error'd: You Can Say That Again!

Fri, 2024-03-22 07:30

In a first for me, this week we got FIVE unique submissions of the exact same bug on LinkedIn. In the spirit of the theme, I dug up a couple of unused submissions of older problems at LinkedIn as well. I guess there are more than the usual number of tech people looking for jobs.

John S., Chris K., Peter C., Brett Nash and Piotr K. all sent in samples of this doublebug. It's a flubstitution AND bad math, together!

 

Latin Steevie is also job hunting and commented "Well, I know tech-writers may face hard times finding a job, so they turn to LinkedIn, which however doesn't seem to help... (the second announcement translates to 'part-time cleaners wanted') As a side bonus, apparently I can't try a search for jobs outside Italy, which is quite odd, to say the least!"

 

Clever Drew W. found a very minor bug in their handling of non-ASCII names. "I have an emoji in my display name on LinkedIn to thwart scrapers and other such bots. I didn't think it would also thwart LinkedIn!"

 

Finally, Mark Whybird returns with an internal repetition. "I think maybe I found the cause of some repetitive notifications when I went to Linkedin's notifications preferences page?" I think maybe!

 

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

CodeSOD: Reading is a Safe Operation

Thu, 2024-03-21 07:30

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

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

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

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

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

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

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

CodeSOD: Do you like this page? Check [Yes] or [No]

Wed, 2024-03-20 07:30

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

CodeSOD: A Debug Log

Tue, 2024-03-19 07:30

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

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

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

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

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

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

CodeSOD: How About Next Month

Mon, 2024-03-18 07:30

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

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

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

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

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

One of the commits in the release described the change:

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

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

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

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

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

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

Error'd: Can't Be Beat

Fri, 2024-03-15 07:30

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

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

 

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

 

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

 

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

 

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

 

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

CodeSOD: Query the Contract Status

Thu, 2024-03-14 07:30

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

The actual code architecture is also a mess.

For example, this code lives in the Java portion.

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

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

But that's just the aperitif.

The PHP front end has code that looks like this:

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

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

Rui is looking forward to the end of this contract.

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

Check Your Email

Wed, 2024-03-13 07:30

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

"Did we change anything this weekend?"

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

"Well, something must have changed?!"

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

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

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

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

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

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

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

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

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

CodeSOD: Wait for the End

Tue, 2024-03-12 07:30

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

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

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

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

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

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

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

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

CodeSOD: Some Original Code

Mon, 2024-03-11 07:30

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

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

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

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

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

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

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

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

Pages