JavaScript sleuthing: Buggy native JSON
Debugging is a huge part of any software engineer’s life: something goes wrong, and it’s your job to figure out what happened and how to fix it. The more time I spend debugging, the more I feel like a detective trying to tickle out details and evidence in order to determine what happened. Whenever I discover some obscure bug that was biting us, people often ask me how I figured it out. And so, I thought I’d start a series of posts based on some of the strangest bugs I’ve encountered in the hopes that it will help others to better understand how I work.
The bug
Our service engineering team flagged an issue on our servers. Requests were coming in that were causing PHP errors. Whenever requests are causing server-side errors, the natural first place to look is at the access logs to see exactly what the request is. The bug that was filed showed a request in (roughly) the following format:
/entry?someId={}&anotherId=27&foo=true&requestId={}From this, it was clear to tell that the requests were invalid because both someId and requestId didn’t actually contain identifying information, just curly braces. This was causing the server-side error as the PHP tried to use these invalid IDs. But why was this happening?
The investigation
Normally when an invalid request is received, my first inclination is that it’s some sort of attack. This has proven to be true in the past, but this didn’t fit in which any attack pattern I’m familiar with. Every request came in with the same format instead of the usual incremental change pattern that most attackers use. So an attack was off the table. That meant the request was coming from our code.
The entrypoint used in the request is for Ajax requests only, meaning that it was JavaScript code that created the URL for the request. I could tell which part of the page was creating the request by the arguments in the query string. The engineer for that part of the page double-checked his code and confirmed that nothing had changed with the recent release. Since all of our Ajax requests go through a common Ajax component, that pointed to a change deeper down in the JavaScript application stack.
To try and figure out what was going wrong, I looked at a valid request being sent from the same part of the page. The request should be in the following format:
/entry?someId=10&anotherId=27&foo=true&requestId=5So almost every query string argument value is a number except for one. Interestingly, the Boolean argument value remained fine as did the value for anotherId.
My next stop was to check out the Ajax component to see if there had been any changes there. After a quick look through the checkin log, I determined that nothing had changed. This pointed to a problem even deeper in the JavaScript application stack. What had changed so deep in the stack?
At that point I realized that we had just upgraded to the latest YUI 3 version in the previous release. Among the changes was a switch in the JSON utility to use the native JSON object if it’s available in the browser.
The theory
I reviewed the Ajax component code again and discovered that JSON.stringify() was getting called on all arguments before being added to the query string. This is done because the values could be arrays or objects. With the YUI upgrade fresh in my mind, I came up with my first solid theory about the problem: what if someone is using a browser whose native JSON implementation has a bug?
After thinking about it for a little while longer, I refined my theory to include what I believed to be the actual bug. I realized that not all numbers were being converted to {}, only some of them, and a quick look through the code made me realize that the missing numbers were most likely zero. My theory then became that there was a browser out there for which a call to JSON.stringify(0) returns “{}”.
The proof
I began testing the browsers that I knew had native JSON support and came up empty; I couldn’t reproduce the bug. Feeling a bit stumped, I asked a service engineer to pull the full request headers for the request in question. When he did, I saw something interesting in the user-agent string:
Mozilla/5.0 (Windows; U; Windows NT 6.0; fr; rv:1.9.1b1) Gecko/20081007 Firefox/3.1b1Fascinating. It looks like the person for whom this error is occuring is actually using Firefox 3.1 Beta 1. For those unaware, Firefox 3.1 became Firefox 3.5 after the third beta (i.e., there was no GA of Firefox 3.1). That means there’s someone out there using Firefox 3.1 Beta 1 for some unknown reason. But is that the problem browser?
I asked our service engineer how often this error was occuring. He responded that it was fairly frequently. I couldn’t imagine that there were that many people using Firefox 3.1 Beta 1, so I wasn’t sure if that was the source of the problem or not. I asked him to pull out a few more of the problem requests, complete with request headers, so I could look across them. That confirmed that every user encountering this problem was, in fact, using Firefox 3.1 Beta 1.
But a good sleuth doesn’t stop there. All I had proved was that all of the users were using the same browser. I hadn’t provided the source of the issue. After a lengthy search, I was able to find a Firefox 3.1 Beta 1 installer on FileHippo. I installed the browser and added Firebug. I popped open the Firebug console and typed JSON.stringify(0). The output was {}. Mystery solved.
The aftermath
I reported the issue to YUI as something that should be address. In the short term, I patched our own version of the JSON utility so that it never uses the native JSON.stringify() method. I wasn’t concerned about the performance impact of this decision since most of our users’ browsers don’t support JSON natively, and we only serialize a very small amount of data. Consistency and the elimination of the error is far more important than the few milliseconds we save by using the native JSON.stringify() method.
Disclaimer: Any viewpoints and opinions expressed in this article are those of Nicholas C. Zakas and do not, in any way, reflect those of my employer, my colleagues, Wrox Publishing, O'Reilly Publishing, or anyone else. I speak only for myself, not for them.
Both comments and pings are currently closed.
9 Comments
Very cool, I like your approach and the way you tackled the bug. This issue goes to illustrate that feature detection isn’t a cure-all. We need to vigilantly verify for bad implementations too. (Or at least catch them and fix them like you just did.)
Ara Pehlivanian on October 20th, 2009 at 11:19 am
For future reference, you can download older Firefox builds from Mozilla’s FTP server.
Markus Stange on October 20th, 2009 at 12:04 pm
This is an interesting situation since it’s only happening in a beta version of a browser, and an early beta at that. What obligation should anybody have to cater to those running on beta browsers? It’s a tough call, though, and I can see both sides of the argument.
Jonathan Snook on October 20th, 2009 at 2:55 pm
Impressive debug saga…
What surprises me though is that there are quite a few requests from Firefox 3.1 Beta 1.
Thanks for sharing the experience.
rnella01 on October 20th, 2009 at 3:06 pm
This is a very good article and I look forward to more in the series as it helps me understand the thought processes when trying to think laterally (i.e. “think outside the box”). I enjoyed this.
Mark McDonnell on October 20th, 2009 at 5:09 pm
Sounds like boolean conversion went wrong.
Wouldn’t it be better to test the parameter on the ‘JSON.stringify’ branch, and when `0′, check if the output was `{}’?
You would then address the bug and still be using the native parser, when appropriate.
Gabriel Gilini on October 20th, 2009 at 7:06 pm
I’ve been following JSON.
When I noticed the topic at hand, I was thinking, IE or Mozilla Beta. Mozilla Beta in this case. Have you come across the IE-problem as well ? Something about an empty string if I remember correctly.
But how many people use the beta ?
Lennie on October 21st, 2009 at 2:46 am
I had a very similar problem happen to us.
In our case we had a JSON string that was being turned into an object.
This JSON string looked like this { “this is a \stest” }
Now, \s is not a valid escape character, but both eval(), several other JSON parsers and the older version of the JSON.org json parser we were using all happily accepted this.
I don’t remember what it did, if it just ignored the backslash and the s or just the backslash, but it was consistent.
At some point however JSON was decided to be finalized as it’s own protocol. At that point json.org’s json2.js parser was updated and Firefox and others got their own native implementations.
These versions failed on parsing that JSON entirely, due to the invalid escape sequence.
It was pretty tricky to track down. First we looked at code changes we made, but couldn’t see anything.
Anyways, I enjoyed your post
Robert Schultz on October 22nd, 2009 at 2:22 pm
@Markus – Thanks so much! I figured there must be something on Mozilla FTP, but my various searches didn’t turn up anything. Bookmarked!
@Jonathan – Yes, we had this debate internally. My feeling is that if there’s just one or two things that need to change to support a beta version (which users are still clearly using), then it’s worthwhile. I use the same argument for rare, X-grade browsers as well – I want this to work everywhere, so if the overhead is low, I’d rather just do it.
@Gabriel – I’m not sure better is the word. That is another approach, though the downside is that I’d be programming around this one particular browser. By using non-native JSON, the code is the same for everyone and I’ve eliminated another code branch to test.
Nicholas C. Zakas on October 23rd, 2009 at 8:35 pm
Comments are automatically closed after 14 days.