Closed Bug 1272864 Opened 4 years ago Closed 4 years ago
Missing securitry flag when creating loading timezone data
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 , 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.  https://mxr.mozilla.org/comm-central/source/mozilla/netwerk/base/nsILoadInfo.idl#40
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 . Maybe the process crash in the gdata test from bug 1266823 is also related to this - let's see.  https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=11d84f17d4f073bd85e57a97096571c35a2a65cd
Assignee: nobody → makemyday
Status: NEW → ASSIGNED
Attachment #8752469 - Flags: review?(philipp)
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 , 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 . Philipp is there any preference to use another flag than SEC_REQUIRE_SAME_ORIGIN_DATA_INHERITS?  https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=37aa0cd8f656b35921f5ec1fb30fdf6c9993b7dd (different try run then mentioned above!)  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.
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.
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)
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 5.2
Attachment #8758029 - Flags: approval-calendar-beta?(philipp) → approval-calendar-beta+
Target Milestone: 5.2 → 5.0
You need to log in before you can comment on or make changes to this bug.