Skip to content
Snippets Groups Projects

Bug 43128: Use DuckDuckGo HTML on the Safest security level for Android

Merge Info

Related Issues

Backporting

Timeline

  • Immediate: patchset needed as soon as possible
  • Next Minor Stable Release: patchset that needs to be verified in nightly before backport
  • Eventually: patchset that needs to be verified in alpha before backport
  • No Backport (preferred): patchset for the next major stable

(Optional) Justification

  • Emergency security update: patchset fixes CVEs, 0-days, etc
  • Censorship event: patchset enables censorship circumvention
  • Critical bug-fix: patchset fixes a bug in core-functionality
  • Consistency: patchset which would make development easier if it were in both the alpha and release branches; developer tools, build system changes, etc
  • Sponsor required: patchset required for sponsor
  • Localization: typos and other localization changes that should be also in the release branch
  • Other: please explain

Merging

  • Merge to tor-browser - !fixups to tor-browser-specific commits, new features, security backports
  • Merge to base-browser - !fixups to base-browser-specific commits, new features to be shared with mullvad-browser, and security backports
    • NOTE: if your changeset includes patches to both base-browser and tor-browser please clearly label in the change description which commits should be cherry-picked to base-browser after merging

Issue Tracking

Review

Request Reviewer

  • Request review from an applications developer depending on modified system:
    • NOTE: if the MR modifies multiple areas, please /cc all the relevant reviewers (since gitlab only allows 1 reviewer)
    • accessibility : henry
    • android : clairehurst, dan
    • build system : boklm
    • extensions : ma1
    • firefox internals (XUL/JS/XPCOM) : jwilde, ma1
    • fonts : pierov
    • frontend (implementation) : henry
    • frontend (review) : donuts, richard
    • localization : henry, pierov
    • macOS : clairehurst, dan
    • nightly builds : boklm
    • rebases/release-prep : dan, ma1, pierov, richard
    • security : jwilde, ma1
    • signing : boklm, richard
    • updater : pierov
    • windows : jwilde, richard
    • misc/other : pierov, richard

Change Description

How Tested

Tested searching from urlbar and text selection with different search engines.

Edited by Pier Angelo Vendrame

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • requested review from @morgan

  • morgan requested review from @pierov and removed review request for @morgan

    requested review from @pierov and removed review request for @morgan

  • Pier Angelo Vendrame marked the checklist item Merge to tor-browser - !fixups to tor-browser-specific commits, new features, security backports as completed

    marked the checklist item Merge to tor-browser - !fixups to tor-browser-specific commits, new features, security backports as completed

  • Pier Angelo Vendrame marked the checklist item ****No Backport (preferred): patchset for the next major stable as completed

    marked the checklist item ****No Backport (preferred): patchset for the next major stable as completed

    • Resolved by Pier Angelo Vendrame

      The patch as it is seems to work.

      However, I wonder if we could be more surgical and limit the diff to something like this, instead of passing the settings around:

      Diff
      diff --git a/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/SearchUseCases.kt b/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/SearchUseCases.kt
      index b3e8a1763b5c..0a4b4f47fec9 100644
      --- a/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/SearchUseCases.kt
      +++ b/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/SearchUseCases.kt
      @@ -9,15 +9,19 @@ import mozilla.components.browser.state.action.EngineAction
       import mozilla.components.browser.state.action.SearchAction
       import mozilla.components.browser.state.search.SearchEngine
       import mozilla.components.browser.state.selector.findTabOrCustomTab
      +import mozilla.components.browser.state.selector.selectedTab
       import mozilla.components.browser.state.state.SessionState
       import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine
       import mozilla.components.browser.state.store.BrowserStore
       import mozilla.components.concept.engine.EngineSession
      +import mozilla.components.concept.engine.UnsupportedSettingException
       import mozilla.components.feature.search.ext.buildSearchUrl
       import mozilla.components.feature.session.SessionUseCases
       import mozilla.components.feature.tabs.TabsUseCases
       import mozilla.components.support.base.log.logger.Logger
       
      +import android.util.Log
      +
       /**
        * Contains use cases related to the search feature.
        */
      @@ -161,9 +165,20 @@ class SearchUseCases(
                   flags: EngineSession.LoadUrlFlags = EngineSession.LoadUrlFlags.none(),
                   additionalHeaders: Map<String, String>? = null,
               ) {
      +            Log.d("SecLevelTest", "Selected tab? " + (store.state.selectedTab == null))
      +            Log.d("SecLevelTest", "Engine state? " + (store.state.selectedTab?.engineState == null))
      +            Log.d("SecLevelTest", "Engine session? " + (store.state.selectedTab?.engineState?.engineSession == null))
      +            Log.d("SecLevelTest", "Settings? " + (store.state.selectedTab?.engineState?.engineSession?.settings == null))
      +            Log.d("SecLevelTest", "Security level? " + store.state.selectedTab?.engineState?.engineSession?.settings?.torSecurityLevel)
      +            var securityLevel: Int
      +            try {
      +                securityLevel = store.state.selectedTab?.engineState?.engineSession?.settings?.torSecurityLevel ?: 0
      +            } catch (e: UnsupportedSettingException) {
      +                securityLevel = 0
      +            }
                   val searchUrl = searchEngine?.let {
      -                searchEngine.buildSearchUrl(searchTerms)
      -            } ?: store.state.search.selectedOrDefaultSearchEngine?.buildSearchUrl(searchTerms)
      +                searchEngine.buildSearchUrl(searchTerms, securityLevel)
      +            } ?: store.state.search.selectedOrDefaultSearchEngine?.buildSearchUrl(searchTerms, securityLevel)
       
                   if (searchUrl == null) {
                       logger.warn("No default search engine available to perform search")

      It seems not to work :unamused:.

      I'd let Claire or Dan chime in, in case they know if we can avoid passing the settings around.

  • Pier Angelo Vendrame requested review from @clairehurst and removed review request for @pierov

    requested review from @clairehurst and removed review request for @pierov

  • Pier Angelo Vendrame
  • cypherpunks1 added 1 commit

    added 1 commit

    • 823c7a32 - fixup! Bug 40925: Implemented the Security Level component

    Compare with previous version

  • cypherpunks1 marked this merge request as draft from cypherpunks1/tor-browser@823c7a32

    marked this merge request as draft from cypherpunks1/tor-browser@823c7a32

  • Pier Angelo Vendrame resolved all threads

    resolved all threads

  • Pier Angelo Vendrame approved this merge request

    approved this merge request

  • cypherpunks1 marked this merge request as ready

    marked this merge request as ready

  • cypherpunks1 added 1 commit

    added 1 commit

    • 469882f0 - fixup! Bug 40925: Implemented the Security Level component

    Compare with previous version

  • cypherpunks1 marked this merge request as draft from cypherpunks1/tor-browser@469882f0

    marked this merge request as draft from cypherpunks1/tor-browser@469882f0

  • cypherpunks1 marked this merge request as ready

    marked this merge request as ready

  • Tested and seems to work fine, approved

  • clairehurst approved this merge request

    approved this merge request

  • added 27 commits

    • 469882f0...6a5b1bc7 - 25 commits from branch tpo/applications:tor-browser-128.2.0esr-14.0-1
    • f52fcb50 - fixup! Bug 40009: [android] Change the default search engines
    • cf07aee1 - fixup! Bug 40925: Implemented the Security Level component

    Compare with previous version

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading