It's Thanksgiving day in the US, and today, I'm thankful I'm not the person who had to spend the weekend hastily attaching baffles to 650 doors in a skyscraper because no one thought about how motion sensors worked. Original --Remy

It was a heck of a party and everyone was invited, from the executive vice president to the janitorial staff. There was champagne, shrimp, cake, and even a string quartet. There were door prizes, balloons, and all sorts of bank-branded knickknacks being given away. And it was all for good reason: the bank had just completed its high-tech, sixty-five story downtown corporate headquarters, and it was the tallest building within a three-hundred mile radius.

Virtually no expense was spared for the bank's skyscraper: a renowned architect was commissioned to design the building, skilled artisans adorned the corridors with marble statues, acoustical consultants made sure the lobby had just the right echo, and, most importantly, the world's foremost security firm was brought in to lock things down tighter than Fort Knox. It was considered less of a building and more of a work of art. The pinnacle of this creation was the high-tech sliding doors used throughout the building; this was the first time that StarTrek-esque doors were used on such a large scale.

Everyone thought the doors were incredibly cool. Oh, and they were. Upon entering a secure area (that is, anywhere except the lobby), one simply waved his RFID-enabled access card across the sensor and the doors slid open almost instantly. When leaving an area, motion detectors automatically opened up the doors. The only thing that was missing was the cool "whoosh" noise and an access panel that could be shot with a phaser to permanently seal or, depending on the plot, automatically open the door. Despite that flaw, the doors just felt secure.

That is, until one of G.R.G.'s colleagues had an idea. He grabbed one of those bank-branded folding yardsticks from the freebie table and headed on over to one of the security doors. He slipped the yardstick right through where the sliding doors met and the motion detector promptly noticed the yardstick and opened the door. He had unfettered access to the entire building thanks to a free folding yardstick.

Immediately following the party, the bank announced that they would delay the opening "due to an unforeseen delay." When G.R.G. returned the following week, he noticed that the doors -- all 650 throughout the building -- were hastily retrofitted with poorly-cut metal flanges to cover up the small slit between the doors. Looks like it's back to relying on phasers to breach the security.

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

https://thedailywtf.com/articles/classic-wtf-insecurity-doors

Enterprise software development gets a bad rap, especially here, because "good code" isn't really a goal of an enterprise. Solving business problems is. And no enterprise is more enterprise than a government, and no government is more government than the US Federal government.

Which brings us to today's anonymous submitter, who wanted to keep up on current events in US politics. While watching some recent videos of Senate proceedings, our submitter got bored watching (as one would), they pulled up the browser tools. And that's where our WTF comes from.

So, in this code sample, each Senate committee has a different Akamai URL and ID for its videos. This is how the developer chose to represent that:

var streamInfo = new Array
	(
		["ag", "76440", "https://ag-f.akamaihd.net", "2036803", "agriculture"],
		["aging", "76442", "https://aging-f.akamaihd.net", "2036801", "aging"],
		["approps", "76441", "https://approps-f.akamaihd.net", "2036802", "appropriations"],
		["armed", "76445", "https://armed-f.akamaihd.net", "2036800", "armedservices"],
		…,
		["arch", "", "https://ussenate-f.akamaihd.net/"],
		["uscp", "", "", "2043685", "uscp"],
		["cio", "", "", "2043686", "cio"]
	);

Snipped for brevity. Ah, arrays containing arrays (where the inner arrays are really objects). That's always fun. What's extra fun is the way they search this.

for (var i = 0; i for (var j = 0; j < streamInfo[i].length; j++){
			if (comm === false ){
				break;
			}else if (streamInfo[i][j] === comm) {
					var streamNum = streamInfo[i][j+1];
					var streamDomain = streamInfo[i][j+2];
					var streamID = streamInfo[i][j+3];
					var msl3 = streamInfo[i][j+4];
					break;
			}else {
				var streamArch = streamInfo[i][j+2];
				break;
			}
	}
}

The outer loop, quite reasonably, iterates across the array. The inner loop then iterates across the inner array. Well, it sort of would, if not for the fact that every branch in the loop breaks out of it. Because the goal isn't to loop at all, the goal is to check the first field in the inner array, and see if it matches the committee we're searching for. Then we populate some variables with the results- which has the added benefit of doing some variable hoisting which means that those variables exist in some larger scope, helping with the lack of readability. And I have no idea what's happening with streamArch, which points to the same field as streamDomain, but because of the outer loop, gets reset every time, meaning it'll just be an empty string, which is the last thing in the array.

As our submitter put it: "US Federal procurement hits the expected quality level."

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

https://thedailywtf.com/articles/enterprise-streaming

Relational Databases and No-SQL Databases take two key different philosophies, by and large. No-SQL is hard to talk about in broad terms, as it's mostly a set of unrelated technologies all solving the data storage problem in different ways. But we can still make some broad generalizations.

In No-SQL-land, we mostly store data the way we plan to query it. Ad-hoc queries are likely a bad choice, if they're even allowed. In RDBMSes, we store data according to a platonic ideal of normal forms, driven by the data in our domain. We can query the data however we like, using a language that only declares the data we want, not how to get it. Indexes and views and other behind-the-scenes structures make the query efficient.

Neither approach is wrong or bad or better, they are tuned to different ways of thinking about the problem. But the "invisible performance" of RDBMSes means that developers can invent new footguns without even knowing it.

Dotan had a database query that went from "performing fine" to "performing poorly" in the space of a few hours. This was the query:

select bars.*, DATE_FORMAT(bars.date_entered,'%Y-%m-%d %h:%i') as entered, 
	sales_reps.name as rep_name, 
	case monthly_sales when 'Not in business yet' then 'no business' 
		else monthly_sales end as new_monthly_sales, 
	left(bars.name,20) as new_name, left(bars.company,15) as new_company, 
	left(bars.email,23) as new_email, 
	(select max(action_date) from bar_action 
		where bar_action.bar_id=bars.id) as modify_date, 
	(select count(distinct foo_id) from bar_foos 
		where bar_foos.bar_id=bars.id) as total_foos, 
	bar_action.id AS credit_report_uploaded, 
	credit_report_requests.id AS credit_report_requested, 
	applications.id as app_id 
from bars LEFT JOIN sales_reps ON bars.rep_id=sales_reps.id 
	LEFT JOIN applications ON bars.id=applications.bar_id 
	LEFT JOIN bar_action ON 
		(bars.id=bar_action.bar_id AND action_desc='credit_report_uploaded') 
	LEFT JOIN credit_report_requests ON bars.id=credit_report_requests.bar_id 
WHERE bars.is_deleted =0 and 
	bars.status IN ('O/LU','O/LAU','O/LA','O/LAA','IP/L','IP/LA','H/','IP/N') 
ORDER BY CASE assign_date WHEN NULL THEN bars.date_entered 
	WHEN '0000-00-00 00:00:00' THEN bars.date_entered 
	ELSE assign_date END DESC LIMIT 0,5

I've broken it up across lines, because as found, it was all on one line.

Now, the query itself maybe doesn't have any explicit WTFs in it. It's hard, with SQL, to see what's wrong from the query alone. But there are definitely smells. It's weird that so many fields get lefted- pulling out the leftmost substring. It's weird that monthly_sales is a text data type (since it can hold Not in business yet), and that we have to convert it to no business for formatting. It's weird that there's a bars.* at the top, then a few more fields from bars that get included. It's weird that we're doing some summaries using correlated subqueries (the max and count queries).

None of these things are inherently wrong though.

Dotan supplied a little more background, though. Each table listed there has hundreds of thousands of rows in it. Note that LIMIT 0,5 at the end: starting from the zeroth row, take only 5 rows. Again, querying across all these large tables to get only five results isn't wrong, but it's suspicious.

Dotan adds: "Not a single index on the poor database."

And that's your real WTF. The query itself isn't a disaster- it's not great, but by the standards of "terrible SQL queries", I've seen far worse. But the database doesn't have any indexes, at all. The miracle is that this wasn't already running poorly years earlier.

But the query is still bad, though it's hard to say exactly how bad without knowing what it actually needs to return for the application. Dotan can give us a ballpark though:

This could have been done in a single simple query that fits on two lines of my text editor.

So in addition to being a terrible database that isn't leveraging the main feature SQL databases need for performance, the query is definitely over-engineered.

We can assume Dotan replaced this query with his shorter version, after a round of testing to make sure he wasn't introducing any regressions. As to whether or not any indexes ever got added, we can only hope.

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

https://thedailywtf.com/articles/sql-with-no-equal

We recently discussed the challenges of testing legacy code. Willy sent us some code from tests for a newer Java application.

Now, one of the main advantages, in my opinion, about unit-tests is that they serve as documentation. They tell you what the method does, and how it expects to be called. A good unit test is far better than a large pile of comments.

A bad unit test (or function supporting a unit test, as in this example) has the opposite effect.

public boolean checkDataByFilter(Map filterValue, boolean equal) {
       boolean same = true;
       boolean different = false;
       try {
           Thread.sleep(3000);
       }
       catch (Exception e) {}
       List> mapList = getData();
       for (Map map : mapList) {
           for (String key : filterValue.keySet()) {
               if (!map.containsKey(key) || !map.get(key).equals(filterValue.get(key))) {
                   if (equal) {
                       same = false;
                   }
                   else {
                       different = true;
                   }
                   break;
               }
           }
           if (!same || different) {
               break;
           }
       }
       return equal ? same : different;
   }

The core of this function is that it fetches data using getData and compares the results against filterValue, key by key. Then it returns true or false based on whether on not they match.

The first problem is that Thread.sleep(3000) call. That is very much a code smell in a unit test, it's doubly a code smell in a function that exists to support a unit test. If your unit test needs to sleep, well first off, it's not a unit test any more. But even if we admit the sleep, this clearly isn't where it belongs. The unit test method should worry about that kind of plumbing, not a supporting method which exists to perform a check. Speaking of, getData shouldn't happen here either.

But the real issue is the equal parameter and its implementation. Because this method can return true if the values are all the same, or if at least one value is different.

Now, first, I question the very idea of a method that negates its output based on a parameter you pass into it. That seems like a bad idea. But the logic required for this method is just a simple negation- return equal ? same : !same would accomplish the same goal.

The end result is that we have a method which makes our tests harder to understand, not easier. In the end, I wonder if the original developer just wanted to demonstrate their thorough understanding of what is the same, and different.

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

https://thedailywtf.com/articles/it-s-not-that-different

Joshua O. repeats "Looks like this week will really be a case of the Mondays!"

Regular reader Argle Bargle reiterates "I know MSN is trying to use an AI to filter inappropriate comments. (See my comment about AI really SI = Simulated Intelligence.) The Daily WTF at least has Real Intelligence, so this is a plus. Disqus seems to do better letting the community monitor. But I'm left with laughing at a powerful entity (MSN) that just seems overly confused. Attached is my latest. Switch under for over and it works. The original generates a 'something went wrong' message, but participants have already figured out what's really happening. I suppose the daily WTF shouldn't do repeats like this, but IMHO, such a big organization needs mocking for such garbage on a perpetual basis."

Coincidentally, commenter Robert A. complains "I gave up posting comments on the Daily WTF weeks ago, because I never see them posted. What does held for moderation even mean? What are the rules? Does moderation ever occur? It’s like Andor getting out of prison." As the esteemed Mr Bargle states above, TDWTF uses only "Real Intelligence" to filter the comments. And as you should have realized by now, there is a grave, grave shortage of anything remotely approximating that rare substance anywhere in this country right now.

Neutral Patrick G. submits "I must have missed the news that the US and Switzerland have merged into one country."

Finally, rider Alex recounts "Minus 12 percent full, on the Coast Starlight. Are we filling that train with anti-people?" I suspect the truth is that it's -12% full, ergo, 12% empty, ergo 88% full.

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

https://thedailywtf.com/articles/monday-monday

Chris was given a program and told to improve it by adding tests. That was a good start to a terrible experience. Just getting it to build was a chore, as the build files had absolute paths hard-coded into them.

But the real problem Chris ran in to is that it's hard to write tests for something if no one knows what it does or why it does it. The application had no documentation. No comments, aside from commented out blocks of dead code. But comments aside, there was no other documentation. Which left Chris with methods like this to work with:

public int priority()
{
  switch (this.type)
  {
  case 1:
  case 2:
    return -1;
  case 3:
  case 4:
    return -1;
  case 5:
    return -1;
  case 6:
    return 1;
  case 7:
    return 2;
  case 8:
  case 9:
    return 3;
  case 10:
  case 11:
  case 12:
    return 4;
  case 13:
    return 7;
  case 14:
    return 7;
  case 15:
    return 8;
  case 16:
    return 9;
  case 17:
  case 18:
    return 15;
  case 19:
  case 20:
    return -1;
  }
  return -1;
}

It's easy to write a test that confirms what this function does, certainly, but it's very difficult to write a test that proves this function is correct. What even is priority in this context? Is -1 meant to be some sort of error code or is it a very low (or very high?) priority? Actually, for that matter, what is type? While we're at it, why do we sometimes collapse cases together and sometimes don't?

There may be good answers to these questions, though probably not. But Chris certainly didn't get those answers as part of the project. At this point, the application has plenty of tests which confirm the application does what it currently does, which is better than no tests at all, but leaves a lot to be desired.

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

https://thedailywtf.com/articles/switching-your-test-cases

Wim works on a web app with a problem. Specifically, the error log is the fastest growing file on the system. Well, perhaps that's not the problem, but actually a symptom. Like so many applications, it's a PHP web app with a MySQL backend, and the previous developer made… choices.

$sqlisgt = "insert into ser_gen_tj values (4, '$type_juridiction', '$enr[23]', 'O')";

There's your SQL injection vulnerability. Just dump variable values directly into SQL statements, what could go wrong?

Well, one problem is that sometimes this application needed to handle names. Names, especially in French, frequently contain '. So this wouldn't work:

$sql = "INSERT INTO personne VALUES ('$matricule','$nom','$prenom','$tel',Null);";

A single quote in $nom would break the query, it'd become syntactically invalid. And that's why the log file was the fastest growing set of data in the system. But the developer responsible "fixed" this, don't you worry.

$sql = "INSERT INTO personne VALUES ('$matricule',\"$nom\",\"$prenom\",'$tel',Null);";

Thank goodness no one has a " in their name, I suppose. Still, Little Bobby Tables is going to have a field day with this application. Or should I say, Petit Robert D'Tables.

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

https://thedailywtf.com/articles/d-tables

An anonymous submitter from Hungary reached out with both some bad code, and a story behind it.

Hungary's school system runs on a software package called KR'ETA. The Sawarim$ hacker group felt that the software was badly designed and left millions of students personal information unprotected- so they hacked in to prove it. The company running the software responded in the worst possible way- by attempting to cover up the breach and pretending nothing ever happened. It's quite the news story.

The hacking group, not interested in releasing any students' private information, instead released the C# source code. Our Anonymous submitter reviewed some of that code, and sends us one method from it.

/// 

/// Removes all elements that could cause problems with input fields
/// To be used with with LoadWithFilter
/// 
/// 
/// 
public static string PreventSQLInjection(string dirtytext)
{
    List<string> disallowedtags = new List<string>()
    {
        "'", " or ", " OR ", " and ", " AND ",
        "=", "<", ".", "<>", " not ", " NOT "
    };
    //theoretically because I trim at first space there can be no AND, OR, or NOT left
    string cleartext = dirtytext;
    
    if (cleartext.Contains(" "))
    {
        cleartext = cleartext.Substring(0, cleartext.IndexOf(" ") + 1).Trim();
    }
    
    foreach (string tag in disallowedtags)
    {
        cleartext = cleartext.Replace(tag, "");
    }
    
    return cleartext;
}

With a method called PreventSQLInjection, I can't imagine how the hackers got in.

I suspect that all of the comment is nonsense- the return value and the parameter are incorrectly documented, I wouldn't think that the summary is correct either. And the very existence of this method tells us that there's something wrong- they're almost certainly building SQL queries via string concatenation.

But, let's trace through this garbage. We start by defining a set of disallowedTags. Which, you'll note, are done in both lower and upper case, because string comparisons are case sensitive by default.

Then, we check if the text includes a space. If it does, we substring the entire string up to that space, including the space, then Trim the space off. This gives us a cleartext value which contains no spaces. Then we iterate through our disallowedtags, Replaceing each substring of our spaceless cleartext with an empty string. Which, also means many of our disallowedtags are never replaced, as they contain spaces. This is something the developer appears to be aware of, at least "theoretically".

"Replacing invalid strings" is always a bad way to prevent SQL injection attacks, because it means you're building queries through string concatenation instead. But this is an exceptionally bad version of this.

Now, our anonymous submitter isn't the sort of person who downloads code posted by hacker groups on Telegram, which is probably for the best. They found this in a discussion of the source. So our anonymous submitter adds:

The best part? According to someone who claims to have downloaded the full source code, this function is never called.

Surely, they must have a better way to protect against SQL injections that they're calling instead. Right? Right?

They don't

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

https://thedailywtf.com/articles/hungry-for-an-education

As we frequently observe, code in scientific applications tends not to have the best code quality. And it's not hard to understand why: a researcher is frequently trying to take a wholistic understanding of a complicated problem domain and turn it into code. Software engineering, on the other hand, teaches us to break that wholistic picture up into little parts that we glue back together. The latter approach produces better code, but requires a lot of thinking about the structure of code, which is a whole layer on top of that wholistic understanding.

Which brings us to Plink, a C program Oliver R has had to work with. It analyzes genetic variance data. It also has a method with this signature:

int32_t plink(char* outname, char* outname_end, char* bedname, 
    char* bimname, char* famname, char* cm_map_fname, 
    char* cm_map_chrname, char* phenoname, char* extractname, 
    char* excludename, char* keepname, char* removename, 
    char* keepfamname, char* removefamname, char* filtername, 
    char* freqname, char* distance_wts_fname, char* read_dists_fname, 
    char* read_dists_id_fname, char* evecname, char* mergename1, 
    char* mergename2, char* mergename3, char* missing_mid_template, 
    char* missing_marker_id_match, char* makepheno_str, 
    char* phenoname_str, Two_col_params* a1alleles, 
    Two_col_params* a2alleles, char* recode_allele_name, 
    char* covar_fname, char* update_alleles_fname, 
    char* read_genome_fname, Two_col_params* qual_filter, 
    Two_col_params* update_chr, Two_col_params* update_cm, 
    Two_col_params* update_map, Two_col_params* update_name, 
    char* update_ids_fname, char* update_parents_fname, 
    char* update_sex_fname, char* loop_assoc_fname, 
    char* flip_fname, char* flip_subset_fname, char* sample_sort_fname,
    char* filtervals_flattened, char* condition_mname, 
    char* condition_fname, char* filter_attrib_fname, 
    char* filter_attrib_liststr, char* filter_attrib_sample_fname, 
    char* filter_attrib_sample_liststr, char* rplugin_fname, 
    char* rplugin_host_or_socket, int32_t rplugin_port, 
    double qual_min_thresh, double qual_max_thresh, double thin_keep_prob,
    double thin_keep_sample_prob, uint32_t new_id_max_allele_len, 
    uint32_t thin_keep_ct, uint32_t thin_keep_sample_ct, 
    uint32_t min_bp_space, uint32_t mfilter_col, uint32_t fam_cols, 
    int32_t missing_pheno, char* output_missing_pheno, 
    uint32_t mpheno_col, uint32_t pheno_modifier, 
    Chrom_info* chrom_info_ptr, Oblig_missing_info* om_ip, 
    Family_info* fam_ip, double check_sex_fthresh, 
    double check_sex_mthresh, uint32_t check_sex_f_yobs, 
    uint32_t check_sex_m_yobs, double distance_exp, double min_maf, 
    double max_maf, double geno_thresh, double mind_thresh, 
    double hwe_thresh, double tail_bottom, double tail_top, 
    uint64_t misc_flags, uint64_t filter_flags, uint64_t calculation_type, 
    uint32_t dist_calc_type, uintptr_t groupdist_iters, 
    uint32_t groupdist_d, uintptr_t regress_iters, uint32_t regress_d, 
    uint32_t parallel_idx, uint32_t parallel_tot, uint32_t splitx_bound1, 
    uint32_t splitx_bound2, uint32_t ppc_gap, uint32_t sex_missing_pheno, 
    uint32_t update_sex_col, uint32_t hwe_modifier, uint32_t min_ac, 
    uint32_t max_ac, uint32_t genome_modifier, double genome_min_pi_hat, 
    double genome_max_pi_hat, Homozyg_info* homozyg_ptr, 
    Cluster_info* cluster_ptr, uint32_t neighbor_n1, uint32_t neighbor_n2, 
    Set_info* sip, Ld_info* ldip, Epi_info* epi_ip, Clump_info* clump_ip, 
    Rel_info* relip, Score_info* sc_ip, uint32_t recode_modifier, 
    uint32_t allelexxxx, uint32_t merge_type, uint32_t sample_sort, 
    int32_t marker_pos_start, int32_t marker_pos_end, 
    int32_t snp_window_size, char* markername_from, char* markername_to, 
    char* markername_snp, Range_list* snps_range_list_ptr, 
    uint32_t write_var_range_ct, uint32_t covar_modifier, 
    Range_list* covar_range_list_ptr, uint32_t write_covar_modifier, 
    uint32_t write_covar_dummy_max_categories, uint32_t dupvar_modifier, 
    uint32_t mwithin_col, uint32_t model_modifier, uint32_t model_cell_ct, 
    uint32_t model_mperm_val, uint32_t glm_modifier, 
    double glm_vif_thresh, uint32_t glm_xchr_model, 
    uint32_t glm_mperm_val, Range_list* parameters_range_list_ptr, 
    Range_list* tests_range_list_ptr, double ci_size, double pfilter, 
    double output_min_p, uint32_t mtest_adjust, double adjust_lambda, 
    uint32_t gxe_mcovar, Aperm_info* apip, uint32_t mperm_save, 
    uintptr_t ibs_test_perms, uint32_t perm_batch_size, double lasso_h2, 
    uint32_t lasso_lambda_iters, double lasso_minlambda, 
    Range_list* lasso_select_covars_range_list_ptr, 
    uint32_t testmiss_modifier, uint32_t testmiss_mperm_val, 
    uint32_t permphe_ct, int32_t known_procs, 
    Ll_str** file_delete_list_ptr) 

Whitespace added for readability.

That's well over a hundred parameters for one function call, with a mix of literal values and pointers being passed in. When invoked, some of the values passed in are already pointers, some are converted to pointers using the & operator, which makes it extra hard to understand where the data actually lives in the program:

retval = plink(outname, outname_end, pedname, mapname, famname, cm_map_fname, cm_map_chrname, phenoname, extractname, excludename, keepname, removename, keepfamname, removefamname, filtername, freqname, distance_wts_fname, read_dists_fname, read_dists_id_fname, evecname, mergename1, mergename2, mergename3, missing_mid_template, missing_marker_id_match, makepheno_str, phenoname_str, a1alleles, a2alleles, recode_allele_name, covar_fname, update_alleles_fname, read_genome_fname, qual_filter, update_chr, update_cm, update_map, update_name, update_ids_fname, update_parents_fname, update_sex_fname, loop_assoc_fname, flip_fname, flip_subset_fname, sample_sort_fname, filtervals_flattened, condition_mname, condition_fname, filter_attrib_fname, filter_attrib_liststr, filter_attrib_sample_fname, filter_attrib_sample_liststr, rplugin_fname, rplugin_host_or_socket, rplugin_port, qual_min_thresh, qual_max_thresh, thin_keep_prob, thin_keep_sample_prob, new_id_max_allele_len, thin_keep_ct, thin_keep_sample_ct, min_bp_space, mfilter_col, fam_cols, missing_pheno, output_missing_pheno, mpheno_col, pheno_modifier, &chrom_info, &oblig_missing_info, &family_info, check_sex_fthresh, check_sex_mthresh, check_sex_f_yobs, check_sex_m_yobs, distance_exp, min_maf, max_maf, geno_thresh, mind_thresh, hwe_thresh, tail_bottom, tail_top, misc_flags, filter_flags, calculation_type, dist_calc_type, groupdist_iters, groupdist_d, regress_iters, regress_d, parallel_idx, parallel_tot, splitx_bound1, splitx_bound2, ppc_gap, sex_missing_pheno, update_sex_col, hwe_modifier, min_ac, max_ac, genome_modifier, genome_min_pi_hat, genome_max_pi_hat, &homozyg, &cluster, neighbor_n1, neighbor_n2, &set_info, &ld_info, &epi_info, &clump_info, &rel_info, &score_info, recode_modifier, allelexxxx, merge_type, sample_sort, marker_pos_start, marker_pos_end, snp_window_size, markername_from, markername_to, markername_snp, &snps_range_list, write_var_range_ct, covar_modifier, &covar_range_list, write_covar_modifier, write_covar_dummy_max_categories, dupvar_modifier, mwithin_col, model_modifier, (uint32_t)model_cell_ct, model_mperm_val, glm_modifier, glm_vif_thresh, glm_xchr_model, glm_mperm_val, ¶meters_range_list, &tests_range_list, ci_size, pfilter, output_min_p, mtest_adjust, adjust_lambda, gxe_mcovar, &aperm, mperm_save, ibs_test_perms, perm_batch_size, lasso_h2, lasso_lambda_iters, lasso_minlambda, &lasso_select_covars_range_list, testmiss_modifier, testmiss_mperm_val, permphe_ct, known_procs, &file_delete_list);

Whitespace not added.

Again, this is written with a different philosophy than a software engineer would. It's not a mistake that the code is this way, the developer was clearly smart and optimizing for writing code that represents their understanding of the problem as quickly as possible. But it doesn't mean that I would ever want to support this.

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

https://thedailywtf.com/articles/the-parameters

With a minimum of curated snark this week, you get only the freshest goods.

Unpreserved jeffphi reports "My friend Jason mentioned this to me. I assume if you select these options, your pickle state will collapse only after observing your delivered goods."

Original Henrik H. highlights "Apparently cache invalidation is still hard, will it ever be refreshed?"

Self-styled The Beast in Black v0t3d for this submission: "I checked the 2022 US general election results for PLACE; I should have WORD. No screenshots were harmed in the making of this errord."

Regular reader Anon E. Mouse "Credly.com is disappointed that I am already signed in. Next time I won't sign in." We're disappointed too, Anonnie.

Finally, diligent mouserover Rick P. discovered "Their design standards must require that every icon must have Help text".

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

https://thedailywtf.com/articles/quantum-ferment

The saying goes that debugging is like solving a murder mystery, where you are both the victim and the killer. This, of course, is a bit inaccurate, since we rarely are the only programmer working on a project.

Cole Tobin brings us a small mystery in C# today, though in this case, the mystery is- how does this even work?

string localDir = "null";
string filePath;
if (save_to_Temp)
{
    dir = Path.GetDirectoryName(engineeringDataPushPath);
}
else
{
#if PRODUCTION
    dir = @"S:\Documents\calibration\Calibration Certs\" + modelNumber + "\\";
#else
    dir = @"calibration cert" + "\\";
#endif
}
if (localDir != null)
    dir = localDir;

The middle of this code is a bit of static about trying to guess where a file should get stored, with bonuses for a hard-coded reference to a mapped network drive when running in production, which I'm sure will never cause problems. Even better, they take advantage of the @ "verbatim string" operator, which disables escape characters- mostly. Each string gets a "\\" appended to the end, which could be a @"\" instead.

But that's just bad code. None of that is a mystery. The key lines that frame the mystery are these:

string localDir = "null";
...
if (localDir != null)
    dir = localDir;

The string "null" is never going to be equal to the value null. So dir is going to always be overwritten by "null".

So yes, obvious bug, the code doesn't work, very funny, but where is the mystery?

Cole writes:

Somehow, despite the last if always evaluating to true, this piece of code somehow worked…

And that's the mystery. This code somehow works- or at least, the code as a whole works. As it's impossible for this code to work, as written, we must assume that this code isn't actually calculating the output path. There's some other code block, potentially more correct, that does the real work.

When you have eliminated the impossible, whatever remains, however improbable, must be the real WTF.

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

https://thedailywtf.com/articles/a-null-mystery

2020 was a rough year for everyone. Eventually, it will end, but as we drag through to the end of 2020.2, it's sometimes hard to remember the beforetimes.

Sometime in that long era before 2020, Marc's team patched a bug in their software. Like good developers, the patch included a unit test. They could prove that the test failed before the fix, passed after the fix, and happily closed the ticket and went on to different things.

In early 2020, Chris, one of the members of the team went on to a position elsewhere. A few months after that- the bug reappeared.

But how? All the tests were showing green. Marc pulled up the code and found that the test had been disabled by Chris shortly before quitting. The commit message was simply: "Fixed?".

It was easy to re-enable the test, but a bit harder to track down the regression. The even harder conversation was amending their processes to make sure a commit like that never got merged ever again.

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

https://thedailywtf.com/articles/fixed-the-bug

My Steam profile doesn't represent the most active gamer on Earth, but in my time I put some pretty serious hours into the recent XCOM games. Each mission your squad goes on gets a procedurally generated name, and that can lead to some funny results just by itself (like "Avenging Avenger"). But, if you've put too many hours into the game, you might notice that every once in awhile, you see a mission with "Chicken" in its name.

Now, it's obvious that this is just a joke. Clearly an Easter Egg hidden in the code by a programmer. But Rich D picked through the UnrealScript which powers the game, finding the code responsible.

class XGMission extends Actor 
	abstract;

var string                      m_strHelp;          // The help string that displays when this mission is selected in MC
var float                       fAnimationTimer;
var bool                m_bRetaliation;
var localized array m_aFirstOpName;
var localized array m_aSecondOpName;
var localized array m_aFirstOpWord;
var localized array m_aSecondOpWord;
var localized string m_strOpAvenger;
var localized string m_strOpAshes;
var localized string m_strOpRandom;
var localized string m_strOpRandomWord;
var localized string m_strChicken;	// I just want to use "Chicken" every once in a while
var localized string m_strTitle;     // The mission's name
var localized string m_strSituation; // A string describing the narrative of the mission
var localized string m_strObjective; // A string describing the objective of the mission
var string m_strOpenExclamationMark; // A string for the open exclamation mark for Spanish language "!"
var string m_strTip;

static function string GenerateOpName(optional bool bTutorial = false)
{
	local XGParamTag kTag;	
	local int iSelection, iTop, iBottom;
	local bool bUseChicken;
	kTag = XGParamTag(`XEXPANDCONTEXT.FindTag("XGParam"));
	bUseChicken = Rand(500) == 0;
	// Choose which random op name generator to use
	if( Rand(2) == 0 )
	{
		kTag.StrValue0 = default.m_aFirstOpName[Rand(default.m_aFirstOpName.Length)];
		kTag.StrValue1 = default.m_aSecondOpName[Rand(default.m_aSecondOpName.Length)];
		if( bUseChicken )
		{
			kTag.StrValue1 = default.m_strChicken;
		}
		return `XEXPAND.ExpandString(default.m_strOpRandom);	
	}
	else
	{
		iSelection = Rand(default.m_aFirstOpWord.Length);
		kTag.StrValue0 = default.m_aFirstOpWord[iSelection];
		iSelection = Rand(default.m_aSecondOpWord.Length);
		kTag.StrValue1 = default.m_aSecondOpWord[iSelection];
		if( bUseChicken )
		{
			kTag.StrValue0 = default.m_strChicken;
		}
		// Make sure the first and second word of the operation name are not the same
		if(kTag.StrValue1 == kTag.StrValue0 )
		{
			iTop = default.m_aSecondOpWord.Length - (iSelection+1);
			iBottom = iSelection;
			if( iTop >= iBottom )
			{
				kTag.StrValue1 = default.m_aSecondOpWord[(iSelection+1) + Rand(iTop)];
			}
			else
			{
				kTag.StrValue1 = default.m_aSecondOpWord[Rand(iBottom)];
			}
		}
		
		return `XEXPAND.ExpandString(default.m_strOpRandomWord);
	}
	
}
defaultproperties
{
}

One of the localization files sets m_strChicken equal to chicken. And then we have the key line, bUseChicken = Rand(500) == 0- 0.2% of the time, part of the mission name string will contain the word "Chicken".

There isn't really a WTF here, just a fun bit of code. Sometimes, you just want to use chicken.

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

https://thedailywtf.com/articles/what-are-you-chicken

In the US, we "fell back"- that is to say we abandoned Daylight Saving Time and moved back to Standard Time, and I think the general consensus is that time changes are the real WTF.

So let's use this as a moment to look at some bad date handling code, an evergreen topic.

First, we turn to Adam, who's doing some minor work on an old PHP application.

# Yes this is really how you do date constants in PHP now :-(
define('XXXCutoffDate',      (new DateTime(YEAR . '-01-19 00:00:00', new DateTimeZone('America/Chicago')))->getTimestamp() );

This isn't inherently how you have to do date constants, but it's certainly a way to do date constants. Somehow, despite going the route of using constructors and objects, the original developer still managed to find a way to work string concatenation into the process, making this possibly the most PHP of all PHP code.

From Tomasz, we get another classic anti-pattern.

    public function getEidtTimeRemaining($createDate, $timeout)
    {
        $oDb = $this->getDefaultAdapter();
        $sql = 'SELECT NOW() AS now';
        $row = $oDb->query($sql)->fetch();
        $now = strtotime($row['now']);
       
        return $timeout - ($now - strtotime($createDate));
    }

Yep, the good ol' "run a database query to know what time it is". If only there were some other way to ensure all your front-end servers knew what time it was. No, we must do a database query.

Someone should probably eidt that code.

Finally, one of Noah's predecessors was thinking about datawarehousing. It's frequently common there to generate tables that have thousands of dates as records, so you can easily join against them, making time range queries easy.

So they implemented a table that looked like this:

CREATE TABLE [dbo].[date_dim](
        [Date_key] [int] IDENTITY(1,1) NOT NULL,
        [full_date] [datetime] NOT NULL,
        [day_of_week] [smallint] NOT NULL,
        [day_num_in_month] [smallint] NOT NULL,
        [day_num_overall] [smallint] NOT NULL,
        [day_name] [varchar](9) NOT NULL,
        [day_abbrev] [char](3) NOT NULL,
        [weekday_flag] [char](1) NOT NULL,
        [week_num_in_year] [smallint] NOT NULL,
        [week_num_overall] [smallint] NOT NULL,
        [week_begin_date] [datetime] NOT NULL,
        [week_begin_date_key] [smallint] NOT NULL,
        [month] [smallint] NOT NULL,
        [month_num_overall] [smallint] NOT NULL,
        [month_name] [varchar](9) NOT NULL,
        [month_abbrev] [char](3) NOT NULL,
        [quarter] [smallint] NOT NULL,
        [year] [smallint] NOT NULL,
        [yearmo] [int] NOT NULL,
        [fiscal_month] [smallint] NOT NULL,
        [fiscal_quarter] [smallint] NOT NULL,
        [fiscal_year] [int] NULL,
        [fiscal_period] [char](20) NULL,
        [last_day_in_month_flag] [char](1) NOT NULL,
        [same_weekday_year_ago_date] [datetime] NOT NULL,
        [days_in_month] [numeric](6, 4) NOT NULL,
        [char_full_date] [char](8) NOT NULL,
 CONSTRAINT [PK_Dates] PRIMARY KEY CLUSTERED
(
        [Date_key] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]

This table had dates from 1900 to 2078 in it, and that's not a terrible idea, honestly. But instead of using it to make date-based queries more efficient, they instead used it to reinvent all of the built-in date functions of SQL Server, like this date diff function.

create FUNCTION [dbo].[cal_day_diff]

(@lcStartDate CHAR(8),
@lcEndDate CHAR(8))

RETURNS INT

BEGIN
RETURN  (select count(*)
                        from date_dim
                        where char_full_date between @lcStartDate AND @lcEndDate
                                AND day_of_week BETWEEN 1 AND 7)

END

Dates, as always, are hard.

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

https://thedailywtf.com/articles/the-falling-dates

Amateur mathematician Chris F. thoughtfully sums up this screencap. "Although Numberblocks is a brilliant show for kids learning to count, I can't help but feel the BBC perhaps need to watch it themselves when it comes to the order of the series (or 'seasons' as many of your readers may know them) in the show..."

In a similar vein, Countist Allie C. figures "Either Github forgot how to count, or I did." On the thrid hand, Allie, it's correct as unary.

With a bit of equal time for the competition Chris B. counters "On second thoughts, gitlab, let's cancel the cancelation of the comment."

Sensing weakness, Marc W"urth piles on. "Seems for GitLab 20 minutes and 20 minutes aren't the same." I vaguely recall having seen this before; are we bullying poor gitlab?

Finally, infrequent commenter Daniel D. has sent us a head-scratcher. We'll welcome any insights into what kind of architectural horror must drive an error like this. Says Daniel:
"I use this local e-shop from time to time and they have recently moved to a new system. Now I am unable to change or delete my previous delivery addresses as this would crash their database.
The message Prevedenie tejto akcie ... translates to English as Performing this action would cause inconsistency in the table dbo.Expedition, column ' (where no column is listed!)

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

https://thedailywtf.com/articles/foreign-keys

Cassi continues the fight against ancient, creaky PHP code.

Today, it's particularly ancient- according to source control, it predates PHP 5.0 (and thus structured exceptions, and in this case may predate the current century). So the fact that this sets a pass-by-reference $errMsg variable and returns false on an error isn't the WTF in the code.

	$infoPtr->mysqlName = $row["mysqlName"];
	if (!$infoPtr->mysqlName) {
		$errMsg = "No table named \"" . $infoPtr->mysqlName ."\"";
		return(false);
    }

The surrounding code supports dynamically generating queries (through string concatenation, naturally), but does its best to do some validation of those inputs. For example, it attempts to confirm that the table we're about to query exists in the database.

So we read the data out of the $row in the database, and if the resulting value is false-y (we'd expect it to be empty), we want to generate an error message… using that empty value. So no matter the inputs, the output would be No table named "".

This code is as it was when first added to source control. There have been no modifications, which means this has never correctly generated output. The weird irony of this is that the only way the error would say anything else is if you had a table named false, which is also a case where you shouldn't have an error to begin with.

Cassi is refactoring this code to add actual exceptions and clean up these mistakes, but this has been in the code base for longer than most of our careers.

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

https://thedailywtf.com/articles/nothing-wagered-nothing-errored

Mark was trawling through the WordPress code, tracking down a bug in an extension, when he noticed this particular method for deciding whether or not WordPress is served via HTTPS or not.

function is_ssl() {
        if ( isset($_SERVER['HTTPS']) ) {
                if ( 'on' == strtolower($_SERVER['HTTPS']) )
                        return true;
                if ( '1' == $_SERVER['HTTPS'] )
                        return true;
        } elseif ( isset($_SERVER['SERVER_PORT']) && ( '443' == $_SERVER['SERVER_PORT'] ) ) {
                return true;
        }
        return false;
}

So, the first half of this is ugly but forgivable, given the problems of truthiness. We first check if the HTTPS server variable is set to either a 1 or an on. According to the PHP docs, any non-empty value means it's running on HTTPS, so it's technically an incorrect check. I strongly suspect, though, that it's a workaround for a badly behaving web server- I see a world where some server running PHP sets that variable to off or something equally inane, violating the spec.

Which brings us to the elseif, which likely is the same thing- a compatibility hack for a badly behaving web server. If we're running on port 443, assume SSL. Is this an out-and-out mistake? Well, probably not. If we're getting picky about specifications, 443 is officially reserved for SSL, so you shouldn't be running anything but an SSL enabled web server on that port. So it's a good thing nobody runs things on non-standard ports ever.

I wouldn't call this particular function a WTF, but the problem it's solving- a messy, ugly, disastrous landscape of badly configured servers, coupled with a product that's meant to be easy to install and use for non-technical people, leads to a function that just raises my blood pressure and makes me stressed to look at. I don't like it.

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

https://thedailywtf.com/articles/on-ssl

Sometimes, I have to wonder if I'm the dummy. Today I want to take a look at two examples from readers that are pretty close to code I've written, and I'm left wondering: am I the real WTF?

meowcow moocat writes:

This little gem of mathematic wizardry was in a shell script designed to check the existence of a pid file. If pid file was younger than an hour - do nothing. If older than an hour - delete it. The fantastic bit about the script was how the guy worked out seconds in an hour.

The offending code:

MAX_SECONDS = $((60*60*1))

I don't hate this. I understand that the *1 at the end is superfluous, but it also makes the intent a bit more clear. Sure, I could just make it 3600, but then I have to remember that 3600 is the number of seconds in an hour, and I hate that. I'd probably give that 1 a variable name, like LIFETIME_HOURS or something, but I like including the extra operation, for clarity. It's not the most efficient, but how many times is this line actually invoked? I'd rather have something more readable.

Michael on the other hand, found some code that needs to limit file uploads to 20MB. This was the responsible code:

// files must be less than 20MB
if (round($uploadedFile->getSize() / 1024 / 1024) > 20) {
   [ ... throw some error message ]
}

Okay, I have some problems here, though I'm not sure I consider them full on WTFs- specifically the round. But the general idea of spamming some 1024s in there doesn't bother me. I'd multiply instead of divide: if ($uploadedFile->getSize() > 20 * 1024 * 1024).

Again, better naming around constants and variables would be good, but I've written code very much like that.

So today's article either doesn't have a real WTF, or I am the real WTF.

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

https://thedailywtf.com/articles/not-a-wtf

Ashton works for a large bank, on an application that handles millions of dollars of transactions. The previous developer left this behind, which has everyone on the team scratching their heads.

if (BatchProcessor.ProcessBatch())
{

}
else
{

}

It seems obvious that the if is unnecessary. But it's there. It was clearly put there for a reason. No one wants to remove it, for fear of breaking some invisible connection in the code. Maybe it's just superstition, and I'm sure it will eventually be removed or perhaps replaced with actual error handling, but for the moment, everyone is just hesitant to touch something that works and they don't understand.

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

https://thedailywtf.com/articles/the-bad-batch

Prescriptivist Phil pronounces "Alas, I've been saying these wrong my whole life. But at least my way I can say it in one breath." At least your way I can say it within my remaining lifespan, Phil!

A different Phil sums up this image, postulating "I must have skipped this day of Set Theory"

Breaking the run of Phils, this item from Adam offers a can't lose get-rich-quick scheme. Sign us up!

Dapper Michael R. delivers: Colourful is the new black . Since black is the color which absorbs all the visible wavelengths, is it so wrong to consider that every other color is just a subcategory of black?

And finally, regular reader Argle Bargle admits "This isn't a true WTF. It's in some doc I'm finding quite useful, but someone's going to want to be the frist to comment on it." He's quite right that it's not really a WTF but it clears the low bar for "mildly amusing" so we'll permit it.

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

https://thedailywtf.com/articles/paint-it-black