Closed Bug 1777801 Opened 3 months ago Closed 3 months ago

Incomplete condition before calling GetScriptLoadContext()

Categories

(Core :: XPConnect, defect)

defect

Tracking

()

RESOLVED FIXED
104 Branch
Tracking Status
firefox104 --- fixed

People

(Reporter: arai, Assigned: jonco)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

https://searchfox.org/mozilla-central/rev/da3cc1b31be7a7d919d6ebde4e3c033a83446e05/js/loader/ScriptLoadRequest.cpp#114,116-117

void ScriptLoadRequest::Cancel() {
...
  if (HasLoadContext()) {
    GetScriptLoadContext()->MaybeCancelOffThreadScript();

https://searchfox.org/mozilla-central/rev/da3cc1b31be7a7d919d6ebde4e3c033a83446e05/js/loader/ScriptLoadRequest.cpp#126-133

bool ScriptLoadRequest::HasScriptLoadContext() const {
  return HasLoadContext() && mLoadContext->IsWindowContext();
}

mozilla::dom::ScriptLoadContext* ScriptLoadRequest::GetScriptLoadContext() {
  MOZ_ASSERT(mLoadContext);
  return mLoadContext->AsWindowContext();
}

GetScriptLoadContext() requires HasScriptLoadContext(), instead of HasLoadContext() alone.

Same situation in
https://searchfox.org/mozilla-central/rev/da3cc1b31be7a7d919d6ebde4e3c033a83446e05/js/loader/ScriptLoadRequest.cpp#177-180

nsresult ScriptLoadRequest::GetScriptSource(JSContext* aCx,
                                            MaybeSourceText* aMaybeSource) {
  // If there's no script text, we try to get it from the element
  if (HasLoadContext() && GetScriptLoadContext()->mIsInline) {
Attached file testcase

Also, the file not found error isn't propagated to mozJSModuleLoader::ImportESModule and it fails with nullptr deref, even if the above HasLoadContext() is replaced to HasScriptLoadContext()

Flags: needinfo?(jcoppeard)
Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)

This fixes the overly general context kind checks pointed out in the bug and
adds error checking in ImportESModule.

This also showed that JS::ClearModuleEnvironment could fail if the module's
environment had not been initialized yet.

This patch also adds a couple more tests.

Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dee83eddd306
Check for failure before attempting to instantiate module graph in mozJSModuleLoader::ImportESModule r=arai
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 104 Branch
You need to log in before you can comment on or make changes to this bug.