Closed Bug 1497259 Opened Last year Closed Last year

Limit GV logging in official builds

Categories

(GeckoView :: General, enhancement, P1)

All
Android
enhancement

Tracking

(geckoview62+ fixed, firefox-esr60 wontfix, firefox62 wontfix, firefox63 fixed, firefox64 fixed)

RESOLVED FIXED
mozilla64
Tracking Status
geckoview62 + fixed
firefox-esr60 --- wontfix
firefox62 --- wontfix
firefox63 --- fixed
firefox64 --- fixed

People

(Reporter: jchen, Assigned: jchen)

References

(Regressed 1 open bug)

Details

Attachments

(3 files)

We should turn off debug logging for official builds of GV so things like URLs are not leaked to the log.
Fix the GV log module names so they are all under the GeckoView group.
Auto-fill values can contain personal information and must not appear in
the log.
Use the "geckoview.logging" pref to control GV logging.
The Focus team plans to release a new Focus 7.0.x version soon. We might consider uplifting at least the "Control GV logging through pref" patch to the GV 62 relbranch if it has privacy or performance benefits.
Priority: -- → P1
Comment on attachment 9015416 [details]
Bug 1497259 - 1. Fix log module names; r?droeh

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: N/A

User impact if declined: Sensitive URLs and other data can be exposed through Android logs

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Only affects how we do logging in GeckoView

String changes made/needed: None
Attachment #9015416 - Flags: approval-mozilla-beta?
Comment on attachment 9015416 [details]
Bug 1497259 - 1. Fix log module names; r?droeh

[GeckoView Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for consideration: This bug improves privacy of GeckoView consumers such as Focus

User impact if declined: Sensitive URLs and other data can be exposed to the Android logs.

Fix Landed on Version: 64

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Patch only affects how we do logging in GeckoView

String or UUID changes made by this patch: None
Attachment #9015416 - Flags: approval-mozilla-geckoview62?
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/50b9d2e6ac5f
1. Fix log module names; r=droeh
https://hg.mozilla.org/integration/autoland/rev/20cd9ac747da
2. Don't log auto-fill values; r=droeh
https://hg.mozilla.org/integration/autoland/rev/0df87bbe161e
3. Control GV logging through pref; r=droeh
Hi Chris, merge day is on Oct 22nd and moz-release will have beta63 code merge on Oct 15th (start of RC week). If we take this in beta63 this week (Pascal, fyi), will that be sufficient to ensure Focus ultimately gets it as GV63 starting from next week. Thoughts?
Flags: needinfo?(pascalc)
Flags: needinfo?(cpeterson)
Comment on attachment 9015416 [details]
Bug 1497259 - 1. Fix log module names; r?droeh

Removing GV uplift nom, Beta63+ as it's needed for GV/Focus/Klar
Attachment #9015416 - Flags: approval-mozilla-geckoview62?
Attachment #9015416 - Flags: approval-mozilla-beta?
Attachment #9015416 - Flags: approval-mozilla-beta+
(In reply to Ritu Kothari (:ritu) from comment #8)
> Hi Chris, merge day is on Oct 22nd and moz-release will have beta63 code
> merge on Oct 15th (start of RC week). If we take this in beta63 this week
> (Pascal, fyi), will that be sufficient to ensure Focus ultimately gets it as
> GV63 starting from next week. Thoughts?

Focus 8.0 is going to pick up GV 63 (Beta) this week, so we probably don't need to uplift this logging fix to the GV 62 relbranch.

OTOH, the Focus 8.0 release date is not until 11/13 so there might be another Focus 7.0.x dot release before then. If there is, we can revisit this bug after it has landed on GV 64 Nightly and 63 Beta. We can leave this bug marked status-geckoview62=fix-optional to remember it.
Flags: needinfo?(cpeterson)
Thanks Chris! If a Focus 7.0.x needs this uplift, I'll be happy to A+.
Flags: needinfo?(pascalc)
Comment on attachment 9015416 [details]
Bug 1497259 - 1. Fix log module names; r?droeh

I was told that we may need a GV62 build soon and that we should consider uplifting this fix. GV62+
Attachment #9015416 - Flags: approval-mozilla-geckoview62+
Product: Firefox for Android → GeckoView
Target Milestone: Firefox 64 → mozilla64
Regressions: 1570752
You need to log in before you can comment on or make changes to this bug.