Closed Bug 1442551 Opened 2 years ago Closed 2 years ago

Improve CORS console messages

Categories

(Core :: DOM: Security, defect, P1)

58 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: setok, Assigned: vinoth)

References

(Blocks 1 open bug)

Details

(Whiteboard: [domsecurity-active])

Attachments

(2 files)

Attached file testmod.html
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_1) AppleWebKit/604.3.5 (KHTML, like Gecko) Version/11.0.1 Safari/604.3.5

Steps to reproduce:

Create a HTML file that imports an ES6 module from its parent directory. The exact same setup when the imported module is in the same directory as the HTML file.


Actual results:

None of the code is run, and no error is given. Ie. the module code inside the HTML file is not run and neither is the module code that it imports.


Expected results:

Both modules should have been run. At the very least an error should be displayed on the console if this setup is, for some reason, unacceptable for Firefox.
if you're testing it with file:/// scheme, it's the restriction of it.
it cannot access parent directory for security reason.
Yes, it's with file. I can't find any documentation on that policy (and the same setup does work with Safari). Without debating the merits of that policy, the very least Firefox could do is deliver an error reflecting the problem. At the moment it fails silently.
Status: UNCONFIRMED → NEW
Component: Untriaged → Networking: File
Ever confirmed: true
Product: Firefox → Core
Summary: ES6 modules cannot be loaded from parent directory → Accessing parent directory in file:/// should show descriptive error message
Yes, I saw some other people bump into this issue when developing locally.
I think it's better describing the restriction in the error message, and maybe suggesting development using web server.
Blocks: 435025
We fail AsyncOpen()'ing the file channel exactly here:

>	xul.dll!nsCORSListenerProxy::CheckPreflightNeeded(0x000001be5c18ce88, Default) Line 1024	C++	Symbols loaded.
 	xul.dll!nsCORSListenerProxy::UpdateChannel(0x000001be5c18ce88, Allow, Default) Line 962	C++	Symbols loaded.
 	xul.dll!nsCORSListenerProxy::Init(0x000001be5c18ce88, Allow) Line 423	C++	Symbols loaded.
 	xul.dll!DoCORSChecks(0x000001be5c18ce88, 0x000001be5463f840, {...}) Line 295	C++	Symbols loaded.
 	xul.dll!nsContentSecurityManager::doContentSecurityCheck(0x000001be5c18ce88, {...}) Line 616	C++	Symbols loaded.
 	xul.dll!nsBaseChannel::AsyncOpen2(0x000001be5d857430) Line 747	C++	Symbols loaded.
 	xul.dll!mozilla::dom::ScriptLoader::StartLoad(0x000001be5d81af60) Line 1175	C++	Symbols loaded.
 	xul.dll!mozilla::dom::ScriptLoader::StartFetchingModuleAndDependencies(0x000001be5d81a650, 0x000001be5c189900) Line 756	C++	Symbols loaded.
 	xul.dll!mozilla::dom::ScriptLoader::StartFetchingModuleDependencies(0x000001be5d81a650) Line 722	C++	Symbols loaded.
 	xul.dll!mozilla::dom::ScriptLoader::ProcessFetchedModuleSource(0x000001be5d81a650) Line 461	C++	Symbols loaded.
 	xul.dll!mozilla::dom::ScriptLoader::ProcessInlineScript(0x000001be5bc34828, eModule) Line 1548	C++	Symbols loaded.
 	xul.dll!mozilla::dom::ScriptLoader::ProcessScriptElement(0x000001be5bc34828) Line 1310	C++	Symbols loaded.
 	xul.dll!mozilla::dom::ScriptElement::MaybeProcessScript() Line 141	C++	Symbols loaded.
 	xul.dll!nsIScriptElement::AttemptToExecute() Line 247	C++	Symbols loaded.
 	xul.dll!nsHtml5TreeOpExecutor::RunScript(0x000001be5bc347a0) Line 736	C++	Symbols loaded.
 	xul.dll!nsHtml5TreeOpExecutor::RunFlushLoop() Line 543	C++	Symbols loaded.
 	xul.dll!nsHtml5ExecutorFlusher::Run() Line 133	C++	Symbols loaded.


mRequestingPrincipal->CheckMayLoad fails few lines above (expected), so we get here:

nsresult
nsCORSListenerProxy::CheckPreflightNeeded(nsIChannel* aChannel, UpdateType aUpdateType)
{
  // If this caller isn't using AsyncOpen2, or if this *is* a preflight channel,
  // then we shouldn't initiate preflight for this channel.
  nsCOMPtr<nsILoadInfo> loadInfo = aChannel->GetLoadInfo();
  if (!loadInfo ||
      loadInfo->GetSecurityMode() !=
        nsILoadInfo::SEC_REQUIRE_CORS_DATA_INHERITS ||
      loadInfo->GetIsPreflight()) {
    return NS_OK;
  }

  bool doPreflight = loadInfo->GetForcePreflight();

  nsCOMPtr<nsIHttpChannel> http = do_QueryInterface(aChannel);
> NS_ENSURE_TRUE(http, NS_ERROR_DOM_BAD_URI);

The channel is not an http channel, apparently.  The error then bubbles up and is swallowed w/o any console warning.


Fwding to DOM:Sec.
Component: Networking: File → DOM: Security
:vino, in all the cases where we return NS_ERROR_DOM_BAD_URI, we should actually log to the console. Not only in this particular case, but also in other cases where we return that error. I think we can simply use LogBlockedRequest().
Assignee: nobody → cegvinoth
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [domsecurity-active]
(In reply to Phabricator Automation from comment #6)
> Created attachment 8960118 [details]
> Bug 1442551 - Console log added for NS_ERROR_DOM_BAD_URI

Initially added console log related to this particular case.
Working on adding console log to all the cases were NS_ERROR_DOM_BAD_URI is returned.
Attachment #8960118 - Flags: review?(ckerschb)
Comment on attachment 8960118 [details]
Bug 1442551 - Console log added for NS_ERROR_DOM_BAD_URI

Christoph Kerschbaumer [:ckerschb] has approved the revision.

https://phabricator.services.mozilla.com/D766
Attachment #8960118 - Flags: review+
Summary: Accessing parent directory in file:/// should show descriptive error message → Improve CORS console messages
Comment on attachment 8960118 [details]
Bug 1442551 - Console log added for NS_ERROR_DOM_BAD_URI

That was already r+ed by me.
Attachment #8960118 - Flags: review?(ckerschb)
Keywords: checkin-needed
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/24893a95845d
Console log added for NS_ERROR_DOM_BAD_URI. r=ckerschb
Keywords: checkin-needed
Blocks: 1456721
https://hg.mozilla.org/mozilla-central/rev/24893a95845d
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
QA Whiteboard: [good first verify]
You need to log in before you can comment on or make changes to this bug.