I’ll give an example. At my previous company there was a program where you basically select a start date, select an end date, select the system and press a button and it reaches out to a database and pulls all the data following that matches those parameters. The horrors of this were 1. The queries were hard coded.
-
They were stored in a configuration file, in xml format.
-
The queries were not 1 entry. It was 4, a start, the part between start date and end date, the part between end date and system and then the end part. All of these were then concatenated in the program intermixed with variables.
-
This was then sent to the server as pure sql, no orm.
-
Here’s my favorite part. You obviously don’t want anyone modifying the configuration file so they encrypted it. Now I know what you’re thinking at some point you probably will need to modify or add to the configuration so you store an unencrypted version in a secure location. Nope! The program had the ability to encrypt and decrypt but there were no visible buttons to access those functions. The program was written in winforms. You had to open the program in visual studio, manually expand the size of the window(locked size in regular use) and that shows the buttons. Now run the program in debug. Press the decrypt button. DO NOT EXIT THE PROGRAM! Edit the file in a text editor. Save file. Press the encrypt button. Copy the encrypted file to any other location on your computer. Close the program. Manually email the encrypted file to anybody using the file.
A program that HR had built so that all employees could they their payment receipts online
The username was the companies’ email address, the password was a government personal id code that you can lookup online, a don’t change, and you can’t update the password to something else.
So I told the director of HR this was a bad idea. She told me I was overreacting until I showed her her own receipt, then she finally understood that this is a really fucking bad idea.
Okay, so now she out me in charge of debugging that program.
So I setup a meeting with the director of the company they hired, he came by with the developer: a 21 yo girl who I think hadn’t finished college yet. Great start! Apparently it was her idea to do the authentication like that so that explains a few things.
So we dive in to the code.
First of all, the “passwords” were stored in blank, no hashing, no encryption, nothing. That wasn’t the worst.
For the authentication she made a single query to check if the user email existed. Of that was true, then step two was a second query to see if the password existed. If that were true, the email had been authenticated.
So let’s say, hypothetically, that they had actual passwords that people could change… I could still login with the email from anyone, and then use MY OWN password to authenticate.
This just blew my mind so hard that I don’t think I ever fully recovered, I still need treatment. The stupidity hurts
I wouldnt blame that on stupidity as much as on ignorance and naivety. Many people simply don’t think about anybody deliberately misusing their design. The idea that somebody could even want to access somebody elses receipts didn’t occur to them. And if they were still doing their studies they might not have known that you can “combine” SQL queries and ask for two things at once.
I don’t blame the girl, but whoever chose her to design a system with sensitive information.
I don’t blame a girl for doing a job that lands her food on the table. I blame the guy employing her because she’s the cheapest option
Having said that, this design was so bad that she should not have been doing any of this. If you don’t know that SQL allows you to select multiple columns then by all means, do a tutorial, it’s not that hard.
If you don’t even know what encryption is, that passwords need hashing and what not, then you should really question what you’re doing
OPs question was about the worst code I’ve seen, that was the worst I’ve seen
If you don’t even know what encryption is, that passwords need hashing and what not, then you should really question what you’re doing
I agree with your point, but I would phrase it more generally: when we’re assigned a task in a problem space we are unfamiliar with, we should always take some time to research that space before designing our solution.
After all, if we don’t know what encryption or password hashing are, how could we know that we need to learn about them first? But spending just a couple hours one morning reading about password and authentication management would have given the developer a good sense of best practices.
So she either, A) didn’t think to familiarize herself with a new topic prior to working on it, or B) did read about it and ignored general industry guidance. Both of those options are more problematic to me than simply not knowing specific things. Those are process problems that need to be addressed to build her skills as a developer.
But ultimately, in my opinion, this is really all the fault of the cheapass director who didn’t want to pay any experienced professionals to handle the task.
It wouldn’t take much google-fu to get a worked example of good authentication in whatever language. She can’t have tried, she must have just gone “programming 104 covered how to SQL, I can use that”
XML-DOM page templates stored in a database, line by line.
So rendering a page started with:
select * from pages
where page_id = ‘index’
order by line_number asc;
Each line of XML from each record was appended into a single string. This string was then XSLT transformed to HTML, for every page load.
This has to be one of the worst ways to reinvent a filesystem that I’ve ever heard. At the very least, storing static data in an relational database at this scale should be a slappable offense.
The session data, that would have been fantastic to have in a relational, queryable, reliable and trustable format was stored as a single giant string of PHP pickled data structure in a session file associated with the users cookie id.
This sentence has made me violently ill. Please take it back.
The architect sending a pointer over an API, in hexadecimal string format. char *c = “71E4F33B” just cast it on the right structure bro.
Just to add, we only did C/C++, on windows mfc, in a monolithic software.
I spent quite some time assuring myself that I was not the insane person before bringing it up with him.
A memory pointer? So it must have been a program sending a pointer using an API to itself so it ends up in the same process again?
A raw memory pointer.
Mine was very much like that, but they also deleted the pointer after sending it, but before receiving it for good measure.
That makes it for one of those “rare production bugs” 😁
Oh boy, this one was a doozy…
Was working at a very big company named after a rainforest on smart home products with integrations for a certain home assistant…
New feature was being built that integrates the aforementioned home assistant with customer’s printers so they can ask the assistant to print stuff for them.
The initial design lands from our partner team with a Java backend service fairly nicely integrated with some CUPS libraries for generating the final document to be sent to the customer’s printer. All good.
They are about to launch when… uh oh… the legal team notices an AGPL licensed package in one of the CUPS library’s dependencies that was absolutely required for the document format needed by the project and the launch is cancelled.
So the team goes off in a panic looking for alternatives to this library and can’t find any replacements. After a month or two they come back with their solution…
Instead of converting the document directly in the backend service with the linked CUPS library (as AGPL is a “forbidden license” at this company) the backend uploads the initial document to an S3 bucket, then builds a CUPS document conversion bash shell script using some random Java library, the shell script is then sent (raw) to a random blank AWS host that comes prepackaged with CUPS binaries installed (these hosts were not automated with CI/CD / auto updates as was usually mandated by company practice because updating them might remove the CUPS binaries, so they required a ton of manual maintenance over the service’s lifetime…), the bash shell script is then executed on that “clean” host, downloading the document from S3, converting it via the CUPS command line binary, then reuploading it to another S3 bucket where the Java backend picks it up and continues the process of working the document through the whole backend pipeline of various services until it got to the customer’s printer.
This seemed to satisfy the legal team at the very least, and I have no doubt is probably still in production today…
The kicker though? After all those months of dev work from a whole team (likely all on 6 figure salaries), and all the time spent by various engineers including myself on maintenance and upkeep on that solution after it was transferred to us?
An alternative, completely unrestricted corporate license was available for the package in question for about $100 per year so long as you negotiated it with the maintainers.
But that was a completely unacceptable and avoidable cost according to upper management…
For info for those not familiar with minor open source licences: AGPL is descended from GPL 3, and is properly copyleft. No surprise it is poison to Amazon
Wait 100 per year total or 100 per seat per year? If it’s per seat I can understand, if it’s total wtf…
$100 total, per year… as a FOSS enthusiast, it made me very angry that such a rich company was so petty over such a small cost for a product that raked in multiple millions of dollars per year 😾
I wonder if they would have bought it had the dev priced it at $10k/year
Yeah that’s fucked up. From two perspectives 1. Who ever wrote that library needs money to survive. 2. From the company point of view they wasted WAY more money on the development then the license. Hell if 1 developer spent a day to do it, they paid more than they would for the license
The first time something goes wrong with that complicated setup, it probably pays for a
decadehalf a century or more of it’s fee.
There was something like
# sleep for about a second on modern processors math.factorial(10000)After it was found we left it in the code but commented out along with a
sleep(1)for posterity.In the readme: if you want this program to be usable, press the turbo button until the turbo light is OFF.
I saw one where the program ran a busy loop on startup to calculate how long it took. Then it used that as an iterations-to-seconds conversion for busy loops between scheduled actions.
That’s atleast pretty creative
There was a website where users could request something or other, like a PDF report. Users had a limited number of tokens per month.
The client would make a call to the backend and say how many tokens it was spending. The backend would then update their total, make the PDF, and send it.
Except this is stupid. First of all, if you told it you were spending -1 tokens, it would happily accept this and give you a free token along with your report.
Second of all, why is the client sending that at all? The client should just ask and the backend should figure out if they have enough credit or not.
I agree but I would say if there are variable token costs depending on report it would be nice if client sent request to server, server calculates x tokens to be used, sends x to client, client confirms that’s acceptable, server does work.
Like if I expected a report to be 2 tokens but because of some quirk or a typo or something it cost 200 tokens I would like a chance to cancel it if it’s not worth it.
I don’t have any specific examples, but the standard of code is really bad in science. I don’t mean this in an overly judgemental way — I am not surprised that scientists who have minimal code specific education end up with the kind of “eh, close enough” stuff that you see in personal projects. It is unfortunate how it leads to code being even less intelligible on average, which makes collaboration harder, even if the code is released open source.
I see a lot of teams basically reinventing the wheel. For example, 3D protein structures in the Protein Database (pdb) don’t have hydrogens on them. This is partly because that’ll depend a heckton on the pH of the environment that the protein is. Aspartic acid, for example, is an amino acid where its variable side chain (different for each amino acid) is CH2COOH in acidic conditions, but CH2COO- in basic conditions. Because it’s so relative to both the protein and the protein’s environment, you tend to get research groups just bashing together some simple code to add hydrogens back on depending on what they’re studying. This can lead to silly mistakes and shabby code in general though.
I can’t be too mad about it though. After all, wanting to learn how to be better at this stuff and to understand what was best practice caused me to go out and learn this stuff properly (or attempt to). Amongst programmers, I’m still more biochemist than programmer, but amongst my fellow scientists, I’m more programmer than biochemist. It’s a weird, liminal existence, but I sort of dig it.
First of all, lack of ORM isn’t bad. It’s not a good or bad thing to use them out not use them. What’s bad is not sanitizing your query inputs and you don’t need an ORM to do that.
I think the worst thing I’ve seen is previous devs not realize there’s a cost to opening a DB connection. Especially back when DBs were on spinning rust. So the report page that ran one query to get the all the items to report on, then for each row ran another individual query to get that row’s details was probably one of the slowest reports I’ve ever seen. Every DB round trip was at minimum 0.1 seconds just to open the connection, run the query, send back the data, then close the connection. So 10 rows per second could be returned. Thousands of rows per page has people waiting several minutes, and tying up our app server. A quick refactor to run 2 queries instead of hundreds to thousands and I was a hero for 10 min till everyone forgot how bad it was before I fixed it.
It’s the round trips that kill you.
Oracle drivers for .NET are fun. Have a user client application which uses quite a lot of data, but a few thousand rows are fetched some queries. It’s way too slow for any larger query, turns out for the batch query kind of work we do, the default FetchSize for Oracle is just a performance killer. Just throw it to 128 MB and it doesn’t really hurt at all.
Worst thing i’ve seen though, apart from the 150 line long dynamic sql stored in our database, was probably a page in our program that loaded about 150 rows from the database. Normally we do create a new connection for each query, but it’s fine since Oracle has a connection pool. Whatever millisecond is trumped by the round trip. But imagine a UI so badly written, it did 4 separate database queries for EACH row it loaded into the UI list. Useless things like fetching a new ID for this row in case it is changed, reading some data for the row i think, and more. Thing took a solid minute to load. There was so many bad patterns in that page that even during the PR for improving the speed it was just dealing with a mess because you couldn’t just rewrite the entire thing, so they had to make it work within the constraints. Horrible thing to work with.
So this is not as bad as some of the other stories I’ve seen, but I’ll bite.
It was an old .NET Framework MVC app. Some internal product management system or something. There was a need to do a PDF export in one of the use cases, so someone implemented it. It wasn’t a good implementation: one big controller, mixing UI and business logic, etc. However, it basically came down to a single private method in a specific controller for a page.
Now time passes and lo and behold, we need a PDF export in another page for a different use case. “No problem,” - same dev, probably - “I already solved this problem. I’ll just reuse the PDF generation logic.”
Now, any sane person would probably try to refactor the code responsible for PDF stuff into a separate service (class) and reuse it. A less sane, but somewhat, acceptable approach would have been to just copy paste the thing into another controller and call it a day.Ha! No no no no no no… Copy pasting is bad, code should be reused…
The end solution: REFLECTION. So the dev decided that the easiest way to make it work was to: 1) use reflection to inject one controller into another; 2) then use reflection again to get access and call that private method for PDF rendering into a stream.
Fortunately I didn’t have to fix that fragile mess. But I did my fair share of DevExpress corpse hacking and horrible angular “server side rendering” workarounds.
For anyone who knows and understands Android development, process death, and saved state…
The previous dev had no understanding of any of it, and had null checks with returns or bypassing important logic littered all over the app, everywhere.
I could only assume he didn’t understand how all these things were randomly null or why it was crashing all the time so he thought oh, i’ll just put a check in.
Well, you minimize that app for a little bit, reopen it, and every screen was fucked visually and unusable, or would outright crash. It was everywhere. This was before Google introduced things like view models which helped but even then for awhile weren’t a full solution to the problem.
It was many many months of just resolving these problems and rewriting it the correct way to not have these problems.
Oh I remember. There are tons of events and associated handlers. Even just switching to landscape view stops and restarts an android view I think. Friends at uni handled that problem by disallowing landscape view instead of handling it hahah
Friends at uni handled that problem by disallowing landscape view instead of handling it hahah
😭
Such a tragic and common ‘solution’ because it doesn’t actually solve it, it just delays it until someones minimizes the app for 30 minutes and re opens it, or one of the many many other ways that also trigger it.
I’ve had some apps that I do lock to portrait, but I would disable that flag on debug builds, since rotating the phone was the easiest way to test for some of those bugs. I didn’t worry about a good looking UI since it’d be locked in portrait, I just used it to test for bugs.
I was told about a bug in a specific tool. It was being used in production. Apparently we’ve gotten a lot of complaints about it over the years, and they would complain if the site was actively used it always failed.
I couldn’t find it in the development branch in source control.
I asked if this tool was purchased from a third party. My boss, who was not a developer, said no. And he was very sure of that. But he didn’t know where the code was.
I was the developer with the most seniority, and I was there for less than a year at this point.
I looked again. I finally found it… In an unapproved pull request from a few years prior.
The meat of this tool basically took information to make an order and create an order in the system.
Any time we needed to insert a record, it would find the highest Id in the table, increment 1, and insert the new record, id and all. It did this for every entity that needed to be inserted. Address, customer… Everything.
Worse, very little validation was done. Want to order something but it’s out of stock? No problem, this tool just pushed it right through.
Want to ship something using a shipping method that doesn’t exist? You got it.
Want to ship something to an address that doesn’t exist? Sounds like the warehouse’s problem.
Knowing about the level of knowledge here, you know that there were no transactions. All sorts of unused records were left in the database when there was an error. The users would get a generic error and try again several times, too.
The worst part was, we have an already existing function that would take order information and try to make an order. And it would give you actionable errors!
This reminds me of a time at work when we got sued. The company was allegedly using (or had copies) of some tool we couldn’t have anymore. Annoying, but fine. However, to check this, they scanned all of our computers for the name of that company. They told us all to delete our entire local Maven repository. Someone who worked there was on the commiter list for a couple of open source projects. I just manually deleted those files because I knew for a fact that our central Maven repository didn’t have some of the versions of our own code on it and I wasn’t confident we wouldn’t need them again. Turns out I was right and needed to grab one later on to upload. Because I manually deleted the files with the company’s name instead of just deleting everything, the scanner thing they were running didn’t detect offending files. (Not that a file listing someone’s email address as a commiter to an open source project should be offending, but still.)
@i_dont_want_to I think part of me just died a little reading this.
My favorite in the thread by far
I basically fix other people shitty voice for a living (replacing it with my own shitty code), the “best” one was by a guy, I suppose he was a self taught c programmer from how he wrote code, writing a complex python program. I saw:
- a function called randomNumberGenerator. It was a function which started a webserver. While looking for a python tutorial for something I found out why: he copy pasted the tutorial snippet but then didn’t bother renaming the function
- a program whose job was to listen to all other services and send them to another service via udp BUT it had a maximum buffer size so messages sometimes got truncated. I just directly put the listener in the target program and deleted it
- like another guy in this thread he didn’t use git. First day on the job they told me “yes, we need to check which machine has the latest code because he ssh into them and work there”. His version control was basically putting code in different machines
- lot of copied variables, because of c I suppose? Things like var = self.var
- camelCase python (ok this is just styling in the end)
- files with 10k lines of code
- half the services were in python 2, half in python 3. Don’t ask me why
- variables name in his original language (not English, not the client language)
- single letter variables, I fondly remember self.I (upper case i)
- I remember an
if a == a: (I left it there because lol) - he added a license check which used the ethernet mac address. Too bad ethernet was removed from the machine, and his code launched an exception which returned 00:00:00:00 as mac address, so all licenses were working on all machines
And many other things…
In another project I saw a backend running on the frontend, as in, this guy wrote the logic for a machine on the Javascript running the user interface of the screen
files with 10k lines of code
oh my sweet summer child.
I was once charged with maintaining an application with a median line count of 40k. The largest file was 87kLOC with 2nd place going to a 69kLOC (nice) file filled with interwoven C and inline assembly. My favorite was a 51kLOC file with a 32,621 line function.
Miracle I didn’t develop alcoholism during that job.
Pretty sure there was one over 100k file at one of my old workplaces. It kept growing and growing and was the most critical file in the business. Like if that file suddenly vanished, the business would be done or shut down for at least a year, maybe two kinda thing. Re-certifying the output of that file would probably take 6 months alone.
It had a partner file, also very important, but not as, which was much smaller around 20k-25k
I remember an if a == a: (I left it there because lol)
I once worked on a codebase in js where
a == a // true a == a // false a == a // true(Same variable compar to itself is true at first, then false, then true…)
And when I cried ‘per que’ the explanation made some evil, sadistic sense.
lot of copied variables, because of c I suppose? Things like var = self.var
If we’re very charitable, there’s a micro-optimization w/ Python (or at least, older Python) where assigning to a local variable like this inside a method is faster than the full
self.varlookup, so you’ll see it in Python’s library code while setting up some loops, etc. as a small speedup. “lots of copied variables”, though, is likely an anti-pattern if not in a heavily used piece of library code, imo.What’s really crazy is when people write modified Python language pre-processors where:
var = varis a necessary thing (to bring thevarinto the right context for the pre-processor to recognize it; yes, I’ve seen this…)
We had some super old code in our company monorepo that was written by someone who became the CTO, there was a comment forbidding people from writing private methods in the code base because “we aren’t babies”. It explained so much about the awful code and why everything was crazy.
That sounds like someone who didn’t understand the purpose of private
Yet he was still in charge of all the engineers who did. He had people actively working against their best interests lol. Disaster
Yeah, that just seems like a recipe for disaster.
Access modifiers are definitely something I despise about OOP languages, though I understand that OOP’s nature makes them necessary.
I don’t think OOP’s nature makes them necessary, so much so as it enables them and popular programming principles encourage them. I think they’re a good thing, especially if there’s a way around them in case you can’t get the public interface changed and it doesn’t work for you, especially for performance reasons, but that should be done with care.
Funny story, when modding Unity games using external modloaders you’re writing C# code that references the game’s assemblies. And with modding you often need to access something that the developers made private/protected/internal. Now, you can use reflection for that, but a different trick you can use is to publicize the game’s assemblies for referencing in your code, and add an attribute to your assembly that tells the runtime to just… Let you ignore the access checks. And then you can just access everything as public.
Private key for a third-party API hard-coded into the front-end web app
I think the worst software-gore I remember seeing was a web app that dumped all the data to the browser as a huge XML file and then had JavaScript translate the contents of the xml into views. That probably wouldn’t even sound that far off the reservation now if it was JSON, thanks to the sleepless efforts of the JavaScript industrial complex, but back then you’d just render pages and return them.
