Closed Bug 1272864 Opened 3 years ago Closed 3 years ago

Missing securitry flag when creating loading timezone data

Categories

(Calendar :: Internal Components, defect)

Lightning 5.0
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: MakeMyDay, Assigned: MakeMyDay)

References

Details

Attachments

(1 file, 1 obsolete file)

On startup there's a warning when loading timezone information:

Warning: NetUtil.asyncFetch() requires the channel to have one of the security flags set in the loadinfo (see nsILoadInfo). Please create channel using NetUtil.newChannel()

Currently, Components.interfaces.nsILoadInfo.SEC_NORMAL is used when creating a new channel using newChannelFromURI2. When setting one of the five flags mentioned at [1], the warning doesn't appear anymore.

Although I haven't noticed that warning in another context, this probably should be fixed for the other occurences of newChannelFromURI2, too.

[1] https://mxr.mozilla.org/comm-central/source/mozilla/netwerk/base/nsILoadInfo.idl#40
Attached patch AddSecurityFlag-V1.diff (obsolete) — Splinter Review
This patch changes the security flag to SEC_REQUIRE_SAME_ORIGIN_DATA_INHERITS for all occurences of newChannelFromURI2 in calendar. I'm not sure whether this is the best of the available flags to make sure there are no side effects, but it at least makes the startup warning go away. If pushed a try build [1].

Maybe the process crash in the gdata test from bug 1266823 is also related to this - let's see.

[1] https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=11d84f17d4f073bd85e57a97096571c35a2a65cd
Assignee: nobody → makemyday
Status: NEW → ASSIGNED
Attachment #8752469 - Flags: review?(philipp)
Duplicate of this bug: 1266812
Please search for existing reports next time.
Version: Lightning 5.1 → Lightning 5.0
Comment on attachment 8752469 [details] [diff] [review]
AddSecurityFlag-V1.diff

Sorry, I missed the other bug.

Based on [1], there are assertions in xpcshell and mozmill test for debug build, which need to be addressed first, so removing r? for now. Optimized builds have no new failures and some random testing with the opt try build seemed to work ok.

Combined with the patch from bug 1266823, for the opt build the gdata test also passes completely [2].

Philipp is there any preference to use another flag than SEC_REQUIRE_SAME_ORIGIN_DATA_INHERITS?

[1] https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=37aa0cd8f656b35921f5ec1fb30fdf6c9993b7dd (different try run then mentioned above!)
[2] https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=d26f45bdae81da74f855d02a04104490069cdd96
Attachment #8752469 - Flags: review?(philipp) → feedback?(philipp)
Ok, I limit the change to the timezone service as the other ocurrences of SEC_NORMAL doesn't seem to cause any problems/error messages (at least for now). With that, also the tests pass successfully.

If this gets reviewed/approved after the upcoming merge, also an approval for beta would be required.
Attachment #8752469 - Attachment is obsolete: true
Attachment #8752469 - Flags: feedback?(philipp)
Attachment #8758029 - Flags: review?(philipp)
Attachment #8758029 - Flags: approval-calendar-aurora?(philipp)
Comment on attachment 8758029 [details] [diff] [review]
AddSecurityFlag-V2.diff

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

Ok, lets go with this. Not sure about the flags, but this looks like a viable option.
Attachment #8758029 - Flags: review?(philipp)
Attachment #8758029 - Flags: review+
Attachment #8758029 - Flags: approval-calendar-aurora?(philipp)
Attachment #8758029 - Flags: approval-calendar-aurora+
Comment on attachment 8758029 [details] [diff] [review]
AddSecurityFlag-V2.diff

Now that this didn't make it before the merge, we also need it to be uplifted to beta.
Attachment #8758029 - Flags: approval-calendar-beta?(philipp)
https://hg.mozilla.org/comm-central/rev/e7e4387b20c4d4837d2d7245f9a2e47ba42cb253
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 5.2
Attachment #8758029 - Flags: approval-calendar-beta?(philipp) → approval-calendar-beta+
You need to log in before you can comment on or make changes to this bug.