User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20100106 SeaMonkey/2.1a1pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20100106 SeaMonkey/2.1a1pre An attachment with an irregular extension is displayed as inline text. It is unable to open it or saved it as a file. Reproducible: Always Steps to Reproduce: 1. Copy and rename a BINARY file to something like "foo.unknownext", e.g. copy foo.zip foo.unkownext please make sure that foo.zip exists. 2. Compose a new mail and attach both "foo.zip" and "foo.unknownext" 3. Send it to yourself 4. Receive the mail in seamonkey and open it Actual Results: Only "foo.zip" is listed in attachment pane. BASE64 result of "foo.unknownext" is displayed inline. Expected Results: Attachments with irregular extensions in file names shouldn't be displayed inline.

We'd need a unit test for this - it sounds like it might need to be a mozmill test, though it might be possible for it to be an xpcshell test that listens for the attachments as the message is streamed through libmime.

You're right. An xpcshell test is possible. I was going to upload it after my patch for bug 161806 is accepted.

Comment on attachment 426205 [details] Unit test thx very much for writing the unit test! When I run the unit test w/o the bug fix, the failure mode seems to be a timeout. I haven't tried it w/ the bug fix, but is that the expected failure mode?

Actually, with the bug fix, I get the same failure mode, I think: TEST-PASS | C:/mozilla-build/msys/tbirdhq/objdir-tb/mozilla/_tests/xpcshell/test _compose/unit/test_attachment.js | [checkAttachmentBody : 287] 0 != 4294967295 WARNING: NS_ENSURE_TRUE(mHiddenWindow) failed: file C:/mozilla-build/msys/tbirdh q/mozilla/xpfe/appshell/src/nsAppShellService.cpp, line 420 So I can't really tell that the test fails w/o the fix.

mHiddenWindow failure won't be a problem. A new hidden window will be created. It's supposed to fail like: TEST-UNEXPECTED-FAIL | e:/mozilla-build/src/obj-sm-dbg/mozilla/_tests/xpcshell/test_compose/unit/test_attachment.js | false == true - See following stack: JS frame :: e:\mozilla-build\src\mozilla\testing\xpcshell\head.js :: do_throw :: line 202 JS frame :: e:\mozilla-build\src\mozilla\testing\xpcshell\head.js :: do_check_eq :: line 232 JS frame :: e:\mozilla-build\src\mozilla\testing\xpcshell\head.js :: do_check_true :: line 244 JS frame :: e:/mozilla-build/src/obj-sm-dbg/mozilla/_tests/xpcshell/test_compose/unit/test_attachment.js :: MHS_onEndAllAttachments :: line 270 Do you mind sending me a more detailed log?

The test never seems to timeout, or at least not before I get tired of waiting. This is with a debug trunk build on 64 bit Windows 7.

my release build also hangs the same way. Maybe there's something missing from the patch?

I can't found out what is wrong. After disabled ogg, I can build a 64-bit SeaMonkey Suite and run the test. I was wondering if the unit test for bug 161806 works on your system.

I don't ever see the MsgHeaderSink getting notified at all when I run the test. I also notice us trying to find the chrome dir, which seems bad.

That is what bothers me. I don't know why it hasn't been called. It only happens when there is a syntax problem in JavaSript code. As the code works on my machine, I am not sure what is going on. Maybe recompilation will make it work? I tried to avoid accessing chrome dir. It is just that I can't find another way to save the attachment. I can easily create a mime emitter. However, It seems impossible to use it.

(In reply to comment #11) > I can easily create a mime emitter. I mean create an nsIMimeEmitter.

I'm pretty sure I can stream a message and get header sink notifications w/o using chrome. I got busy with a firedrill yesterday, but I'll try to get back to this today.

ok, the reason the test is failing for me is because nsMsgContentPolicy::DisableJSOnMailNewsUrlDocshells is erroring out on this code: nsCOMPtr<nsIFrameLoaderOwner> flOwner = do_QueryInterface(aRequestingContext, &rv); NS_ENSURE_SUCCESS(rv, rv); we hit that code when we try to load the message. I think this disable js stuff has been in some flux in the past couple months; is the rest of your tree up to date?

So the question for dmose (and maybe standard8) is, should DisableJSOnMailNewsUrlDocshells return errors when it can't get to the docshell, or should it do what it does when there's no requestingContext, which is to say, there's no docshell to run js in, so we're ok? Zane, I patched DisableJSOnMailNewsUrlDocshells locally, and your test then did finish, and fail, as it should.

With your patch applied, I ran into an error where we try to bring up the download manager. I fixed that by adding this line: prefs.setBoolPref("browser.download.manager.showWhenStarting", false); it looks like you're developing against SeaMonkey? that would explain the differences we're seeing!

Right. DisableJSOnMailNewsUrlDocshells hasn't been called on my machine.

right, but the patches and tests have to work with TB. Otherwise, the tinderboxes would all break when we checked in.

ping dmose/standard8 about #c15

(In reply to comment #15) > So the question for dmose (and maybe standard8) is, should > DisableJSOnMailNewsUrlDocshells return errors when it can't get to the > docshell, or should it do what it does when there's no requestingContext, which > is to say, there's no docshell to run js in, so we're ok? I think we've typically done that because if we don't know the requesting context, we can't 100% be sure what is requesting and what is safe. Is the questing context null, or just failing that QI?

My recollection is that it's failing the QI, but it's been a long time. I'd need to try it again to be sure. I'll try to add some more details later today.

Yes, the requestingContext is an nsGlobalChromeWindow, which is an nsIScriptGlobalObject So it can't be QI'd to a frame loader owner, and we bail with an error: nsCOMPtr<nsIFrameLoaderOwner> flOwner = do_QueryInterface(aRequestingContext, &rv); NS_ENSURE_SUCCESS(rv, rv); We check for a null aRequestingContext earlier in the method, and treat that as success. Here's the stack trace for the request: > msgbase.dll!nsMsgContentPolicy::DisableJSOnMailNewsUrlDocshells(nsIURI * aContentLocation=0x037917b8, nsISupports * aRequestingContext=0x03df4de8) Line 704 C++ msgbase.dll!nsMsgContentPolicy::ShouldLoad(unsigned int aContentType=0x00000006, nsIURI * aContentLocation=0x037917b8, nsIURI * aRequestingLocation=0x00000000, nsISupports * aRequestingContext=0x03df4de8, const nsACString_internal & aMimeGuess={...}, nsISupports * aExtra=0x00000000, short * aDecision=0x0044c144) Line 274 + 0x10 bytes C++ gklayout.dll!nsContentPolicy::CheckPolicy(unsigned int (unsigned int, nsIURI *, nsIURI *, nsISupports *, const nsACString_internal &, nsISupports *, short *)* policyMethod=0x65bd2a20, unsigned int contentType=0x00000006, nsIURI * contentLocation=0x037917b8, nsIURI * requestingLocation=0x00000000, nsISupports * requestingContext=0x03df4de8, const nsACString_internal & mimeType={...}, nsISupports * extra=0x00000000, short * decision=0x0044c144) Line 157 + 0x2f bytes C++ gklayout.dll!nsContentPolicy::ShouldLoad(unsigned int contentType=0x00000006, nsIURI * contentLocation=0x037917b8, nsIURI * requestingLocation=0x00000000, nsISupports * requestingContext=0x03df4de8, const nsACString_internal & mimeType={...}, nsISupports * extra=0x00000000, short * decision=0x0044c144) Line 218 + 0x29 bytes C++ docshell.dll!NS_CheckContentLoadPolicy(unsigned int contentType=0x00000006, nsIURI * contentLocation=0x037917b8, nsIPrincipal * originPrincipal=0x00000000, nsISupports * context=0x03df4de8, const nsACString_internal & mimeType={...}, nsISupports * extra=0x00000000, short * decision=0x0044c144, nsIContentPolicy * policyService=0x00000000, nsIScriptSecurityManager * aSecMan=0x00000000) Line 223 + 0x7e bytes C++ docshell.dll!nsDocShell::InternalLoad(nsIURI * aURI=0x037917b8, nsIURI * aReferrer=0x00000000, nsISupports * aOwner=0x00000000, unsigned int aFlags=0x00000001, const wchar_t * aWindowTarget=0x00000000, const char * aTypeHint=0x00000000, nsIInputStream * aPostData=0x00000000, nsIInputStream * aHeadersData=0x00000000, unsigned int aLoadType=0x00000001, nsISHEntry * aSHEntry=0x00000000, int aFirstParty=0x00000000, nsIDocShell * * aDocShell=0x00000000, nsIRequest * * aRequest=0x00000000) Line 7634 + 0x2b bytes C++ docshell.dll!nsDocShell::LoadURI(nsIURI * aURI=0x037917b8, nsIDocShellLoadInfo * aLoadInfo=0x00000000, unsigned int aLoadFlags=0x00000000, int aFirstParty=0x00000000) Line 1369 + 0x53 bytes C++ msglocal.dll!nsMailboxService::FetchMessage(const char * aMessageURI=0x03e3b078, nsISupports * aDisplayConsumer=0x03d94338, nsIMsgWindow * aMsgWindow=0x03e238f0, nsIUrlListener * aUrlListener=0x00000000, const char * aFileName=0x00000000, int mailboxAction=0x00000001, const char * aCharsetOverride=0x00000000, nsIURI * * aURL=0x00000000) Line 269 + 0x3b bytes C++ msglocal.dll!nsMailboxService::DisplayMessage(const char * aMessageURI=0x03e3b078, nsISupports * aDisplayConsumer=0x03d94338, nsIMsgWindow * aMsgWindow=0x03e238f0, nsIUrlListener * aUrlListener=0x00000000, const char * aCharsetOveride=0x00000000, nsIURI * * aURL=0x00000000) Line 305 C++ msgbase.dll!nsMessenger::OpenURL(const nsACString_internal & aURL={...}) Line 578 + 0x58 bytes C++ msgbase.dll!nsMsgDBView::LoadMessageByViewIndex(unsigned int aViewIndex=0x00000000) Line 1039 C++

In nsDocShell::InternalLoad, there's no requestingElement, because we're an xpcshell test, and this code fails: nsCOMPtr<nsIDOMElement> requestingElement; // Use nsPIDOMWindow since we _want_ to cross the chrome boundary if needed nsCOMPtr<nsPIDOMWindow> privateWin(do_QueryInterface(mScriptGlobal)); if (privateWin) requestingElement = privateWin->GetFrameElementInternal(); so we hit this code, and use mScriptGlobal as the requesting context. nsISupports* context = requestingElement; if (!context) { context = mScriptGlobal; }

> In nsDocShell::InternalLoad, there's no requestingElement, because we're an > xpcshell test Dare I ask how a docshell came to be in an xpcshell test? In any case, security-related things will be all weird in xpcshell; we don't even use the normal caps security manager there, iirc.

(In reply to comment #24) > Dare I ask how a docshell came to be in an xpcshell test? > Please check the last paragraph of comment #11. David said that he knows how to avoid that.

> Please check the last paragraph of comment #11. Nothing there screams "docshell" to me...

Yes, I didn't understand the reference to #c11. But the reason there's a docshell is that the test creates a hidden window, associates it with a msg window, and the test ends up using its docshell. We need a msg window and a dom window for the msg header sink stuff to work, but we could create fake ones like we do for other tests.

bz, also note that the only automated tests mailnews has are xpcshell tests. While Thunderbird can run mozmill tests automatically (not running on all tier-1 platforms right now, Linux seems missing), it ca not run anything based on mochitests, as those suites needs a browser window. SeaMonkey on the other hand can and does run mochitest* suites, but not mozmill, as its automation is a Thunderbird-specific tooling right now. So, in the end, all the shared mailnews code has that can be run by all of us is xpcshell-tests.

(In reply to comment #27) > We need a msg window and a dom > window for the msg header sink stuff to work, but we could create fake ones > like we do for other tests. I used to do that and I can't remember why I chose hidden window instead. Maybe because of memory leak or it seems complicated. I can recheck that.

Comment on attachment 420324 [details] [diff] [review] Don't return an empty file type thx, Zane. Clearing review request while you work on the test...

nsMessenger::SetWindow expects the DOM window to implement nsPIDOMWindow, which is not scriptable. And I can't find out how to create a DOM window without docshell. So I'm stuck in here. I know a way to bypass DisableJSOnMailNewsUrlDocshells var catMan = Cc["@mozilla.org/categorymanager;1"] .getService(Ci.nsICategoryManager); catMan.deleteCategoryEntry("content-policy" , "@mozilla.org/messenger/content-policy;1" , true ); I am not sure if it's acceptable.

> And I can't find out how to create a DOM window without docshell. You can't create one without the other.