Coding Horrors: You can gather all your technical debt into one easy framework!

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›1503 »

Steve French: Sep 8, 2003

Soricidus posted:

can't we just be impressed that this codebase is actually using a non-terrible method for storing passwords in the first place?

Not really, considering that the administrator has to know the user's password in order to change it.

# ? Jul 25, 2015 18:23

Adbot: ADBOT LOVES YOU

# ? Jun 1, 2024 06:40

Volguus: Mar 3, 2009

Soricidus posted:

can't we just be impressed that this codebase is actually using a non-terrible method for storing passwords in the first place?

Heh...until 3 weeks ago that codebase stored passwords in javascript. Literally

code:

if(username=="X" && password=="Y") {....}

Not to mention that the PasswordEncoder interface only has 2 methods: encode and matches. Pretty hard to miss what they are doing, as they state that clearly in the javadoc.

# ? Jul 25, 2015 18:27

Soricidus: Oct 21, 2010; freedom-hating statist shill

Volguus posted:

Heh...until 3 weeks ago that codebase stored passwords in javascript. Literally
code:
if(username=="X" && password=="Y") {....}

ok, I take it back. that's a good horror.

# ? Jul 25, 2015 18:33

Suspicious Dish: Sep 24, 2011; 2020 is the year of linux on the desktop, bro; Fun Shoe

yeah, you should be using a dictionary to store pair combos instead of an if...else chain

# ? Jul 25, 2015 19:21

Blinkz0rz: May 27, 2001; MY CONTEMPT FOR MY OWN EMPLOYEES IS ONLY MATCHED BY MY LOVE FOR TOM BRADY'S SWEATY MAGA BALLS

Volguus posted:

code:
@Autowired
private PasswordEncoder passwordEncoder;
was defined 5 lines above his method. The controller only had one other service injected. It was used in another method (not written by him, true) . There was absolutely no reason whatsoever to not use it.

Wait so does Spring let you do DI by declaring a property and decorate it with @Autowired? That's pretty awesome.

# ? Jul 25, 2015 19:25

Janitor Prime: Jan 22, 2004; PC LOAD LETTER

What da fuck does that mean; Fun Shoe

Yeah, anyone doing it differently is stuck in the stone ages. How do other languages do it?

# ? Jul 26, 2015 08:48

dwazegek: Feb 11, 2005; WE CAN USE THIS

Janitor Prime posted:

Yeah, anyone doing it differently is stuck in the stone ages. How do other languages do it?

Depends on the IOC container mostly. E.g. Castle Windsor for .NET can be configured in different ways. AFAIK the 2 default behaviors are to either inject through the constructor (resulting in an exception if a dependency can't be resolved) or injecting through public properties (in which case I think it'll only inject properties of types it knows). For property injection you can also mark a property to always be ignored, and I think you can also either mark the property or configure Castle so that it'll throw on missing dependencies.

However, you can extend/change Castle's behavior in so many ways, that I wouldn't be suprised if you could allow for injecting private fields as well.

For the most part we use constructor injection, and sometimes allow for injecting non-required stuff (mostly loggers) through properties, so you'd have something like this:

C# code:

private readonly ISomeRepository _someRepository;

public SomeService(ISomeRepository someRepository)
{
   _someRepository = someRepository;
   Logger = Logger.NullLogger;
}

public ILogger Logger { get; set; }

So you're guaranteed by Castle to get a valid ISomeRepository, and if an ILogger is configured we'll also get one of those, but if it isn't we'll just default to an ILogger that does nothing.

A slight advantage is also that, when you're writing unit tests, and thus creating an instance of SomeService yourself and mocking dependencies, if someone adds a dependency, it'll be caught compile time, rather than resulting in failed tests due to null references.

# ? Jul 26, 2015 09:30

Suspicious Dish: Sep 24, 2011; 2020 is the year of linux on the desktop, bro; Fun Shoe

quote:

Octet values are represented in this document as two decimal numbers in the form col/row. This means the value (col * 16) + row. For example, 02/01 means the value 33.

# ? Jul 27, 2015 05:16

karms: Jan 22, 2006; by Nyc_Tattoo; Yam Slacker

Every single aspect of that is :psyduck:

but pretending an octet is 16 bits is the worst.

# ? Jul 27, 2015 15:28

nielsm: Jun 1, 2009

KARMA! posted:

Every single aspect of that is but pretending an octet is 16 bits is the worst.

What? It's just a decimal notation of hexadecimal. From 00/00 to 15/15.

Still dumb.

# ? Jul 27, 2015 15:30

Ellie Crabcakes: Feb 1, 2008; Stop emailing my boyfriend Gay Crungus

KARMA! posted:

Every single aspect of that is but pretending an octet is 16 bits is the worst.

You might want to check your math here.

# ? Jul 27, 2015 20:31

New Yorp New Yorp: Jul 18, 2003; Only in Kenya.; Pillbug

Microsoft coding horror: .NET 4.6 has a massive bug.

# ? Jul 27, 2015 20:34

Factor Mystic: Mar 20, 2006; Baby's First Post-Apocalyptic Fiction

Ithaqua posted:

Microsoft coding horror: .NET 4.6 has a massive bug.

Bugs are gonna happen. I'm going to assume that they do have test and this slipped through some how. The real horror is that the bug was fixed a few days ago with a lot of explanation and then quietly reverted with no message.

# ? Jul 28, 2015 00:53

fritz: Jul 26, 2003

Surely this has been posted here before but it's new to me

http://journals.plos.org/plosone/article?id=10.1371/journal.pone.0038234

quote:

FreeSurfer is a popular software package to measure cortical thickness and volume of neuroanatomical structures. However, little if any is known about measurement reliability across various data processing conditions. Using a set of 30 anatomical T1-weighted 3T MRI scans, we investigated the effects of data processing variables such as FreeSurfer version (v4.3.1, v4.5.0, and v5.0.0), workstation (Macintosh and Hewlett-Packard), and Macintosh operating system version (OSX 10.5 and OSX 10.6). Significant differences were revealed between FreeSurfer version v5.0.0 and the two earlier versions. These differences were on average 8.8�6.6% (range 1.3�64.0%) (volume) and 2.8�1.3% (1.1�7.7%) (cortical thickness). About a factor two smaller differences were detected between Macintosh and Hewlett-Packard workstations and between OSX 10.5 and OSX 10.6. The observed differences are similar in magnitude as effect sizes reported in accuracy evaluations and neurodegenerative studies.

# ? Jul 28, 2015 01:13

Mr Shiny Pants: Nov 12, 2012

fritz posted:

Surely this has been posted here before but it's new to me

http://journals.plos.org/plosone/article?id=10.1371/journal.pone.0038234

So depending on the software used you could be totally healthy or suffering from a degenerative brain disease?

Wow......

# ? Jul 28, 2015 07:05

Hammerite: Mar 9, 2007; And you don't remember what I said here, either, but it was pompous and stupid.; Jade Ear Joe

Factor Mystic posted:

The real horror is that the bug was fixed a few days ago with a lot of explanation and then quietly reverted with no message.

Do you have more detail on this? Where's this discussed?

# ? Jul 28, 2015 09:15

karms: Jan 22, 2006; by Nyc_Tattoo; Yam Slacker

nielsm posted:

What? It's just a decimal notation of hexadecimal. From 00/00 to 15/15.

Still dumb.

John Big Booty posted:

You might want to check your math here.

Octet != octal, I will repent for my sins.

# ? Jul 28, 2015 10:06

Hiowf: Jun 28, 2013; We don't do .DOC in my cave.

Ithaqua posted:

Microsoft coding horror: .NET 4.6 has a massive bug.

Optimizer bugs happen. That article though, is quite something. I can see you not deploying it if you're hit by the bug, but why the hell dis-recommend everyone else from updating? The optimizer is gonna have more bugs even if this one gets fixed. Sounds like alarmist whining to me.

And then this paragraph, oh my:

quote:

To address an obvious question: is this a security issue? ... For example, if your code makes an assumption with a null param like if (user == null) { user = SystemUser; }, then a null passed in will certainly be a problem, giving other users that access sporadically.

# ? Jul 28, 2015 11:20

Gul Banana: Nov 28, 2003

what? that's absolutely a serious enough bug that nobody should use .net 4.6 in production until it's fixed. "your methods are called with wrong parameters unpredictably" could break absolutely any application.

# ? Jul 28, 2015 11:39

sarehu: Apr 20, 2007; (call/cc call/cc)

Pfft, sounds like some of you aren't practicing defensive coding.

# ? Jul 28, 2015 12:47

Subjunctive: Sep 12, 2006; ✨sparkle and shine✨

Gul Banana posted:

what? that's absolutely a serious enough bug that nobody should use .net 4.6 in production until it's fixed. "your methods are called with wrong parameters unpredictably" could break absolutely any application.

Yeah, especially since you can't reliably verify it in test: small things can cause JITs to compile differently.

# ? Jul 28, 2015 13:01

Xarn: Jun 26, 2015

Gul Banana posted:

what? that's absolutely a serious enough bug that nobody should use .net 4.6 in production until it's fixed. "your methods are called with wrong parameters unpredictably" could break absolutely any application.

Yes, but you probably should set up a testing environment with .NET 4.6 anyway.

Also if this holds for you

quote:

That means you�ll only see this in RELEASE, which for most people is only production.

you deserve the bugs.

# ? Jul 28, 2015 13:16

Hiowf: Jun 28, 2013; We don't do .DOC in my cave.

Gul Banana posted:

what? that's absolutely a serious enough bug that nobody should use .net 4.6 in production until it's fixed. "your methods are called with wrong parameters unpredictably" could break absolutely any application.

So could any other optimizer bug. MS presumably tested NET 4.6's JIT extensively and didn't see this bug. Why do you think your code is a special snowflake that's more likely to be hit by this bug than any random other one?

# ? Jul 28, 2015 13:22

Subjunctive: Sep 12, 2006; ✨sparkle and shine✨

Skuto posted:

So could any other optimizer bug. MS presumably tested NET 4.6's JIT extensively and didn't see this bug. Why do you think your code is a special snowflake that's more likely to be hit by this bug than any random other one?

Because this one is really bad and affects a very common pattern. Code doesn't need to be a special snowflake to hit it by any means.

But also, your logic is just faulty: if you know about a bad bug, avoiding it makes sense. That there might also be other bugs in there somewhere (somewhat counter to your assertion about comprehensive testing) is less reason to upgrade, not more.

# ? Jul 28, 2015 13:27

Hiowf: Jun 28, 2013; We don't do .DOC in my cave.

Subjunctive posted:

Because this one is really bad and affects a very common pattern. Code doesn't need to be a special snowflake to hit it by any means.

So common MS apparently has no tests for it and it apparently also didn't affect any of their own code.

FWIW: https://github.com/dotnet-bot/coreclr/commit/838b04319ad2c1ce5e2130e09651c7261b40c42d

quote:

But also, your logic is just faulty: if you know about a bad bug, avoiding it makes sense.

No, testing/checking to see if you're hit makes sense. If I tell you that all JIT's have bad bugs, are you going to avoid them all? That reasoning really makes no sense. Software (including compilers/JIT) has bugs and we've learned to live with it by testing stuff.

quote:

That there might also be other bugs in there somewhere (somewhat counter to your assertion about comprehensive testing) is less reason to upgrade, not more.

The old version might have bugs that are fixed in the new one, too. Again, reasoning makes no sense. I do agree that upgrading in general risks exposing more bugs because you've presumably already worked around the existing ones.

But then you should ask yourself why you're upgrading to begin with and this bug has no bearing on that.

# ? Jul 28, 2015 13:47

Subjunctive: Sep 12, 2006; ✨sparkle and shine✨

Skuto posted:

So common MS apparently has no tests for it and it apparently also didn't affect any of their own code.

FWIW: https://github.com/dotnet-bot/coreclr/commit/838b04319ad2c1ce5e2130e09651c7261b40c42d

Is the unit test for that fix in another commit? I've read that PR, but I'm not familiar enough with the IR to really follow all of it.

JITs are very hard to test because they're so context-dependent and by their nature dynamic. Way harder than AOT compilers in my personal experience. There are definitely going to be bugs, of different severities, which is why upgrading carries risk (probability x severity). This bug manifests in a particularly dangerous way, in that it changes behaviour rather than simply crashing or skipping an optimization, so in my opinion the severity is high. It has an 8-line repro. The probability is 1. If you asked someone on the .NET team off the record, I'm certain that they would tell you not to update right now.

The best part is that just having 4.6 installed causes the bug to manifest even when running a 4.5 application. That's some horror right there.

E: seriously, does this look like some obscure pattern? https://github.com/dotnet/coreclr/issues/1296#issuecomment-125568026

# ? Jul 28, 2015 14:00

Hiowf: Jun 28, 2013; We don't do .DOC in my cave.

Subjunctive posted:

The probability is 1.

If it's 1, then why wasn't it caught before?

quote:

E: seriously, does this look like some obscure pattern? https://github.com/dotnet/coreclr/issues/1296#issuecomment-125568026

Depends on how closely you have to match the repro. If you have to match the variable types, call tree depth etc, the odds are something entirely different from "this general pattern".

# ? Jul 28, 2015 14:12

Subjunctive: Sep 12, 2006; ✨sparkle and shine✨

Skuto posted:

If it's 1, then why wasn't it caught before?

The probability of this bug is 1 because it is known to exist.

I guarantee that there are SDETs in Redmond asking how it wasn't caught before too. I've shipped bugs in a widely-used JIT that made me ask myself the same question, and the answer is usually "I guess we didn't think of that combination" or "the JIT didn't get into that sequence of modes in the test framework". Have you never shipped a bad bug that you were surprised you didn't hit in test?

You can look at the other repros in that bug report to get a sense of how varied the cases can be.

And yeah, it brings this bug to 4.5.2 as well.

# ? Jul 28, 2015 14:22

Subjunctive: Sep 12, 2006; ✨sparkle and shine✨

Fundamentally, a position of "their testing would have caught all severe bugs, therefore this bug that their testing didn't catch must not be severe" is charitable towards software developers to the point of na�vet�.

# ? Jul 28, 2015 14:34

LOOK I AM A TURTLE: May 22, 2003; "I'm actually a tortoise."; Grimey Drawer

Skuto, is your argument basically that people shouldn't hold off on upgrading to .NET 4.6 because: 1) They're probably not affected by the bug to begin with because the preconditions are quite specific, and because Microsoft have probably tested it a lot without noticing it; and 2) Even if they're hit by the bug they should still upgrade because they should be able to catch any issues caused by the bug before deploying? (1) is based on unfounded assumptions about precisely what causes the bug to happen and Microsoft's own development process, and if you believe in (2) then you must have a lot more faith than me in the software developers of the world.

This is leaving aside the fact that installing 4.6 on a machine could cause the bug to start happening in programs targeting earlier .NET 4.x and I think even 2.0 as long as they're 64-bit. This isn't just an issue for developers.

The big problem with this particular bug is that it could cause issues so subtle you may not be able to tell they're happening until much later, because they might just be causing stuff like numbers or dates to be wrong but without making anything crash or giving obvious error messages. To me the worst bugs are always the ones you don't know are there until it's too late. If the incorrect values get stored in a database or something then people should notice eventually, but it could also be that the incorrect values are only used in intermediate steps of a calculation and cause the program to go down the wrong code path. I'm thinking of stuff like this:

C# code:

// I have no idea if this code would actually trigger the bug, but humor me for a second.

void Step1(DateTime d, int someOtherValue, decimal someThirdValue)
{
	d = d.AddDays(1);
	Step2(d, someOtherValue, someThirdValue);
}

void Step2(DateTime d, int someOtherValue, decimal someThirdValue)
{
	// Assuming the bug is in play, it's effectively random whether you get into the if or the else, since
	// d will contain garbage from the other variables on the stack or whatever else was in memory at the time.
	// The value of d will be some random date between 0001-01-01 and 9999-12-31.
	// Both outcomes will seem correct at a glance, but obviously only one of them is actually correct.
	// Good luck figuring out which customers got an incorrect value here.
	if (d > DateTime.Today) { .. }
	else { ... }
}

And yes, you should of course have automatic tests that detect all this stuff, but you probably don't. Or maybe you actually do but they still don't catch it because your test setup somehow doesn't use RyuJIT, or it runs an older version that isn't affected, or it runs with optimizations off (it shouldn't, but it might), or some other minor environment difference. The fact that the bug can affect library code running inside your process also contributes to making it harder to track down when it hits you. Your unit tests may also fail to help you because they may go down a different code path in a third-party library or sidestep it entirely because of mocks, meaning it only happens in integration tests and in production. Are you sure you have assertions for all the different values in all those awesome integration tests you definitely have and run all the time? Are you sure your manual testers will pick up on the fact that an obscure date value sometimes seems to receive a random value 1/100 times because the code that was hit by the bug is only called in rare cases?

This may all seem a little alarmist, but this bug sounds like it has the potential to cause endless hours of developer pain because of how arbitrary it is. I would definitely not upgrade if there was a chance I might be hit by this, and I don't know how you can ever definitely say that there's no chance.

By the way, here's another recent RyuJIT bug: https://github.com/dotnet/coreclr/issues/1299. The outcome is similar to the one we're discussing, but the preconditions are much more arcane.

LOOK I AM A TURTLE fucked around with this message at 14:40 on Jul 28, 2015

# ? Jul 28, 2015 14:37

Subjunctive: Sep 12, 2006; ✨sparkle and shine✨

If I'm reading the PR correctly, tail calls with int? parameters will exhibit this behavior. (Maybe not if they're the final parameter? Maybe all optional value types?)

# ? Jul 28, 2015 14:44

Hiowf: Jun 28, 2013; We don't do .DOC in my cave.

quote:

(1) is based on unfounded assumptions about precisely what causes the bug to happen and Microsoft's own development process

Subjunctive posted:

Fundamentally, a position of "their testing would have caught all severe bugs, therefore this bug that their testing didn't catch must not be severe" is charitable towards software developers to the point of na�vet�.

My position is that the odds that you're actually affected by this bug are such that it's not a reason to hold off on upgrading. Though retroactively affecting 4.5 may cause rolling back to be somewhat of a PITA and that may be a factor.

I'm not saying the bug isn't severe. I'm saying the overall odds of hitting it can't be all that high (which is again something different from "definitely say that there's no chance") and aren't necessarily much higher than random other bugs.

quote:

The probability of this bug is 1 because it is known to exist.

Who cares? What matters is if it affects your code. The probability there is far less than 1. What the gently caress is "upgrading carries risk (probability x severity)" even supposed to mean if "probability" goes to 1 if any bug exists?

quote:

I guarantee that there are SDETs in Redmond asking how it wasn't caught before too.

This just reinforces my point.

quote:

because your test setup somehow doesn't use RyuJIT, or it runs an older version that isn't affected, or it runs with optimizations off (it shouldn't, but it might)

These are really bad and open you open to a whole class of similar bugs. I agree it's not possible to catch these 100% of the time, and JITs do make it worse because you can't get coverage of the "actual code that runs". But the above is asking for trouble.

Hiowf fucked around with this message at 14:59 on Jul 28, 2015

# ? Jul 28, 2015 14:55

Subjunctive: Sep 12, 2006; ✨sparkle and shine✨

Skuto posted:

I'm not saying the bug isn't severe. I'm saying the overall odds of hitting it can't be all that high

Why is that the case? How are you assessing the odds, and under what assumptions?

The pattern looks pretty simple, but I am not an expert .NET developer. Is it uncommon for people to use nullable int? Are calls in tail position rare? Does the JIT not often choose to perform this optimization for some reason?

# ? Jul 28, 2015 15:03

Hiowf: Jun 28, 2013; We don't do .DOC in my cave.

Subjunctive posted:

Why is that the case? How are you assessing the odds, and under what assumptions?

My assumption is that by now MS must have pretty extensive regression tests for their .NET stuff, including JIT specific stuff, and (AFAIK) they dogfeed it on their own, substantial codebase.

Clearly also not everyone started yelling that .NET 4.6 is broken.

Realistically I wouldn't expect any major compiler/JIT (GCC/LLVM/Java/Dalvik/ART/JSCore/SpiderMonkey/V8/etc) to ship with bugs that are likely to break a given individual user (which, note, is an entirely different probability from "hitting any user at all"). I'm not sure I'm explaining this clearly, so let's phrase it like this: it's unlikely that when GCC 5.3 ships, it will break *your* C++ application, even though it's guaranteed to be full of optimizer bugs.

You can find really insane GCC bugs (especially on embedded!) where the compiler gives incorrect results on trivially simple code. You're just not all that likely to actually hit them.

The probability game is different on the *vendor* side, though. Even if this bug hits 0.1% of the codebases, that's one in every thousand .NET customers that is hosed, which makes it a disaster.

Hiowf fucked around with this message at 15:13 on Jul 28, 2015

# ? Jul 28, 2015 15:10

Subjunctive: Sep 12, 2006; ✨sparkle and shine✨

Skuto posted:

Realistically I wouldn't expect any major compiler/JIT (GCC/LLVM/Java/Dalvik/ART/JSCore/SpiderMonkey/V8/etc) to ship with bugs that are likely to break a given individual user

I have personally been responsible for bugs in one of those listed JITs that broke major web sites for many people, and which survived extensive explicit and dogfood tests (with far more people than MSFT has internally), so I'm willing to believe that it can happen elsewhere as well. Maybe only to preserve my own ego!

(And I once broke FarmVille for about 1/2 of their Firefox users, though not a JIT bug.)

# ? Jul 28, 2015 15:21

Blotto Skorzany: Nov 7, 2008; He's a PSoC, loose and runnin'
came the whisper from each lip
And he's here to do some business with
the bad ADC on his chip
bad ADC on his chiiiiip

Subjunctive posted:

(And I once broke FarmVille for about 1/2 of their Firefox users, though not a JIT bug.)

Doing God's work

# ? Jul 28, 2015 15:26

Factor Mystic: Mar 20, 2006; Baby's First Post-Apocalyptic Fiction

Subjunctive posted:

(And I once broke FarmVille for about 1/2 of their Firefox users, though not a JIT bug.)

Even though you only achieved 50% eradication, we'll still count this in your favor.

# ? Jul 28, 2015 15:31

No Safe Word: Feb 26, 2005

Skuto posted:

Clearly also not everyone started yelling that .NET 4.6 is broken.

It's been out less than a week and the bug only manifests itself in production, so really only teams like StackOverflow would have publicly unearthed such a thing as they are pretty aggressive with migrating to new stuff.

# ? Jul 28, 2015 15:43

raminasi: Jan 25, 2005; a last drink with no ice

Plus, this is a total JIT rewrite, primarily for faster throughput, so "It probably fixes more bugs than it introduces" is less likely to be true than for typical maintenance upgrades.

# ? Jul 28, 2015 15:58

Adbot: ADBOT LOVES YOU

# ? Jun 1, 2024 06:40

LOOK I AM A TURTLE: May 22, 2003; "I'm actually a tortoise."; Grimey Drawer

Skuto posted:

These are really bad and open you open to a whole class of similar bugs. I agree it's not possible to catch these 100% of the time, and JITs do make it worse because you can't get coverage of the "actual code that runs". But the above is asking for trouble.

Oh, I totally agree, but the part you quoted there was part of a long chain of things that can go wrong in the development/testing process, and everyone is vulnerable at some point. My point was that many developers are at the very beginning of that chain, meaning that they either have no tests that cover the affected code, or they have imperfect routines for executing the tests that cause the tests to be worthless. This probably goes for most companies, and these are the same people who shouldn't upgrade until the issue is fixed because they will have no idea if their program is still working correctly after the upgrade. You always run a risk when upgrading something, but this particular bug is subtle and insidious. I regard it as a complete show-stopper.

Subjunctive posted:

Why is that the case? How are you assessing the odds, and under what assumptions?

The pattern looks pretty simple, but I am not an expert .NET developer. Is it uncommon for people to use nullable int? Are calls in tail position rare? Does the JIT not often choose to perform this optimization for some reason?

Nullable ints and other nullable value types are used all the time, and calls in tail position are as common as in any language, i.e. very common. There's nothing very unusual about the code. The main thing here is that RyuJIT is quite new and hasn't been tested very much out in the wild yet. RCs and stuff have been around for a while, but it was only officially released about a week ago, so it's safe to say that 99.99% of .NET programs have never been run with RyuJIT before. It also only runs in 64-bit.

# ? Jul 28, 2015 16:02

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›1503 »