ASP: Int, glue on 2FA fail, temp fail, index fail, reusability fail, copy paste coding, documentation

Post date: Jun 17, 2012 2:43:31 PM

Some interesting develoment fails I found from one ASP project.

• Int = Truncate, or maybe it isnt. Fixed this issue in ASP webapp. It seems that the developer who made the app, didn't know how int behaves with floating points.
  • int(x)
  • 1.0=1
  • 1.1=1
  • 1.9=1
  • So, it just truncates any decimals. But when we enter negative side, it still keeps "rounding down", which might be quite a surprise if you don't know what you're doing.
  • -1.0=-1
  • -1.1=-2
  • -1.9=-2
  • And in this case it caused very intersting application behavior when some larger number was first divided and then itegerpart was taken out and then rest of data was processed using modulus from the original number. If we use clock as example, it looked like this:
  • 59 miunutes = 0 hour 59 minutes
  • 60 miutes = 1 hour 00 minutes
  • 61 minutes = 1 hours 1 minutes
  • But on negative side results do look like this:
  • -59 minutes = -1 hour -59 minutes
  • -60 minutes = -1 hour -0 minutes
  • -61 minutes = -2 hours -1 minutes
  • It took quite a while to find that error and then to figure out how strangely the int function behaves with negative numbers. Only thing that was clear from the beginning was that issue only existed when values were negative and it didn't affect values that didn't have any minutes.
• Security fail: Spotted and fixed another super classic famework security error. When user was logging in, first user login screen was shown with username and password fields. When those did match stuff in database, framework login function was called. After that if user had 2FA flag enabled, then additional authentication screen was shown. So what's the major fail here? Login function was called before authentication process was completed. Now if user did click any other link / button than "authenticate" or gave known good url to service, they could totally bypass the 2FA. Only if they pressed authenticate button, with incorrect authentication data, then frameworks logout function was called. This is exactly what happens if 2FA is carelessly "glued on" to product that didn't originally support it. - Security has to be there by design, it can't be glued on afterwards!
• Temporary data/files: When app uses temporary files, those are added to apps own temp path. Unfortunately there is no mechanism to clean this path. It's up to administrator to delete older than files from that path regularly. Great design, isn't it? I personally like to ask from developers and then what? If something is added to somewhere. if it's not supposed to be there for ever, then there has to be a plan how to get rid of it. I think it's way too common to forget to finish the process and just do the parts that are required to make it "work". Same rule of course applies to any tables, files, etc. As you might guess also temp tables aren't properly purged ever.
• Indexing: Do you really need it? System works just fine with these three test records. Or customer can use really powerful server which allows to locate all data in main memory. Uh oh, yeah. - No! With this project I checked what are slowest queries it's making, and about 5% of queries (as daily volume) are using indexes. This isn't problem as long as there is very little data, or server is super fast. But yeah, it will be a problem sooner or later. When we add more and more data, system starts slow but sure process grinding to halt. Worst queries take about 10 minutes to complete. Tables with millions of rows and no useful indexes.
• Reusability: Some parts of app use libs, that are copied from other libs with slight modifications. So if any bug is found, it requires usually 5-20 places to be modified. In optimal case, there should be only one function for one thing.
• Copy paste coding: This is bad, it creates apps that are very hard to maintain. Confising bugs are also way common. It would be much better to make clear separate units, than one absolutely huge function with tons of different status variables, 200 lines row nested loops and if statements. Worst thing is that part of code that should be in independent function, is just copy pasted in code at several places after different if statments. Now if something breaks up, it's very very hard to know what and where should be updated or isn't updated etc. Also after final loop round, there has to be code to extract data possibly left in stacks or different status variables to final output stream. This is excellent sample of quickly emerging technical debt! It's clear that when code is needed once, naa, I don't make function about it. On second time you need the code, you'll simply copy paste it. After a while you have copied that same code, maybe even with slight modifications to several places. Now it's clear that some refactoring would be needed, but uh. Now I finally got this huge mess to work, so if I get refactoring I might get on slipperly slope again. It's just safer to leave it this way. All these problems are waiting the next unlucky guy who needs to debut / edit code.
• "Documentation is like sex. Having even bad documentation is much better than having none at all." - I have one Ubuntu workstation which seem to break it's grub after every distribution upgrade. When I enountered this problem again, I had totally forgotten the problem. But when I searched for the issue from Google, I was quite glad. I had posted issue description to Ubuntu Forums and just about 30 minutes later I had also replied to my first post describing the solution. Now I might have helped others, but most importantly I helped my self by documenting the problem and solution.

I have now extracted about 50% from my laptop's blog stuff queue. So more will be coming, when weather is bad again. Over and out.