Wednesday, December 28, 2016

The new design of viva64.com. The story behind it, told by the PVS-Studio developers

Viva64.com - the main PVS-Studio developers site, turned 10 this year! The domain was registered on Nov. 9, 2006, and the latest major design update was done in December 2010. It's time for a change!
Picture 3

Project participants

At the moment, 4 people are involved in the development of viva64.com:

  • Constantine Potapov - web-developer;
  • Sergey Harleev - web-designer;
  • Ilya Teterin - Linux engineer;
  • Evgeniy Ryzhkov - project manager.
Other people contribute to the content of the website, but are not related to the infrastructure of the site, its design, or functionality.

Our product

We develop our own software product - PVS-Studio, a code analysis tool for developers. It helps to find bugs in programs by doing analysis of the source code. The client companies save money by detecting the bugs before beta-testing of the product, and of course, before the release. This is the product we sell.
To spread the word about our analyzer, we write articles about the inspections we have done on open source projects, where we find bugs with our analyzer.

The concept of the site

For a long time we couldn't figure out, what category our site fell into.
On the one hand, we are a company, and therefore, this is a corporate site. But we're neither Disney, nor Microsoft, so nobody really cares about our company.
On the other hand, we have a product for programmers, but in the era of the AppStore, Play Market, and other stores, the site of a separate tool isn't that interesting to end-users. A lot of successful products live happily without a website.
Finally, on the other hand, we have a content site. After all, the main aim of people who visit our web site, is to read articles about the checks done on projects using our analyzer, to see which bugs we managed to find, and learn how to write bug-free code. It turns out that it is something of a learning resource.
Having combined all of these options, we have come to the following conclusion. We have a content site; but every page should make it clear what PVS-Studio is.

What idea do we want to convey to visitors to our website?

We have come to the conclusion that we have a content project. What idea do we want to convey to those who happen to visit our website? First, we should understand how visitors land on our site, and which pages are the most popular. Let's turn to Google Analytics. Here is the overview of traffic for 10 months of the year 2016 (Acquisition Overview for Jan-Oct 2016):
Picture 1
If have a look at the most popular pages, the main page will only be in 15th place. Thus, we can conclude that people almost never go to the main page, but go to some content (articles) posted on the blog, either from a search, or from social networks, or via a link from a different resource.
Therefore, here is the first important conclusion: our visitors get right to the "middle" of the site, bypassing the main page, which means that the idea to "lead" the person from a main page to purchasing the product won't work.
Ok, we sorted that out. But how can we "direct" the person to the product purchase? After all, all the articles are written with this goal in mind.
We see that our sales cycle is about 3-6 months. This is usually the time in which people read 3-4 articles about PVS-Studio, and test the tool on their project. Further on, if we are lucky enough, and the person likes the product, then they purchase it.
Thus, our task is to convey two ideas to the visitor:
  • They have to read 3-4 more articles, rather than leaving after reading just the one that brought them to our web-site.
  • We should persuade the visitor that the product we offer is not a fraud, but a really great thing.
One more thing - we should show that there are more articles on the site, so that the person reads 3-4 more. That's why we have a 'Recent articles' block, which should always be visible.
I am convinced that we do a great job - we find bugs in open source projects, and this makes a person believe in the tool. The aim of the website is to let the person know about the checks. That's why we have the block 'the number of projects we checked, and the number of bugs'. In this section, the user will see the full list of projects that we have ever inspected, and all the errors we found.
On any page of the site, the visitor should be aware that there is such a software product as PVS-Studio, and that it can be gotten on this website. So, the product is mentioned in various parts of the site.

Elements we designed to reach our goal

We have decided that the purpose of the site is to inform visitors that the site is full of interesting articles about the project inspections that were done with the help of PVS-Studio, a tool that is objectively worth trying out.
How exactly do we convey this thought? Let's look at the main points.

The header of the site

The header states the company business: "We develop a static code analyzer". From my experience, quite often, even if a lot of money is invested in site development, the site will not necessarily reflect the company business. It can still work when these are corporations like Siemens or Samsung, with multiple business spheres. But it tends to happen, that small companies of 10-20 focus so much on wanting to be creative, trying to make the site really bright and shiny, but failing to depict the company business direction.
Our contacts can be seen as an e-mail, and a feedback form. It shouldn't be any trouble to write to us. That's why we offer two ways to contact us, and we want them to always be available for site visitors.

The left menu of the site

The left menu of the site is quite minimalistic. It has only the most important points. The most things important on our site? In our opinion:
  • Our blog, so that readers can always see new articles about checked projects.
  • A list of checked projects, and the list of bugs found with specific numbers, so that people can see that the work is on-going.
  • Programming tips.
  • And of course, the product itself.

The site footer

The footer is quite simple:
  • One more reminder of PVS-Studio.
  • A list of the main pages and sections of the site.
  • Contacts.
  • Twitter, RSS.
  • Site search.

The right block of the site (side-bar)

In the right block we have the following traditional elements:
  • Recent articles.
  • Twitter timeline.
  • A list of links "It's interesting!".
A couple of non-traditional elements:
  • A block "Bugs found" - with the number of checked projects and collected errors.
  • A block "examples of errors" - real code fragments from different projects.
The purpose of the right block is that the visitor, reading the articles, would realize that there are many other interesting materials.

For the product fans

As with all products which have a history, PVS-Studio has fans (and haters, but I have nothing to say about them here). For this category of people we created a section with a free souvenir section.
Picture 4
There you can find wallpapers for your desktop, and pictures to print on T-shirts and cups. It's quite amusing to see these things in someone's office.

Interview with the developers of the site

This article about the development of a new version of the site would be incomplete without an interview with the project participants. I asked them to answer a couple of questions:

Sergey Harleev - web-designer;

ER: Sergey, tell me please, was it easy to come up with the concept of the site? We are neither a store, nor a landing, and not just a blog. How did you understand the main direction of our activity?
SH: Yes, the project turned out to be bigger and more complex than it was at first glance. The main difficulty was to not make the site too promotional, but still to keep in mind that the main aim is to sell the tool. Our target audience is rather special, very skeptical, and cannot tolerate us begging them to buy. That's why we created inside the company a so called 'focus group', and discussed with them some elements and blocks of the site.
Firstly, we put the post block at the forefront of viva64.com, as it is first and foremost a learning and informational resource. This is why we focused on the readability of the articles and convenience for users. The analyzer itself moved to the "back" of the site, to one of the subsections.
We don't use marketing elements such as colorful banners, large and beautiful illustrations, or sparkling headlines, because developers aren't really impressed by these. These "cool features" can tend only to have the opposite effect.
However, we don't forget about the product, gently mentioning it for example in the block "Checked projects". There we tell how PVS-Studio has found serious bugs in the code of large and famous tools, and give examples. On top of this we demonstrate quite the impressive statistics of the checked open source projects and found errors.
ER: What way did you choose to encourage users to return to the site?
SH: In general, there is quite a number of them in the world, but very few will work for our target audience. For example, we do not use e-mail subscription, because our users aren't really big fans of such mail outs, to say the least, and social nets. Our choice were those channels where we already have subscribers - the Twitter page of Andrey Karpov, and RSS.
ER: How much was the creative process subjected to censorship?
SH: First, I tried experimenting with the graphics; for example, with the concept of an imaginary world, like in 'Adventure Time', for our unicorn mascot. But the team rejected this idea, because the focus shifted to our main character. After several months of living with the product, you start understanding it better; its principles, philosophy, and closer to the end of the work I was completely on the same page as the team.
ER: How many pages did you have to render? Is this number typical for a website design?
SH: More than 60 layouts; we designed a tablet and a mobile version of every page. Such an approach is typical of online services, but is not often used for corporate and informational sites.
ER: How was this project different from other projects?
SH: It was the first order of a brand book for a website with exact guidelines and rules concerning the fonts, colors, intervals, elements and blocks. It was like a revelation, it turned out to be a handy tool for developers and for myself, the document significantly speeded up and the simplified process.

Constantine Potapov - web-developer;

ER: Constantine, the first question is about deadlines. What were your initial estimations of the target date, and how did it turn out? What do you think - why is there such a huge difference?
CP: In the beginning, I thought this website to be just a simple site, with articles. So, I set the deadline of creating a website with a more modern design, of three weeks. I cannot comment on the outcome, as some say "once you release the site, your work finally begins".
ER: Well, I am quite aware about the time it took us to make this project. We started working on the design in the middle of May 2016, and finished (so to say) in the middle of November, so roughly half a year later. The first work on the site code started in July. One may say that it has nothing to do with the design, but it's important to understand that this site is more just 'a site with some articles'. We started working directly on the new design, and the code, only in mid-September, while the release happened in the middle of December. I.e., three months to explore the code and three more to do the main work.
Well, almost three weeks... (Sarcasm)
CP: I could never imagine that such a small site could have so much inside. It was good and bad at the same time. The good thing was a cool mechanism to publish articles on the web-site. The articles are written in Word, then with the help of a custom plugin, they are converted into .htmls, which can be published on the main site and on other resources. In addition, there were integrations with the PVS-Studio application, and a bunch of marketing analytics.
The bad side of it was the 'production-code'. I like to say to my programmers, "It is not production, there is no place for kludges". But after comparing pros and cons, spending two weeks of trying to put all our eggs in one basket, and also a considerable amount of time to polish the legacy-code, I came to the conclusion about the evolutionary approach. Apparently, the method just 'rewrite' everything doesn't work for such projects anymore.
Another difficulty was the new design. I am used to working structurally; using grids, and thinking about the point of the site, not about the emotional image of the site (without drawing well). I see a lot of flaws, but, "An ideal product costs endless money, endless time, and just doesn't exist". So we had to make compromises.
ER: By the way, what is the size of the site now? The size of the content and the source code?
CP: At the moment of writing this text, we have this:
  • The amount of articles (blog, news, terminology, lessons and so on.) - 836;
  • The documentation articles - 512 (a normal thing for programmers);
In total, approx. 1350 pages in English and Russian. Besides that, there are 255 pages (where do these programming pages some from?) with examples of errors.
In short, there are about 1,500 articles in Russian and English.
Speaking about the code, there are 14 megabytes of code, not including libraries, but this is a slightly artificial figure. For fans of static analysis, here is a more artificial number in the picture.
Picture 2
ER: Tell me about the 'guts' of the site? Which technologies were used?
CP: This site has a long history, it was made more than 10 years ago and developed by different programmers. So, the code leaves much to be desired - to say at least. Being a responsible person, I decided to use the minimum of non-standard and rare technologies, so that it won't be difficult to find a new developer, in case I get hit by a plane. The site is written on standard Python 2.7, and Django 1.9. If I were to start a project from scratch, I would use more modern technologies, and perhaps even implement them during refactoring. But this honor was given to another person 10 years ago.
ER: Is there anything you can say about the peculiarities of the layout?
CP: I tried to use mostly the 'universal' layout, and cut off the ready-made layout in the BEM-style (when each element is developed separately). This approach is good for unmanaged projects with hundreds of developers and hot changes. But I support the idea that CSS is a cascading style table, and I tried working in this paradigm. Also, I tried to use the abilities of the framework (imposition framework) to develop various elements without getting into CSS. Too bad, it's always a compromise. And I am not sure that my decision is the best. For example, I would like to remake the layout using SASS, and make it more flexible. When I started working, a lot of compositional problems weren't solved, which emerged during the development. Besides that, I haven't realized all the designing ideas of Sergey, but I tried to do the best I could.
A website is software, but as the HP director once said (as far as I remember, it was him), "If you want to make great software, start making your own hardware". There are numerous devices and solution, and I tried to find a compromise.
ER: How was the development process organized? Branches, various servers, and things like that? More details, please!
CP: Here is the infrastructure we have now:
  • A production server - that's clear.
  • A stage-server (for the development) in the hot swap mode of a new release and the old site to test the changes.
  • "A hibernate server" (as we called it) for the reserve deployment of the full version of the production server.
We implemented automation and synchronization between different servers on the programming level. Looks like it worked. You can update the version of the site 'by air', directly from the administration system. Besides that there is a separate server on Jenkins that automatically converts the .docx-articles into .html, and uploads them to the site.
ER: Did you have to remake a lot? If yes, then why?
CP: I have a lot of respect for efficiency. I really cannot understand people who stare at the monitor for several hours waiting for the work day to be over, or for the boss to nag them. This site set an architectural challenge that I faced only once in my life, and I cannot say that I fully solved it. I finished at a Physics University, so I have the ideology of a scientist-engineer-researcher. I created about 5 versions of the engine, with various technologies and architectures, but realized it would be too risky to change everything at once. I remember when I was developing an IP-telephony for Tajikistan. At that time, they could get the electricity by voucher, not speaking about the internet. I faced a problem that they needed G729 codec (not open-source 723). The license for it was only in Intel, and a couple more. But the point is that wanting to save 5% of the traffic, I almost had a nervous breakdown.
There was just nobody around to stop me in this perfectionism.
ER: Now, post factum, how much time could it take to make the new design for the website? What about the new site engine?
CP: Three weeks could be enough. On the condition that I don't have to integrate with the current infrastructure, I will do the design using ready-made components, and editing through the administrative panel. What? It's just a site with articles! I hope you realized that this is sarcasm.
ER: What are the further steps?
CP: As a minimum, we should do the following:
  • Asynchronously loading statics.
  • Practice some magic with the server settings.
  • Improve the site display on different devices.
  • Do some 'fire drills', and see how the reserve systems work.
  • Document the current system (I have some troubles with that).
  • If I create the docs, I would like to take a vacation, or die with the feeling that "the work is done".

Ilya Teterin - Linux engineer;

ER: Ilya, can you tell the story of how you became a part of our team?
IT: The main site was disabled as if it were sending spam, and blacklisted for 'spreading viruses'. We needed to quickly sort out what was going on. And that's something that I like doing. In my youth I was in a hacker team, so I still had some skills. However, it turned out that there were no malicious wreckers, it was just badly set up protection against the simplest viruses, stealing passwords from FTP. The office was well-secured, but the home PCs, where there is also some official information stored, was not that safe. But it would be unfair to rebuke a common software firm, when even the US presidential candidates stepped on the same rakes. I think that the situation got much better - we got rid of dangerous protocols, cut the rights, talked with the employees, etc.
ER: Ok, now please tell about the hardware that the site works on.
IT: This site does not require any extraordinary computing power, just stable working conditions. We use a virtual dedicated server, which provides a good reserve. In case something goes wrong (extreme attendance growth, DDoS-attacks) - we are to migrate to a more powerful server in the shortest time or to organize a cluster, and connect external protection services to ensure the smooth operation of the site.
ER: Which tasks concerning the server organization were solved, and what should be done in the future?
IT: We are gradually decreasing the vulnerability of the system from external threats, and the speed of recovery in case of a crash. We moved to more secure exchange protocols, put things in order with the user roles and access rights. Now the backup is done to the cloud service with triple reservation.
Now, in the final stages is the organization of repositories, for the users of the Linux version of PVS-Studio, making our product easy to use and update. We would also like to embed the regulations for information security, and work with the backups. A triple backup is great, but it also needs to be monitored.
Besides that, we should create a script for automatic deployment of a complete system from an image on any server (so that it is done in a few minutes, not in dozens of minutes).
ER: Ok, I already want it all!

Conclusion


We have done a great job with the website, but we cannot say that the work is over. This why we will be very glad to get your comments, and suggestions about improvements to the site, and any feedback about the site www.viva64.com.

Friday, December 23, 2016

Stories about Christmas and New Year Bugs

Do you believe in magic? Of course not - it's just against logic! Programmers are serious-minded and well-educated people of a realistic outlook. Well, you didn't favor fairy tales as a child either, did you? OK, I'm not going to answer for you. Just please make yourself a cup of tea, peel a tangerine, look at the snowflakes falling outside the window, and only then go on to read this Story.
What you are about to read is a story about an Evil Bug and its multiple attempts to spoil Christmas Eve and New Year's Eve. It did manage to fulfill its sinister plans a number of times, but, fortunately, in every true fairy tale, evil is always opposed by good.
Picture 3

Christmas-tree virus

On December 17, 1987, a student at the Clausthal University of Technology, former West Germany, a beginner programmer at the time, had a bright idea of an ingenious Christmas greeting for his friends. He sent them a Christmas tree! Of course, he hadn't cut it down in a forest, nor had he even bought it in a store. He was a programmer, remember? He just wrote a program in the scripting language REXX for VM/CMS that would draw a nice Christmas tree on the screen and print a few warm words.
Figure 1 - Christmas Tree worm
Figure 1 - Christmas Tree worm
The hero of our story surely meant well, but Evil Bug interfered, overloading the network and exploiting the self-replicating Christmas program to paralyze private email network IBM Vnet all over the world for two days (the chain was this: university network - EARN - BitNet - IBM Vnet). The hero was suspected to be an anti-hero and his touching greeting, a worm. The programmer's malicious intent was never proved, but Evil Bug was surely involved in that story.

Unprecedented-generosity show

People traditionally exchange presents on Christmas Eve and New Year's Eve. Beautifully packed boxes under the Christmas tree or cute souvenirs in Christmas stockings hung by the fireplace - this is what traditional Christmas and New Year presents look like. However, surprises are particularly pleasant.
Amazon was one of the first Internet services with dozens of thousands of goods of all kinds sold and bought daily. A perfect place to pick presents! That's what site visitors were doing on December 12, 2014. Huge excitement was caused by the fact that thousands of goods were selling for the wonderful price of just 1 penny (source). Infinitely grateful to Amazon for such a gorgeous Christmas present, the buyers were enthusiastically filling their carts. Meanwhile, Evil Bug was watching and smirking, anticipating the reaction of the sellers who knew nothing yet about the huge losses they had suffered.
The bug was hiding in RepricerExpress software responsible for synching prices in online stores. This software facilitates competition by enabling sellers to respond promptly to price fluctuations for like products.
What did Evil Bug do? It sneaked into RepricerExpress when it was only going through development and testing, but never showed up until... one of the sellers, caught in the pre-holiday turmoil, accidentally set a single price - 1 penny - for all of their stock. The software took that value as a minimum price and cut the prices for other sellers' like products accordingly.
That behavior had to do with the fact that when developing the UI, the software authors had not implemented a feature to allow sellers to specify individual minimum prices. More than that, the prices would automatically update within certain intervals. The bug was fixed in the subsequent versions of the software.
Figure 2 - Fixed UI (with newly added column Your Minimum Price)
Figure 2 - Fixed UI (with newly added column Your Minimum Price)
The day when the bug revealed itself will be remembered for long by the Amazon sellers. That day, they lost thousands of dollars and many nearly went bankrupt (source). But for the prompt action taken by Amazon, which managed to cancel the majority of orders placed on the affected items, the largest online store's reputation would have been severely damaged.
The RepricerExpress developers apologized for the bug in a statement posted on their official blog.

Apple VS New Year

Remember the film "How the Grinch Stole Christmas"? It seems that the Evil Bug took it as a source of inspiration when thinking up a plan of attacking Apple devices. In February 2016, Apple users discovered an interesting bug. There was a legend going around on social networks saying that if you changed the date to January 1, 1970, on your iPhone or iPad and restarted it, the system would completely crash leaving you with a brick with an Apple logo on it. The procedure was claimed to be irreversible. The bug was reported to be found on devices that employed 64-bit processors, such as Apple A7, A8, A8X, A9, and A9X: iPhone 5S and newer, iPad Air and iPad Mini 2 and newer, and the 6th generation iPod Touch. The operating system's version number did not matter.
Picture 2
Did anyone try it? Sure! A wave of Apple-gadget killings swept through the world. Fortunately, some handymen found a way to bring the "bricks" back to life. Apple never revealed the official cause of the bug, but they did confirm it could occur when manually changing the date to May 1970 or earlier on an iOS device.
Users carried out their own investigation and came up with the following explanation: the bug could have been caused by a negative-value variable used to store time in UNIX format. How could the value become negative?
Version 1. Since time was stored in UNIX-format, timing would start with January 1, 1970, that is, this date was a zero value. When changing time zones, the value could decrement below zero.
Version 2. The bug was typical of 64-bit devices, so perhaps the 32-bit time mark was computed first and then, after changing time zones, would be cast to the pointer size, causing the most significant bits to be filled incorrectly and... Welcome to the XXII century!

Sleep long with iPhone

Long, long sleep not interrupted by an alarm clock - isn't it what most of us dream of? Though not Gasprom, iPhones do make their owners' dreams come true! All those who were planning to start the first day of 2013 fresh and early and set up an alarm on their devices to January 1, "happily" overslept. Evil Bug obviously meant to turn a huge number of users into "sleeping beauties", as the iPhone alarm clock wouldn't work until January 3.
Picture 7
Apple preferred to keep silent again. However, rumors about the possible cause of the bug spread anyway. Apple uses the ISO week date standard, which is widely used by finance companies, as it enables convenient fiscal year planning. What is special about this standard is that a new year is considered as such only starting with the week the first Thursday of the year falls on. The ISO week date calendar contains 52 or 53 weeks (364 or 371 days), so it turned out iPhones were still living in the previous year and stepped into the new one (2013) on January 7, when the first week of the year began.
There was also an alternative explanation, where Steve Jobs himself took on the role of Evil Bug. The Apple founder was allegedly fond of sleeping in, hence that "feature". It's just a joke of course, but the consequences of that seemingly harmless bug were far from funny: the people were late for work, failed to get to important meetings in time, and lost money (source).

Flight cancelled

The price of a software bug is the factor that developers should never ignore. Here is another Christmas bug story to support this statement.
On December 12, 2014, the UK's air traffic control center of National Air Traffic Services (NATS) was faced with a software glitch, which brought the work of some of the airports, including such heavily loaded giants as Heathrow, Gatwick, Stansted, Birmingham, Cardiff, and Glasgow, to a halt. The problem was aggravated by the time that Evil Bug chose for the attack. It was a Friday afternoon, Christmas Eve.
The fault persisted for a little longer than half an hour - 36 minutes - but the price of the error behind it was steep, as illustrated by the following figures, which Evil Bug can be proud of:
  • 92 flights cancelled
  • 170 flights suspended
  • 10 planes re-routed to alternate airports
  • 125,000 passengers experiencing inconvenience
  • 623 million pounds of losses suffered
A situation like that could not pass unnoticed. An investigation was carried out. In their final report, the Civil Aviation Authority (CAA) and NATS described a bug found in the software of the System Flight Server (SFS). The SFS is responsible for real-time delivery of data to the Controllers of workstations within the NATS management system. There are two identical SFSs: primary and secondary. Both compute the same data. When the primary SFS shuts down, the secondary one comes into operation. The system did provide for hardware faults but for some reason lacked any protection against software exceptions.
The maximum permitted number of operational workstations (i.e. terminals from which traffic control and monitoring are carried out) running at a time was 193 - well, in theory at least. In reality, the SFS's code checked for another value, 151. That's why when 153 workstations attempted to connect simultaneously, the system reset with a subsequent crash. It was found later that the "latent software fault" had been present since as early as 1990. It's a wonder that it hadn't shown earlier.

The Year 2000 and Year 2038 problems

The New Year of 2000 was one of the most anticipated ones. As some experts of all sorts believed, the turn of the millennium was definitely going to be accompanied by the Apocalypse or, which is no less terrifying, rise of the machines.
What arguments did they give for their fear of Terminators? Logic! The first computers were slow, so programmers, unwilling to waste precious performance on trifles, decided to use two digits to represent the year in dates. For example, March 23, 1991, was represented as 23.03.91. This notation is nice and normal to the eye. However, from computers' viewpoint, it's not that simple. The years 2000 and 1900 were encoded by the same pair of digits, 00, so when the New Year of 2000 began, their internal clock would be set back to the year 1900.
People could not help visualizing the dreadful effects of such a terrible fault: software crashes, spontaneous missile launches, the financial market collapse. The most horrible things were expected to happen in Russia as a country worst prepared for the new millennium.
Well, 2017 is approaching, which means the Apocalypse never happened.
That said, certain bugs did show when the new millennium came:
  • British Telecom's computer networks were paralyzed and engineers had to analyze about a million of code lines to bring them back to life. It cost British Telecom quite a big sum of about 0.5 billion dollars.
  • In Spain, emergency conditions were observed at 9 nuclear plants - fortunately, without any serious consequences.
  • In Mongolia, the "Year 2000 problem" affected railway operation and ticket offices.
Some of the bugs were quite amusing:
  • Terms of imprisonment in one Spanish prison were stretched/cut by 100 years
  • In some Greek stores, buyers would get sales slips dated 1900
  • In a South Korean hospital, the patient monitoring software declared a one-year-old baby an old man of 99
  • The citizens of a small US town got electricity bills overdue by 100 years
The "Year 2000 problem" is a striking example of the profound effect that mass media have on humankind. The next wave of mass panic for a similar reason is expected in 2038. On January 19, 2038, at 03:14:07, Greenwich, computers and other devices using 32-bit operating systems will no longer be able to measure time properly. In many devices, system time is measured in seconds starting with January 1, 1970. The seconds are stored in a 32-bit value of type signed int (32-bit signed integer). Soon after the beginning of 2038, the counter will update with the 2,147,483,648th second, which the system will not be able to store, and switch to a negative value.
How to avoid a system error that will follow? Replace all 32-bit processors with 64-bit ones.

How to help Good?

Traditionally, Good always defeats Evil, but the struggle doesn't stop for a moment. Is there any chance to exterminate all Evil Bugs for good? That's unlikely, but we definitely have every chance to deal massive damage to their troops. To do that, programmers fighting on Good's side, i.e. for quality code, should wisely pick tools to help them in the fight. Arm yourself with PVS-Studio static analyzer! And be sure to check this short horror film about Unicorn PVS-Studio saving Penguin Linux from Evil Bug.
Feel inspired? Then let's help Good together! The PVS-Studio team has already made a big step forward by offering you the free version of our analyzer.
Picture 6
Dear programmers, good luck with your projects, and may Good always win in your evil-bug stories! Merry Christmas and a Happy New Year!

Friday, December 16, 2016

Re-analysis of Umbraco code

Time passes inexorably. It feels that just recently we announced the release of the C# static code analyzer, checked the first projects, and started writing articles about it. But a whole year has passed since that moment. It was a year of painstaking and hard work on diagnostic improvements, adding new diagnostic rules, gathering statistics on false positives and eliminating their causes, communicating with users, and tackling a great deal of other issues. It was a year of both small and large successes on this hard, but incredibly interesting, path we have chosen. Now it's time for a re-analysis of the Umbraco project that we checked right after the release of our C# analyzer a year ago.
Picture 3

Introduction

The first article about the Umbraco analysis was written by my colleague Andrey Karpov. This year the project continued development, and so far it contains about 3340 files with the extension ".cs", which is approximately 425 KLOC (at the time of the first check, the project had 3200 files with the extension ".cs", and 400 KLOC respectively).

On the first check, the analyzer found a relatively small number of errors, which were nevertheless quite interesting to write an article on, and to draw first conclusions about the work of the C# analyzer from. It is far more interesting doing the check now, when the analyzer has obtained dozens of new diagnostic rules, and improved it's mechanisms of searching for bugs; it's also quite amusing to compare the results of the present time check with the one we did a year ago. To do the analysis, I used the latest version of Umbraco source code, which is also available at GitHub, and also the latest version of PVS-Studio 6.11.
In the results of the check, we got 508 warnings. 71 warnings were first level, 358 - second level, 79 - third level.
Picture 4
The overall coefficient of issues density (the number of warnings per KLOC) was 1.12. This is a good indicator which corresponds to approximately one warning per a thousand lines of code. But warnings do not necessarily mean real errors. It's normal for any static analyzer to have a certain percentage of false positives. Quite often, the warnings look like real bugs, but later after inspection, it turns out that it's not so. Therefore, I won't be discussing the low level warnings, as the percentage of false positives is usually quite high there.
I have reviewed the warnings issued by PVS-Studio, and detected approximately 56% false positives at High and Meduim levels. The remaining warnings contain quite suspicious constructs that require additional review, as well as real errors in the code.
What can be said about the quality of the analyzer work, in comparison with 2015? The first thing that caught our eye is that there were none of the warnings present, which had been described in the previous article. It seems (or at least we want to believe) that the Umbraco developers paid attention to Andrey's article, and fixed the errors described in it. Although the project is of course in continuous development, and the bugs could be fixed anyway, during daily work. Anyway - there are almost no old mistakes. Still, there are a lot of new ones! I'll go through the most interesting bugs here.

The analysis results

Potential division by zero
PVS-Studio warning: V3064 Potential division by zero. Consider inspecting denominator 'maxWidthHeight'. ImageHelper.cs 154
PVS-Studio warning: V3064 Potential division by zero. Consider inspecting denominator 'maxWidthHeight'. ImageHelper.cs 155
private static ResizedImage GenerateThumbnail(....)
{
  ....
  if (maxWidthHeight >= 0)
  {
    var fx = (float)image.Size.Width / maxWidthHeight;  // <=
    var fy = (float)image.Size.Height / maxWidthHeight;  // <=
    ....
  }
  ....
}
The provided code fragment has two possible errors, although the second one will never be executed. The condition of the if block allows the maxWidthHeight variable to be equal to zero, which acts as a divisor inside the block. In general, this code can work normally for quite a long period of time, and this is the biggest danger in it. Looking at the name of the maxWidthHeight, we can conclude that it's value is most likely not equal to zero. Well, what if it is zero at some point of the execution? The correct version of this construction is as follows:
private static ResizedImage GenerateThumbnail(....)
{
  ....
  if (maxWidthHeight > 0)
  {
    var fx = (float)image.Size.Width / maxWidthHeight;
    var fy = (float)image.Size.Height / maxWidthHeight;
    ....
  }
  ....
}
The case where the variable maxWidthHeight is zero, should be inspected separately.
A vexatious typo
PVS-Studio warning: V3080 Possible null dereference. Consider inspecting 'context.Request'. StateHelper.cs 369
public static bool HasCookies
{
  get
  {
    var context = HttpContext;
    return context != null && context.Request != null &  // <=
           context.Request.Cookies != null &&
           context.Response != null &&
           context.Response.Cookies != null;
  }
}
There is a typo: the & operator is used instead of &&. The condition context.Request.Cookies != null will be checked regardless of the result from the check of the previous condition context.Request != null. This will inevitably lead to access by a zero reference in case the variable context.Request is null. The correct version of this construction is as follows:
public static bool HasCookies
{
  get
  {
    var context = HttpContext;
    return context != null && context.Request != null &&
           context.Request.Cookies != null &&
           context.Response != null &&
           context.Response.Cookies != null;
  }
}
Untimely verification against null
PVS-Studio warning: V3027 The variable 'rootDoc' was utilized in the logical expression before it was verified against null in the same logical expression. publishRootDocument.cs 34
public bool Execute(....)
{
  ....
  if (rootDoc.Text.Trim() == documentName.Trim() &&  // <=
      rootDoc != null && rootDoc.ContentType != null)
  ....
}
The variable rootDoc is verified against null after the accessing via rootDoc.Text. The correct version of this construction is as follows:
public bool Execute(....)
{
  ....
  if (rootDoc != null &&
      rootDoc.Text.Trim() == documentName.Trim() &&
      rootDoc.ContentType != null)
  ....
}
A negative character index in the string
PVS-Studio warning: V3057 The 'Substring' function could receive the '-1' value while non-negative value is expected. Inspect the second argument. ContentExtensions.cs 82
internal static CultureInfo GetCulture(....)
{
  ....
  var pos = route.IndexOf('/');
  domain = pos == 0
    ? null
    : domainHelper.DomainForNode(
      int.Parse(route.Substring(0, pos)), current)  // <=
      .UmbracoDomain;
  ....
}
In the route string the program searches for the '/' character, after which the the variable is assigned with the pos variable. The author took into account the possibility of a character in the beginning of the string (pos == 0), but didn't take into account the possibility of its absence: in this case the variable pos will get the value -1. This will cause an exception upon the subsequent usage of the pos variable to extract the substring route.Substring(0, pos). The correct version of this construction is as follows:
internal static CultureInfo GetCulture(....)
{
  ....
  var pos = route.IndexOf('/');
  domain = (pos <= 0)
    ? null
    : domainHelper.DomainForNode(
      int.Parse(route.Substring(0, pos)), current)
      .UmbracoDomain;
  ....
}
Similar warnings:
  • V3057 The 'Substring' function could receive the '-1' value while non-negative value is expected. Inspect the first argument. DefaultUrlProvider.cs 81
  • V3057 The 'Substring' function could receive the '-1' value while non-negative value is expected. Inspect the second argument. DefaultUrlProvider.cs 84
  • V3057 The 'Substring' function could receive the '-1' value while non-negative value is expected. Inspect the first argument. DefaultUrlProvider.cs 126
  • V3057 The 'Substring' function could receive the '-1' value while non-negative value is expected. Inspect the second argument. DefaultUrlProvider.cs 127
  • V3057 The 'Substring' function could receive the '-1' value while non-negative value is expected. Inspect the first argument. PublishedContentCache.cs 147
  • V3057 The 'Substring' function could receive the '-1' value while non-negative value is expected. Inspect the second argument. PublishedContentCache.cs 148
  • V3057 The 'Substring' function could receive the '-1' value while non-negative value is expected. Inspect the second argument. ContentFinderByNiceUrlAndTemplate.cs 35
  • V3057 The 'Substring' function could receive the '-9' value while non-negative value is expected. Inspect the second argument. requestModule.cs 187
  • V3057 The 'Substring' function could receive the '-1' value while non-negative value is expected. Inspect the second argument. Action.cs 134
  • V3057 The 'Substring' function could receive the '-1' value while non-negative value is expected. Inspect the first argument. LegacyShortStringHelper.cs 130
  • V3057 The 'Substring' function could receive the '-1' value while non-negative value is expected. Inspect the second argument. StringExtensions.cs 573
Time is money
PVS-Studio warning: V3057 The 'DateTime' constructor receives the '0' value while positive value is expected. Inspect the second argument. DateTimeExtensions.cs 24
PVS-Studio warning: V3057 The 'DateTime' constructor receives the '0' value while positive value is expected. Inspect the third argument. DateTimeExtensions.cs 24
PVS-Studio warning: V3057 The 'DateTime' constructor receives the '0' value while positive value is expected. Inspect the third argument. DateTimeExtensions.cs 26
public static DateTime TruncateTo(this DateTime dt, 
  DateTruncate truncateTo)
{
  if (truncateTo == DateTruncate.Year)
    return new DateTime(dt.Year, 0, 0);  // <= x2
  if (truncateTo == DateTruncate.Month)
    return new DateTime(dt.Year, dt.Month, 0);  // <=
  ....
}
This small snippet also contains error 3, immediately detected by diagnostic rule V3057. All the errors related to incorrect initialization of the object of the DateTime class, whose constructor is as follows: public DateTime(int year, int month, int day). At the same time, the parameters yearmonth, and day cannot take values <1. Otherwise, an ArgumentOutOfRangeException will be thrown. The correct version of this construction is as follows:
public static DateTime TruncateTo(this DateTime dt, 
  DateTruncate truncateTo)
{
  if (truncateTo == DateTruncate.Year)
    return new DateTime(dt.Year, 1, 1);
  if (truncateTo == DateTruncate.Month)
    return new DateTime(dt.Year, dt.Month, 1);
  ....
}
Erroneous condition
PVS-Studio warning: V3125 The 'ct' object was used after it was verified against null. Check lines: 171, 163. ContentTypeControllerBase.cs 171
protected TContentType PerformPostSave<....>(....)
{
  var ctId = Convert.ToInt32(....);
  ....
  if (ctId > 0 && ct == null) 
    throw new HttpResponseException(HttpStatusCode.NotFound);
  ....
  if ((....) && 
      (ctId == 0 || ct.Alias != contentTypeSave.Alias))  // <=
  ....
}
There is the possibility of access by the null reference because of the condition (ctId > 0 && ct == null) in this code fragment. The exception HttpResponseException will be thrown only if both parts of the condition are true at the same time. In the case that the ctld variable is <= 0, the work will be continued anyway regardless of the value of the ct variable. The error needs to fixed in the second condition, where the ct is used. The correct version of this construction is as follows
protected TContentType PerformPostSave<....>(....)
{
  var ctId = Convert.ToInt32(....);
  ....
  if (ctId > 0 && ct == null) 
    throw new HttpResponseException(HttpStatusCode.NotFound);
  ....
  if ((....) && 
      (ctId == 0 || 
      (ct != null && ct.Alias != contentTypeSave.Alias)))
  ....
}
Similar warnings:
  • V3125 The '_repo' object was used after it was verified against null. Check lines: 104, 78. Installer.aspx.cs 104
  • V3125 The 'docRequest.RoutingContext.UmbracoContext' object was used after it was verified against null. Check lines: 57, 39. ContentFinderByIdPath.cs 57
  • V3125 The 'User' object was used after it was verified against null. Check lines: 90, 80. config.cs 90
  • V3125 The '_repo' object was used after it was verified against null. Check lines: 254, 247. installedPackage.aspx.cs 254
  • V3125 The 'node.NiceUrl' object was used after it was verified against null. Check lines: 917, 912. NodeExtensions.cs 917
  • V3125 The 'dst' object was used after it was verified against null. Check lines: 58, 55. DataEditorSetting.cs 58
  • V3125 The 'result' object was used after it was verified against null. Check lines: 199, 188. DefaultPreValueEditor.cs 199
  • V3125 The 'result' object was used after it was verified against null. Check lines: 241, 230. usercontrolPrevalueEditor.cs 241
An error in the format string
PVS-Studio warning: V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Format items not used: {1}. Arguments not used: 1st. HtmlHelperRenderExtensions.cs 938
public static IHtmlString EnableCanvasDesigner(....)
{
  ....
  string noPreviewLinks = @"<link href=""{1}"" type=
    ""text/css"" rel=""stylesheet"
    " data-title=""canvasdesignerCss"" />";
  ....
  if (....)
    result = string.Format(noPreviewLinks, cssPath) +  // <=
             Environment.NewLine;
  ....
}
The format string noPreviewLinks doesn't have a specifier '{0}' for the first argument cssPath of the string.Format method. The result of this code execution will be that we'll get an exception. The correct version of this construction is as follows:
public static IHtmlString EnableCanvasDesigner(....)
{
  ....
  string noPreviewLinks = @"<link href=""{0}"" type=
    ""text/css"" rel=""stylesheet"
    " data-title=""canvasdesignerCss"" />";
  ....
  if (....)
    result = string.Format(noPreviewLinks, cssPath) +
             Environment.NewLine;
  ....
}
Similar warnings:
  • V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Format items not used: {1}. Arguments not used: 1st. HtmlHelperRenderExtensions.cs 946
  • V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Arguments not used: path. requestModule.cs 204
  • V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Arguments not used: Alias.Replace(" ", ""). Template.cs 382
  • V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Arguments not used: Alias.Replace(" ", ""). Template.cs 387
  • V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Arguments not used: this.Value.ClientID. SliderPrevalueEditor.cs 221
Untimely verification against null. Again
PVS-Studio warning: V3095 The 'dataset' object was used before it was verified against null. Check lines: 48, 49. ImageCropperBaseExtensions.cs 48
internal static ImageCropData GetCrop(....)
{
  var imageCropDatas = dataset.ToArray();  // <=
  if (dataset == null || imageCropDatas.Any() == false)
    return null;
  ....
}
Unlike V3027 diagnostic - where the untimely verification against null was found within a single condition - here we are dealing with an attempt to access the null reference in a different statement. The variable dataset is converted to array first, and only then is verified against null. The correct version of this construction is as follows:
internal static ImageCropData GetCrop(....)
{
  var imageCropDatas = dataset?.ToArray();
  if (imageCropDatas == null || !imageCropDatas.Any())
    return null;
  ....
}
Similar warnings:
  • V3095 The 'display.PropertyEditor' object was used before it was verified against null. Check lines: 30, 43. ContentPropertyDisplayConverter.cs 30
  • V3095 The 'typedSource' object was used before it was verified against null. Check lines: 164, 198. DynamicQueryable.cs 164
  • V3095 The 'attempt.Result' object was used before it was verified against null. Check lines: 90, 113. DynamicPublishedContent.cs 90
  • V3095 The 'actionExecutedContext' object was used before it was verified against null. Check lines: 47, 76. FileUploadCleanupFilterAttribute.cs 47
  • V3095 The 'type' object was used before it was verified against null. Check lines: 92, 96. assemblyBrowser.aspx.cs 92
  • V3095 The 'httpContext' object was used before it was verified against null. Check lines: 235, 237. UmbracoContext.cs 235
  • V3095 The 'dst' object was used before it was verified against null. Check lines: 53, 55. DataEditorSetting.cs 53
  • V3095 The '_val' object was used before it was verified against null. Check lines: 46, 55. CheckBoxList.cs 46
  • V3095 The '_val' object was used before it was verified against null. Check lines: 47, 54. ListBoxMultiple.cs 47
  • V3095 The 'databaseSettings.ConnectionString' object was used before it was verified against null. Check lines: 737, 749. DatabaseContext.cs 737
  • V3095 The 'path' object was used before it was verified against null. Check lines: 101, 112. IOHelper.cs 101
A logic error
PVS-Studio warning: V3022 Expression 'name != "Min" || name != "Max"' is always true. Probably the '&&' operator should be used here. DynamicPublishedContentList.cs 415
private object Aggregate(....)
{
  ....
  if (name != "Min" || name != "Max")  // <=
  {
    throw new ArgumentException(
      "Can only use aggregate min or max methods on properties
       which are datetime");
  }
  ....
}
As can be seen in the message of the exception, the name variable can only take one of the values "Min", or "Max". At the same time, the condition of the exception should be simultaneous unequal to the name variable "Min" and "Max". But in this fragment the exception will be thrown regardless of the value of name. The correct version of this construction is as follows:
private object Aggregate(....)
{
  ....
  if (name != "Min" && name != "Max")
  {
    throw new ArgumentException(
      "Can only use aggregate min or max methods on properties
       which are datetime");
  }
  ....
}
In the Umbraco code, the analyzer found 32 more potentially dangerous constructions (although they may just be redundant checks). Here are some of them:
  • V3022 Expression 'macro == null' is always false. MacroController.cs 91
  • V3022 Expression 'p.Value == null' is always false. ImageCropperPropertyEditor.cs 216
  • V3022 Expression 'loginPageObj != null' is always true. ProtectPage.aspx.cs 93
  • V3022 Expression 'dictionaryItem != null' is always true. TranslateTreeNames.cs 19
  • V3022 Expression '!IsPostBack' is always true. EditUser.aspx.cs 431
  • V3022 Expression 'result.View != null' is always false. ControllerExtensions.cs 129
  • V3022 Expression 'string.IsNullOrEmpty(UmbracoSettings.TEMP_FRIENDLY_XML_CHILD_CONTAINER_NODENAME) == false' is always false. NotFoundHandlers.cs 128
  • V3022 Expression 'mem != null' is always true. ViewMembers.aspx.cs 96
  • V3022 Expression 'dtd != null' is always true. installedPackage.aspx.cs 213
  • V3022 Expression 'jsonReader.TokenType == JSONToken.EndArray && jsonReader.Value == null' is always false. JSON.cs 263
A strange loop condition
PVS-Studio warning: V3022 Expression '!stop' is always true. template.cs 229
public Control parseStringBuilder(....)
{
  ....
  bool stop = false;
  ....
  while (!stop)  // <=
  {
    ....
  }
  ....
}
Another suspicious construction, detected by the V3022 diagnostic. The variable stop isn't used inside the while block. The block has quite a large code fragment, about 140 lines of code, that's why I won't cite it here. Here is the result of searching for the stop variable:
Picture 5
Most likely, it's not an infinite loop, as we can see a break here, as well as the exception handling blocks. Nevertheless, the loop looks very strange, and may contain a potential error.
Infinite recursion
PVS-Studio warning: V3110 Possible infinite recursion inside 'Render' method. MenuSplitButton.cs 30
protected override void Render(System.Web.UI.HtmlTextWriter writer)
{
  writer.Write("</div>");
  base.Render(writer);
  this.Render(writer);  // <=
  writer.Write("<div class='btn-group>");
}
Apparently, this code fragment has a bug caused by an infinite recursion. After the call of the Render method of the base class, there is a recursive call of the overloaded Render method "by the analogy". Perhaps, the method this.Render should contain some condition of the exit from the recursion. However, it's hard to make a clear conclusion as to what the correct variant of this construction should be.

Conclusion

So, the recheck of the Umbraco project showed significant progress in PVS-Studio, in searching for potentially dangerous and erroneous constructs in C# code. The analyzer has proven its effectiveness once again. Of course, projects should not be checked once a year, because the maximum effect of static analysis is achieved only through regular use. This allows the fixing of bugs effectively, and in good time, without letting them get to the build system, and to the end-users.
Use static analysis! We added the possibility to use our analyzer for free so that everybody could do that. Good luck in the battle against errors and bugless code!