miscoded

Advance warning: this blog post dives into the most complex bits of Yahoo Mail beta. Along the road I'll explain a few tools, tips and tricks I rely on when analysing problems. I'm afraid you may not understand much of this post without prior knowledge of JavaScript. Feel free to read it anyway, but you've been warned!



The other day I claimed to know why Y!Mail was unusable in Kestrel, but even as I posted that I had a feeling I didn't have the full story yet. Yes, I had found some incompatibility between Opera and Firefox that DID trip things up and made confusing "Generic error" messages appear. I even found the Mozilla bug report saying Moz's behaviour was a bug (and by extension that Opera's was correct) - but still, this seemed more like a symptom than an actual cause. Because even if Firefox didn't throw exceptions, how could Y!Mail get away with sending us so much invalid XML in the first place? Some of that XML contained information that WAS available inside Y!Mail when running in Firefox. Was Y!Mail's XML invalid only in Opera? If so, why?

So next day I knew understanding this issue was going to be top priority, no matter how much I'd like to procrastinate with simpler bugs before diving into Y!Mail source code.



For a background, I'll recap the initial debugging from the previous day. The problem was the "Generic error" message on the screenshot above on the right, which appeared on Y!Mail load and caused the whole Y!Mail to be unusable.

First step with code like this is always making the all-on-one-line source more readable with one of the wrap scripts available. Luckily it is very simple to make Opera use the wrapped source instead of the original: open the script's URL in a new tab, view source, copy it and do the wrapping, paste it back to the editor, save and use Opera's "Reload from cache" setting.

The reload from cache menu entry may not look very interesting, but it is currently your best friend when debugging problems in Opera. It allows you to insert any sort of debug code from your source editor and study the output almost instantly. That is very useful. (It does have bugs though, IFRAME contents is usually re-validated from server, and scripts may also be - I guess some part of Opera respects expiry and cache-control headers even for "reload from cache". It usually works but I have to keep an eye on the HTTP requests just in case.)

However, before you can insert any debug code, you have to find the part of the JavaScript that contains the problem. With something as complex as Yahoo Mail, that's not easy. The error message doesn't make it easier: just "Generic error" without a backtrace - hey Opera, we're old friends but I know your shortcomings in the debugging department better than any griping, Firebug-spoiled web developer. That error is so unbelievably unhelpful, did you know?

For searching through code and many other debug tasks, I highly recommend the HTTP debugger Microsoft Fiddler. If you do any sort of web application or web browser development, this tool is a must-have. Below is a screenshot of Fiddler showing parts of the traffic that occurs during a Y!Mail session, the selected session is from an XMLHttpRequest and you'll see how Fiddler shows you the markup that was sent to the server (top right pane) and the markup that was returned (bottom right):



As you see, Fiddler keeps a record of all the code a website loads or sends - and you can search through all that code at once! To attack a problem like this I might try to search for a part of the error message:


The response seems promising:


The files that contained this message are now highlighted, so I can pick one of them and use the inline find field in the source pane to find the actual location:


Here's a match:

function launchWindowEventHandler(id, msg)
{

switch( id )
{
case "login_error":
alert( "Yahoo! Mail Beta experienced a login error: " + msg);
cancelLaunch();
break;

To see where this function is called from, a quick cheat might be to edit the source and call a non-existing function just inside it to check the stack trace in the error console. Then work backwards..

Actually, the patch itself came to the rescue and helped me find the error. To quickly add debug code to the patch I just used Opera's opera:config to turn off browser.js and make Opera read the browser.js file as a user JavaScript instead. When the signature check no longer applies I can modify the file:


User JavaScript can be really helpful when debugging. They partly make up the lack of a proper JavaScript debugger: you can replace specific functions with your own modified versions and do a lot of cool, custom stuff. In this case I happened to base my debug user script on browser.js.

Playing around with the "loadXML" section of the patch I noticed that DOMParser sometimes threw "Generic" errors: aha, it's Opera's way of saying "some very specific error happened when parsing that string you pretended was XML, but I can't be bothered telling you which one".

This much I knew when resuming the analysis the next day. Some searching gave me an overview of the places where Yahoo used the DOMParser to parse XML. What next?

In my experience, you can find shortcuts and resolve issues faster if you start writing tests when you start getting an overview of what a script is doing. When I think I know what's going on, I write a test that covers what I think happens - if I'm right, I've saved time by not exploring the full details. If I'm wrong, I'll have to dig around more. At this point I felt it was test case time..

I first decided to get a copy of all the "XML" Y!Mail tried parsing and compare Opera's and Firefox's output. Again I added debug code to the patch to harvest all the XML (note the postError call):

// 194334, Y!Mail faking IE's loadXML method
HTMLElement.prototype.loadXML = function (xml) { opera.postError(xml);

I copied all that XML to a single test file and checked each parse operation for three types of results: success, exception or error document.
Here is the first test case. (Slightly modified to avoid spam: I've replaced all E-mail addresses in the markup with a bogus one).

It shows that Firefox and Opera are exactly equivalent. Firefox creates "parseerror" documents where Opera throws. They agree entirely that some of the XML Opera tries to parse is invalid. So that experiement gave me no new information

Where exactly is the error thrown? Time to add some try...catch and lots of debug information to every place DOMParser is used. For example here:

O.__defineGetter_XMLDocument{
return;

}
if(this._XMLDocument==null){
try{
this._XMLDocument=(new DOMParser()).parseFromString(this.innerHTML,"text/xml");
}catch(e){ 

opera.postError( 'died in XMLDocument getter' ); 

}
}
return this._XMLDocument;

}
);

There is a pretty interesting observation to be made from the code above: what they try to parse here is the innerHTML of an element!. That's strange.. so the element already has an HTML DOM, but they want to serialize it with .innerHTML just to immediately parse it again??

Well.. In my line of work you learn to simply stop expecting things to make sense... Expecting sense from a typical piece of the JavaScript out there is a prejudice that can only mislead you.

What next? Well, let's focus on a single piece of apparently invalid XML. Let's pick the <subjects> one. Opera and Firefox agree that what Y!Mail asked Opera to parse is invalid XML, and it is due to the ampersand that occurs here:

Leather & Tweed

. In valid XML you can't have a sole ampersand. If you need one it must be written &amp;.

So, a theory combining the information we have now: maybe Opera sometimes outputs & and not &amp; when you read innerHTML? That might cause such problems for sure! So, let's whip up another quick test case...

let's see..

Again, theory is invalidated. Firefox and Opera sing perfectly from the same sheet here.

Where does the <subjects> markup come from? That I actually knew from random source browsing, but Fiddler would have found it easily: The localised part (url will likely change) of the Y!Mail libraries contained this code

var gXmlSubjectOMatique='<xml id="subjectOMatique"><subjects><p>Welcome to SPACE X</p><p>Astonishing feats of MENTALISM!</p>...

and tons of other funny subjects you might choose for your E-mails. And - wait - the problematic ampersand is indeed written &amp; here. Even in the source Opera gets. Somewhere that amp turns into an un-escaped &. But where and how?

Back to Fiddler to work out where Y!Mail uses the variable gXmlSubjectOMatique (finally a unique variable name it makes sense to search for - thanks Yahoo). It's here:

function w9(){var O=['<div style="display:none;">',j2(gXmlBranding+gXmlSubjectOMatique+gXmlLocaleSettings)

The w9 function just returns all that markup joined up. Where is it called from?

var O=w9();document.body.insertAdjacentHTML("afterBegin",O);appOnLoadHandler()

Right, so lots of "XML" markup is added to the HTML document with insertAdjacentHTML before Y!Mail calls the appOnLoadHandler which apparently at some point will run through those XML elements, read out their .innerHTML and send it to DOMParser. So, are ampersands inserted with inserAdjacentHTML for SOME reason treated differently in Opera?
No, they aren't. Another quick test case barking up the wrong tree.

Back to the w9 function: if you notice, it actually calls another function on those three strings. A function called j2. Wonder what it does?


Here's another quick trick: rather than searching through the code for a function declaration, if a function looks like it is visible from the global scope I usually use a bookmarklet to quickly check it out. So, back to the Y!Mail window and javascript:alert(j2); - now, what's all that about??

This is very odd.

First it just decides to do nothing if a variable is true.

  if (jo.O)
    {
      return L;
    }

I'll bet that there is some sort of browser sniffing behind that if. This really smells like a workaround for a problem in a specific browser..

The actual work going on in this function is here:

  var D = new RegExp("<xml(.*?)>(.*)");
  for (var O = 0;O < T.length;O++)
    {
      T[O] = T[O].replace(D, "<xml$1><!--$2-->");
    }

and it does.. it does.. eh?? looks like it wraps everything inside the XML element inside a comment tag?!? Let's see if that's right..

javascript:alert(j2( gXmlSubjectOMatique ));.
THERE YOU GO! It outputs a string starting <xml> with innerHTML, it outputs .

This breaks Y!Mail because they take a string of perfectly fine XML, wrap it in HTML comment tags, put it inside an <XML> tag, add it inside the BODY tag with that horrendous IE thingy called insertAdjacentHTML, serialize the DOM of said tags by reading .innerHTML, strip away the comment tags again and send the string to the DOMParser.

I can think of a few simpler, safer ways to parse XML. But apparently the "keep it simple, stupid" principle does not apply when adding compatibility layers on top of existing IE-only web apps like the Yahoo mail team has done with the webmail formerly known as Oddpost. And as usual, nothing is harder than being compatible with the workarounds against other browsers' bugs .