You are viewing a read-only archive of the Blogs.Harvard network. Learn more.

Shame on Me: the missing code review (or is it unit testing?)

So a bug was reported not long ago. Let’s say “sensitive data” was available where it shouldn’t have been. I had an API view, a view that was returning json only, or should have been. Apparently early on in the project, I had added a comment to that view, to make sure it was returning the appropriate data. So I had added an html comment to a view that was supposed to be returning json. And I had it outputting everything under the sun it could output.

<!--
"secure data" => "stuff I don't want the user to see",
... 30-40 lines of this ...
-->
{"success": true}

So there’s 2 problems with that. The most important of which is that with DOM inspection tools, like chrome developer tools or firebug, you can inspect the return value of that request and see the data I don’t really want you to see. The other problem is that I’ve created invalid json, and whatever is checking that success is clearly not using that value.

Why did I do this?

This was early on in development and I was debugging the easiest way I could.

What could have prevented this?

A code review of any kind. One look at that file would have been a red flag to anyone. But a view file that was only outputting true or false never seemed like something I ever needed to look at once I got it working.