Closed Bug 1099587 Opened 10 years ago Closed 9 years ago

Make JS callers of ios.newChannel call ios.newChannel2 in mail/ and mailnews/

Categories

(MailNews Core :: Security, defect)

Product:
MailNews Core
Component:
Security
Type:
defect
Priority:
Not set
Severity:
blocker

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 41.0

People

(Reporter: philip.chee, Unassigned)

References

Details

Attachments

(4 files, 3 obsolete files)

patch for mailnews v2
1.19 KB, patch
neil
: review+
Details | Diff | Splinter Review
patch for Thunderbird v2
1.33 KB, patch
Details | Diff | Splinter Review
replace usage in mail/
9.49 KB, patch
rkent
: review+
Details | Diff | Splinter Review
replace usage in mailnews/
20.71 KB, patch
rkent
: review+
Details | Diff | Splinter Review 
+++ This bug was initially created as a clone of Bug #1099585 +++

(From Bug 1087720 comment #0)
> We have to change all JS callers of ioservice.newChannel to call
> ioservice.newChannel2. Since there are so many, we categorize and split them
> into different sub-bugs and use this bug as a tracking bug:
Attached patch patch for mailnews (obsolete) — — Splinter Review 
I only converted .newChannel() that was in Services.io. Other methods called the same in other objects are untouched (e.g. NetUtils). Is that enough?

While I did the changes according to https://bug1099585.bugzilla.mozilla.org/attachment.cgi?id=8527395, there seem to be some xpcshell test failures (in the touched imap code). I don't understand that.
Attachment #8530964 - Flags: review?(neil)
Attached patch patch for Thunderbird (obsolete) — — Splinter Review 

        Attachment #8530965 -
        Flags: review?(mkmelin+mozilla)

    
    

    
  
Bad, looks like we'd need to convert NetUtils.newChannel() too :(
https://bug1087720.bugzilla.mozilla.org/attachment.cgi?id=8521520

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch for Chat (obsolete)
        — 
          — Splinter Review
      

    


  
This one is untested.

        Attachment #8530968 -
        Flags: review?(florian)

    
    

    
  
Comment on attachment 8530968 [details] [diff] [review]
patch for Chat

Sorry, wrong bug.

        Attachment #8530968 -
        Attachment is obsolete: true

        Attachment #8530968 -
        Flags: review?(florian)

    
  
Depends on: 1095798

    
    

    
  


  
Adding the missing 8th argument fixed the test failures for me.

        Attachment #8530964 -
        Attachment is obsolete: true

        Attachment #8530964 -
        Flags: review?(neil)

        Attachment #8530978 -
        Flags: review?(neil)

    
    

    
  


  
Adding the missing 8th argument.

        Attachment #8530965 -
        Attachment is obsolete: true

        Attachment #8530965 -
        Flags: review?(mkmelin+mozilla)

        Attachment #8530979 -
        Flags: review?(mkmelin+mozilla)

    
    

    
  
I strongly encourage you to wait till Bug 1087442 lands which has information/documentation about all of these 8 arguments. You then also have to convert NetUtil.newChannel() calls. Bug 1087442 should hopefully make it into the codebase this week.

    
    

    
  
Comment on attachment 8530979 [details] [diff] [review]
patch for Thunderbird v2

Review of attachment 8530979 [details] [diff] [review]:
-----------------------------------------------------------------

Do we actually need this one? The other aboutRedirector.js files in m-c have not been changed this way

        Attachment #8530979 -
        Flags: review?(mkmelin+mozilla)

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
(In reply to Magnus Melin from comment #10)
> Do we actually need this one? The other aboutRedirector.js files in m-c have
> not been changed this way
I got the idea they want to remove .newChannel() once all callers are away. So they will probably convert their aboutRedirector.js soon too.

    
    

    
  
We probably need to wait for bug 1095798 yet so we can convert the NetUtils calls too.

    
    

    
  
(In reply to :aceman from comment #19)
> (In reply to Magnus Melin from comment #10)
> > Do we actually need this one? The other aboutRedirector.js files in m-c have
> > not been changed this way
> I got the idea they want to remove .newChannel() once all callers are away.
> So they will probably convert their aboutRedirector.js soon too.

Bug 1087725 has a patch which is outdated at the moment, but once it lands, you can base this patch based on Bug 1087725.
Depends on: 1087725

    
    

    
  
(In reply to Magnus Melin from comment #9)
> Those bugs now landed, and the c-c tree is orange. 
> E.g.
> http://ftp.mozilla.org/pub/mozilla.org/thunderbird/tinderbox-builds/comm-
> central-linux-debug/1418472395/comm-central_ubuntu32_vm-debug_test-xpcshell-
> bm06-tests1-linux32-build12.txt.gz

I am really curious how all these crashes happen, can someone provide a better stacktrace? Or tell me where exactly in NewChannelFromURIWithProxyFlags2 we crash? Scares me a little, because if we crash here we might also crash the browser.
Flags: needinfo?(acelists)

    
    

    
  
Christoph: there's a full stack for it in bug 1111304
Flags: needinfo?(acelists)

    
  
Blocks: 1111304

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
It would be really nice to not have such massive permabustage.
Severity: normal → blocker

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Yes, but the crashes are actually being fixed in bug 1111304.

    
    

    
  
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #22)

> I am really curious how all these crashes happen, can someone provide a
> better stacktrace? Or tell me where exactly in
> NewChannelFromURIWithProxyFlags2 we crash? Scares me a little, because if we
> crash here we might also crash the browser.

Can you and others who have vested interest in proper loadinfo 
take a look at the stack trace in
https://bugzilla.mozilla.org/show_bug.cgi?id=1111304#c54
and, if possible, suggest possible avenue for debugging or where the particular culprit is?

TIA
Flags: needinfo?(mozilla)

    
    

    
  
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #22)
> NewChannelFromURIWithProxyFlags2 we crash? Scares me a little, because if we
> crash here we might also crash the browser.
Not sure in which Firefox versions this may apply, but I am using FF aurora (the dev. edition) and it is exceptionally crashy these days, which wasn't a problem before. I need to upload some crash report. On the other hand, if this newchannel problems would be the culprit, then it probably wouldn't be able to load a single page or crash consistently. But I only observe random crashes.

    
    

    
  
(In reply to ISHIKAWA, Chiaki from comment #86)
> Can you and others who have vested interest in proper loadinfo 
> take a look at the stack trace in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1111304#c54
> and, if possible, suggest possible avenue for debugging or where the
> particular culprit is?

Yes, so in particular everything you need to know is covered by
https://bugzilla.mozilla.org/show_bug.cgi?id=1111304#c10

To sum it up, every ProtocolHandler that implements ::NewChannel2() has to attach a loadinfo on the newly created channel inside ::NewChannel2().
If a non-null LoadInfo is handed to the ioservice and a call to NewChannel2() succeeds, we assert that the channel indeed has a loadInfo attached to the newly created channel.

Let me know if you have further questions.
Flags: needinfo?(mozilla)

    
    

    
  
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #88)
> (In reply to ISHIKAWA, Chiaki from comment #86)
> > Can you and others who have vested interest in proper loadinfo 
> > take a look at the stack trace in
> > https://bugzilla.mozilla.org/show_bug.cgi?id=1111304#c54
> > and, if possible, suggest possible avenue for debugging or where the
> > particular culprit is?
> 
> Yes, so in particular everything you need to know is covered by
> https://bugzilla.mozilla.org/show_bug.cgi?id=1111304#c10
> 
> To sum it up, every ProtocolHandler that implements ::NewChannel2() has to
> attach a loadinfo on the newly created channel inside ::NewChannel2().
> If a non-null LoadInfo is handed to the ioservice and a call to
> NewChannel2() succeeds, we assert that the channel indeed has a loadInfo
> attached to the newly created channel.
> 
> Let me know if you have further questions.

What if, for some obscure reason, loadinfo attached to a channel is null(or nullptr)???

This alone would be considered a bug, it seems, but I have to ask.

TIA

    
  
Depends on: 1125618

        Attachment #8530978 -
        Flags: review?(neil) → review+

    
    

    
  
FYI, using the deprecated API now triggers a debug assertion as shown below. This causes Thunderbird to crash, e.g. during xpcshell tests on the Debug build servers.

> ###!!! ASSERTION: Deprecated, use NewChannel2 providing loadInfo arguments!
> ###!!! ASSERTION: Deprecated, use NewChannelFromURI2 providing loadInfo arguments!

    
    

    
  
(In reply to Stefan Sitter from comment #90)
> FYI, using the deprecated API now triggers a debug assertion as shown below.
> This causes Thunderbird to crash, e.g. during xpcshell tests on the Debug
> build servers.
> 
> > ###!!! ASSERTION: Deprecated, use NewChannel2 providing loadInfo arguments!
> > ###!!! ASSERTION: Deprecated, use NewChannelFromURI2 providing loadInfo arguments!

I have noticed this in my local test (which keeps on running after printing stack trace.)
This behavior is controlled by:
# NS_ASSERTION --- behavior can be modified by the following:
export XPCOM_DEBUG_BREAK=stack
# export XPCOM_DEBUG_BREAK=warn
# export XPCOM_DEBUG_BREAK=stack-and-abort

I think we need to fix this before the new release, don't we?

    
    

    
  
I'll try to create an updated patch for the issue.

    
    

    
  
(In reply to Stefan Sitter from comment #92)
> I'll try to create an updated patch for the issue.

FYI, in my local |make mozmill| testing of full debug build of C-C TB,
the following is the typical (and probably unique) stacktrace when the assertion is triggered.
I *THINK* Java code calls NewChannelFromURI via XPC remote call mechanism?

Excerpt from the log:
13478] ###!!! ASSERTION: Deprecated, use NewChannelFromURI2 providing loadInfo arguments!: 'false', file /REF-COMM-CENTRAL/comm-central/mozilla/netwerk/base/nsIOService.cpp, line 603
#01: nsIOService::NewChannelFromURI(nsIURI*, nsIChannel**) (/REF-COMM-CENTRAL/comm-central/mozilla/netwerk/base/nsIOService.cpp:610)
#02: NS_InvokeByIndex (/REF-COMM-CENTRAL/comm-central/mozilla/xpcom/reflect/xptcall/md/unix/xptcinvoke_x86_64_unix.cpp:178)
#03: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (/REF-COMM-CENTRAL/comm-central/mozilla/js/xpconnect/src/XPCWrappedNative.cpp:1417)
#04: XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) (/REF-COMM-CENTRAL/comm-central/mozilla/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1141)
#05: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (/REF-COMM-CENTRAL/comm-central/mozilla/js/src/jscntxtinlines.h:235)
#06: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (/REF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:727)
#07: Interpret(JSContext*, js::RunState&) (/REF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:2956)
#08: js::RunScript(JSContext*, js::RunState&) (/REF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:677)
#09: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (/REF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:747)
#10: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) (/REF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:784)
#11: JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) (/REF-COMM-CENTRAL/comm-central/mozilla/js/src/jsapi.cpp:4422)
#12: mozilla::dom::Function::Call(JSContext*, JS::Handle<JS::Value>, nsTArray<JS::Value> const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) (/REF-OBJ-DIR/objdir-tb3/dom/bindings/FunctionBinding.cpp:36)
#13: nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) (/REF-OBJ-DIR/objdir-tb3/dom/base/../../dist/include/mozilla/dom/FunctionBinding.h:53)
#14: nsGlobalWindow::RunTimeout(nsTimeout*) (/REF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsGlobalWindow.cpp:12642)
#15: nsGlobalWindow::TimerCallback(nsITimer*, void*) (/REF-OBJ-DIR/objdir-tb3/dom/base/../../dist/include/nsRefPtr.h:65)
#16: nsTimerImpl::Fire() (/REF-COMM-CENTRAL/comm-central/mozilla/xpcom/threads/nsTimerImpl.cpp:622)
#17: nsTimerEvent::Run() (/REF-OBJ-DIR/objdir-tb3/xpcom/threads/../../dist/include/nsRefPtr.h:50)
#18: nsThread::ProcessNextEvent(bool, bool*) (/REF-COMM-CENTRAL/comm-central/mozilla/xpcom/threads/nsThread.cpp:846)
#19: NS_InvokeByIndex (/REF-COMM-CENTRAL/comm-central/mozilla/xpcom/reflect/xptcall/md/unix/xptcinvoke_x86_64_unix.cpp:178)
#20: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (/REF-COMM-CENTRAL/comm-central/mozilla/js/xpconnect/src/XPCWrappedNative.cpp:1417)
#21: XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) (/REF-COMM-CENTRAL/comm-central/mozilla/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1141)
#22: ??? (???:???)
----the above is followed immediately by the below excerpt:
----the two top-most frames in the stack trace below remain the same for
----almost all the stack dumps, but the frames below the 2nd one are
--- different from one example to the other...
--- In any case, this means that NewChannelFromURI is called from Java code(?)
[13478] ###!!! ASSERTION: Please pass security info when creating a channel: 'loadInfo', file /REF-COMM-CENTRAL/comm-central/mozilla/netwerk/base/nsIOService.cpp, line 768
#01: nsIOService::NewChannelFromURIWithProxyFlags2(nsIURI*, nsIURI*, unsigned int, nsIDOMNode*, nsIPrincipal*, nsIPrincipal*, unsigned int, unsigned int, nsIChannel**) (/REF-COMM-CENTRAL/comm-central/mozilla/netwerk/base/nsIOService.cpp:768 (discriminator 1))
#02: nsIOService::NewChannelFromURI2(nsIURI*, nsIDOMNode*, nsIPrincipal*, nsIPrincipal*, unsigned int, unsigned int, nsIChannel**) (/REF-COMM-CENTRAL/comm-central/mozilla/netwerk/base/nsIOService.cpp:575)
#03: nsIOService::NewChannelFromURI(nsIURI*, nsIChannel**) (/REF-COMM-CENTRAL/comm-central/mozilla/netwerk/base/nsIOService.cpp:611)
#04: NS_InvokeByIndex (/REF-COMM-CENTRAL/comm-central/mozilla/xpcom/reflect/xptcall/md/unix/xptcinvoke_x86_64_unix.cpp:178)
#05: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (/REF-COMM-CENTRAL/comm-central/mozilla/js/xpconnect/src/XPCWrappedNative.cpp:1417)
#06: XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) (/REF-COMM-CENTRAL/comm-central/mozilla/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1141)
#07: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (/REF-COMM-CENTRAL/comm-central/mozilla/js/src/jscntxtinlines.h:235)
#08: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (/REF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:727)
#09: Interpret(JSContext*, js::RunState&) (/REF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:2956)
#10: js::RunScript(JSContext*, js::RunState&) (/REF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:677)
#11: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (/REF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:747)
#12: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) (/REF-COMM-CENTRAL/comm-central/mozilla/js/src/vm/Interpreter.cpp:784)
#13: JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) (/REF-COMM-CENTRAL/comm-central/mozilla/js/src/jsapi.cpp:4422)
#14: mozilla::dom::Function::Call(JSContext*, JS::Handle<JS::Value>, nsTArray<JS::Value> const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) (/REF-OBJ-DIR/objdir-tb3/dom/bindings/FunctionBinding.cpp:36)
#15: nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) (/REF-OBJ-DIR/objdir-tb3/dom/base/../../dist/include/mozilla/dom/FunctionBinding.h:53)
#16: nsGlobalWindow::RunTimeout(nsTimeout*) (/REF-COMM-CENTRAL/comm-central/mozilla/dom/base/nsGlobalWindow.cpp:12642)
#17: nsGlobalWindow::TimerCallback(nsITimer*, void*) (/REF-OBJ-DIR/objdir-tb3/dom/base/../../dist/include/nsRefPtr.h:65)
#18: nsTimerImpl::Fire() (/REF-COMM-CENTRAL/comm-central/mozilla/xpcom/threads/nsTimerImpl.cpp:622)
#19: nsTimerEvent::Run() (/REF-OBJ-DIR/objdir-tb3/xpcom/threads/../../dist/include/nsRefPtr.h:50)
#20: nsThread::ProcessNextEvent(bool, bool*) (/REF-COMM-CENTRAL/comm-central/mozilla/xpcom/threads/nsThread.cpp:846)
#21: NS_InvokeByIndex (/REF-COMM-CENTRAL/comm-central/mozilla/xpcom/reflect/xptcall/md/unix/xptcinvoke_x86_64_unix.cpp:178)
#22: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (/REF-COMM-CENTRAL/comm-central/mozilla/js/xpconnect/src/XPCWrappedNative.cpp:1417)
#23: XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) (/REF-COMM-CENTRAL/comm-central/mozilla/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1141)
#24: ??? (???:???)
--- end excerpt.

So let me check where the JS calls the problematic function.

Here is what I found out.
JavaScript calls newChannelFromURI() function via XPC RPC mechanism, I think.

--- listing from MXR for searching newChannelFromURI identifier ---
newChannelFromURI

If you can't find what you're looking for, you can always perform a free-text search for it.
Defined as a function in:

*** CI comment: we could probably remove the following function to
*** force link-time errors:
    mozilla/netwerk/base/nsIOService.cpp (View Hg log or Hg annotations)
        line 601, as member of class nsIOService -- nsIOService::NewChannelFromURI(nsIURI *aURI, nsIChannel **result) 

Defined as a function prototype in:

*** CI comment: we could probably remove the following to
*** to force compile-time errors, etc.
    mozilla/netwerk/base/nsIIOService.idl (View Hg log or Hg annotations)
        line 149 -- nsIChannel newChannelFromURI(in nsIURI aURI); 

*** CI: the following needs to be changed, I think, although it may not show up
in mozmill testing.
    mailnews/compose/src/nsMsgQuote.cpp (View Hg log or Hg annotations)
        line 179 -- rv = netService->NewChannelFromURI(aURL, getter_AddRefs(mQuoteChannel)); 

Referenced (in 28 files total) in:

*** CI comment. There are many calls from within calendar *.js files.
    calendar/base/content/calendar-dnd-listener.js (View Hg log or Hg annotations)
        line 287 -- var channel = Services.io.newChannelFromURI(uri); 
    calendar/base/modules/calProviderUtils.jsm (View Hg log or Hg annotations)
        line 33 -- let channel = aExisting || Services.io.newChannelFromURI(aUri); 
    calendar/base/src/calProtocolHandler.js (View Hg log or Hg annotations)
        line 52 -- var channel = Services.io.newChannelFromURI(uri, null); 
    calendar/providers/gdata/modules/gdataRequest.jsm (View Hg log or Hg annotations)
        line 200 -- let channel = Services.io.newChannelFromURI(uri); 
    calendar/providers/ics/calICSCalendar.js (View Hg log or Hg annotations)
        line 119 -- var channel = Services.io.newChannelFromURI(this.mUri);
        line 176 -- var channel = Services.io.newChannelFromURI(this.mUri);
        line 320 -- var channel = Services.io.newChannelFromURI(savedthis.mUri);
        line 770 -- var channel = Services.io.newChannelFromURI(this.mUri); 
    calendar/providers/wcap/calWcapRequest.js (View Hg log or Hg annotations)
        line 410 -- var channel = Services.io.newChannelFromURI(uri); 
    calendar/resources/content/publish.js (View Hg log or Hg annotations)
        line 129 -- var channel = Services.io.newChannelFromURI(icsURL); 
*** CI comment. there are many that need changing...
    mail/base/content/msgHdrViewOverlay.js (View Hg log or Hg annotations)
        line 1863 -- let stream = Services.io.newChannelFromURI(url).open(); 
    mail/components/addrbook/content/abCommon.js (View Hg log or Hg annotations)
        line 805 -- let channel = Services.io.newChannelFromURI(Services.io.newURI(aUri, null, null)); 
    mail/components/compose/content/MsgComposeCommands.js (View Hg log or Hg annotations)
        line 3809 -- let channel = Services.io.newChannelFromURI(url); 
    mail/components/nsMailDefaultHandler.js (View Hg log or Hg annotations)
        line 87 -- var channel = Services.io.newChannelFromURI(uri); 
    mail/test/resources/mozmill/mozmill/extension/resource/stdlib/securable-module.js (View Hg log or Hg annotations)
        line 317 -- var channel = ios.newChannelFromURI(newURI); 
    mailnews/base/prefs/content/accountcreation/util.js (View Hg log or Hg annotations)
        line 69 -- let chan = Services.io.newChannelFromURI(uri); 
    mailnews/extensions/bayesian-spam-filter/test/unit/resources/trainingfile.js (View Hg log or Hg annotations)
        line 47 -- var oStream = Services.io.newChannelFromURI(oUri).open(); 
    mailnews/imap/test/unit/test_dod.js (View Hg log or Hg annotations)
        line 84 -- let channel = Services.io.newChannelFromURI(uri.value); 
    mailnews/imap/test/unit/test_imapContentLength.js (View Hg log or Hg annotations)
        line 68 -- let channel = Services.io.newChannelFromURI(urlToRun);
        line 74 -- let attachmentChannel = Services.io.newChannelFromURI(attachmentURL); 
    mailnews/imap/test/unit/test_chunkLastLF.js (View Hg log or Hg annotations)
        line 73 -- let channel = Services.io.newChannelFromURI(uri.value); 
    mailnews/local/test/unit/test_mailboxContentLength.js (View Hg log or Hg annotations)
        line 39 -- let channel = Services.io.newChannelFromURI(urlToRun);
        line 45 -- let attachmentChannel = Services.io.newChannelFromURI(attachmentURL); 
    mailnews/news/test/unit/test_nntpContentLength.js (View Hg log or Hg annotations)
        line 51 -- let channel = Services.io.newChannelFromURI(urlToRun);
        line 58 -- let attachmentChannel = Services.io.newChannelFromURI(attachmentURL); 
*** CI comment: test code?
    mailnews/news/test/unit/test_bug37465.js (View Hg log or Hg annotations)
        line 23 -- let channel = Services.io.newChannelFromURI(uri); 
*** CI comment: the following is problematic. It is within M-C code!?
    mozilla/netwerk/base/NetUtil.jsm (View Hg log or Hg annotations)
        line 384 -- return this.ioService.newChannelFromURI(uri); 
*** CI comment: the following is problematic. It is within M-C code and part of mozmill!?
    mozilla/services/sync/tps/extensions/mozmill/resource/stdlib/securable-module.js (View Hg log or Hg annotations)
        line 317 -- var channel = ios.newChannelFromURI(newURI); 
**** CI comment: the following instances are from "suite".
    suite/common/src/nsAbout.js (View Hg log or Hg annotations)
        line 52 -- Services.io.newChannelFromURI(newURI); 
    suite/common/src/nsGopherProtocolStubHandler.js (View Hg log or Hg annotations)
        line 54 -- ios.newChannelFromURI(newURI); 
    suite/feeds/src/FeedConverter.js (View Hg log or Hg annotations)
        line 254 -- chromeChannel = Services.io.newChannelFromURI(chromeURI, null);
        line 260 -- chromeChannel = Services.io.newChannelFromURI(result.uri, null);
        line 520 -- Services.io.newChannelFromURI(uri); 
    suite/mailnews/addrbook/abCommon.js (View Hg log or Hg annotations)
        line 685 -- var channel = Services.io.newChannelFromURI(Services.io.newURI(aUri, null, null)); 
    suite/modules/WindowsPreviewPerTab.jsm (View Hg log or Hg annotations)
        line 71 -- let channel = Services.io.newChannelFromURI(uri); 
**** CI comment: this is from instant messaging.
    im/modules/ibCore.jsm (View Hg log or Hg annotations)
        line 108 -- Services.io.newChannelFromURI(Services.io.newURI(url, null, null)); 

TIA

    
    

    
  


  

        Attachment #8607158 -
        Flags: review?(rkent)

    
    

    
  


  
With this and the previous patch I get failing xpcshell tests from 100 down to 30 and the assertion is gone. Maybe you can test on try server? Please note that I don't really know what I'm doing in mailnews code, e.g. please check carefully if the chosen arguments match the required usage.

        Attachment #8607160 -
        Flags: review?(rkent)

    
    

    
  
Progress report: I've been making modifications to Stefan's patches, and pushing to the try server. Almost there.

    
    

    
  
Great. Can't wait to test the patches locally.
TIA

    
    

    
  
Comment on attachment 8607158 [details] [diff] [review]
replace usage in mail/

r+ with a few tweaks that I will checkin.

        Attachment #8607158 -
        Flags: review?(rkent) → review+

    
    

    
  
Comment on attachment 8607160 [details] [diff] [review]
replace usage in mailnews/

r+ with a few tweaks that I will checkin.

        Attachment #8607160 -
        Flags: review?(rkent) → review+

    
    

    
  
I tweaked the patches due to a few errors, mostly between confusion of use of Services.io.newChannelFromURI2 (which takes a url XPCOM object) and newChannel or newChannel2 (which takes a string URL).

Thanks for the patches!

http://hg.mozilla.org/comm-central/rev/710fbcea1b16
http://hg.mozilla.org/comm-central/rev/322e381cce2c
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 41.0

    
    

    
  
Comment on attachment 8607158 [details] [diff] [review]
replace usage in mail/

>+++ b/mail/test/resources/mozmill/mozmill/extension/resource/stdlib/securable-module.js
>      getFile: function getFile(path) {
>-       var channel = ios.newChannel(path, null, null);
>+       var channel = Services.io.newChannelFromURI2(Services.io.newURI(path, null, null),
>+                                                    null,
>+                                                    Services.scriptSecurityManager.getSystemPrincipal(),
>+                                                    null,
>+                                                    Components.interfaces.nsILoadInfo.SEC_NORMAL,
>+                                                    Components.interfaces.nsIContentPolicy.TYPE_OTHER);

I did a copy and paste error in statement on line 334. Could you please change it to match the statement on line 320, e.g. |Services.io.| to |ios.| and |Services.scriptSecurityManager.| to |scriptSecurityManager.|? Or add import for Services.jsm?

    
    

    
  
Fixed typo https://hg.mozilla.org/comm-central/rev/ff867369bb2e

(though actually that was one of the places where I switched to newChannel2)

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: