923591 - checkLoadURIStrWithPrincipal() should not warn when returning NS_ERROR_DOM_BAD_URI - WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805303F4: file dev/caps/src/nsScriptSecurityManager.cpp, line 1563

Status: RESOLVED FIXED

(Core :: Security: CAPS, defect)

Description

[Karl Tomlinson (:karlt)]

NS_ERROR_DOM_BAD_URI is an expected result of checkLoadURIStrWithPrincipal().
The purpose of the method is to determine whether aPrincipal can load uri.  It
indicates the result by returning NS_OK or NS_ERROR_DOM_BAD_URI, and so should
not warn when returning these values.  checkLoadURIWithPrincipal() usually correctly doesn't warn when returning this expected result:

http://hg.mozilla.org/mozilla-central/annotate/51b36c5fd45f/caps/src/nsScriptSecurityManager.cpp#l1325

http://hg.mozilla.org/mozilla-central/annotate/51b36c5fd45f/caps/src/nsScriptSecurityManager.cpp#l1339
http://mxr.mozilla.org/mozilla-central/source/caps/src/nsNullPrincipal.cpp#258

http://hg.mozilla.org/mozilla-central/annotate/51b36c5fd45f/caps/src/nsScriptSecurityManager.cpp#l1373

http://hg.mozilla.org/mozilla-central/annotate/51b36c5fd45f/caps/src/nsScriptSecurityManager.cpp#l1394
http://hg.mozilla.org/mozilla-central/annotate/51b36c5fd45f/caps/src/nsScriptSecurityManager.cpp#l1241

http://hg.mozilla.org/mozilla-central/annotate/51b36c5fd45f/caps/src/nsScriptSecurityManager.cpp#l1435

http://hg.mozilla.org/mozilla-central/annotate/51b36c5fd45f/caps/src/nsScriptSecurityManager.cpp#l1473

If the caller thinks that NS_ERROR_DOM_BAD_URI is not expected, then it can
warn, but there is already the consoleservice log if DONT_REPORT_ERRORS is not
set, or in one rare case already a warning from checkLoadURIWithPrincipal():

http://hg.mozilla.org/mozilla-central/annotate/51b36c5fd45f/caps/src/nsScriptSecurityManager.cpp#l1272

The client code triggering 50 warnings in this case is
http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/NewTabUtils.jsm#735
but that is not the bug.

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: patch

I've kept NS_ENSURE_SUCCESS for those that like the way it reports that the
nsresult code isn't changing as the call stack unwinds.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 813677 [details] [diff] [review]
patch

Please add a comment inside that if, both places, explaining why we have that separate codepath.  Thank you for fixing this!

r=me

Comment 3

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/020593d16a93

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/020593d16a93