Closed
Bug 538139
Opened 15 years ago
Closed 14 years ago
Can't access attachments with unknown file name extensions
Categories
(MailNews Core :: MIME, defect)
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 538407
People
(Reporter: ZaneUJi, Assigned: ZaneUJi)
Details
Attachments
(3 files)
891 bytes,
patch
|
Details | Diff | Splinter Review | |
10.73 KB,
text/plain
|
Details | |
28.72 KB,
text/plain
|
Details |
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.
Assignee | ||
Comment 1•15 years ago
|
||
Attachment #420324 -
Flags: superreview?(bienvenu)
Attachment #420324 -
Flags: review?(bienvenu)
Updated•15 years ago
|
Version: unspecified → Trunk
Comment 2•15 years ago
|
||
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.
Assignee | ||
Comment 3•15 years ago
|
||
You're right. An xpcshell test is possible.
I was going to upload it after my patch for bug 161806 is accepted.
Comment 4•15 years ago
|
||
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?
Comment 5•15 years ago
|
||
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.
Assignee | ||
Comment 6•15 years ago
|
||
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?
Comment 7•15 years ago
|
||
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.
Comment 8•15 years ago
|
||
my release build also hangs the same way. Maybe there's something missing from the patch?
Assignee | ||
Comment 9•15 years ago
|
||
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.
Comment 10•15 years ago
|
||
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.
Assignee | ||
Comment 11•15 years ago
|
||
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.
Assignee | ||
Comment 12•15 years ago
|
||
(In reply to comment #11)
> I can easily create a mime emitter.
I mean create an nsIMimeEmitter.
Comment 13•15 years ago
|
||
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.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment 14•15 years ago
|
||
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?
Comment 15•15 years ago
|
||
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.
Comment 16•15 years ago
|
||
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!
Assignee | ||
Comment 17•15 years ago
|
||
Right. DisableJSOnMailNewsUrlDocshells hasn't been called on my machine.
Comment 18•15 years ago
|
||
right, but the patches and tests have to work with TB. Otherwise, the tinderboxes would all break when we checked in.
Comment 19•15 years ago
|
||
ping dmose/standard8 about #c15
Updated•15 years ago
|
Assignee: nobody → ZaneUJi
Component: MailNews: Message Display → MIME
Product: SeaMonkey → MailNews Core
QA Contact: message-display → mime
Comment 20•15 years ago
|
||
(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?
Comment 21•15 years ago
|
||
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.
Comment 22•15 years ago
|
||
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++
Comment 23•15 years ago
|
||
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;
}
Comment 24•15 years ago
|
||
> 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.
Assignee | ||
Comment 25•15 years ago
|
||
(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.
Comment 26•15 years ago
|
||
> Please check the last paragraph of comment #11.
Nothing there screams "docshell" to me...
Comment 27•15 years ago
|
||
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.
Comment 28•15 years ago
|
||
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.
Assignee | ||
Comment 29•15 years ago
|
||
(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 30•14 years ago
|
||
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...
Attachment #420324 -
Flags: superreview?(bienvenu)
Attachment #420324 -
Flags: review?(bienvenu)
Assignee | ||
Comment 31•14 years ago
|
||
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.
Comment 32•14 years ago
|
||
> And I can't find out how to create a DOM window without docshell.
You can't create one without the other.
Assignee | ||
Updated•14 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•