Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 10269: Do not proxy downloads for Widevine #5846

Merged
merged 1 commit into from
Jun 16, 2020

Conversation

jumde
Copy link
Contributor

@jumde jumde commented Jun 15, 2020

Resolves brave/brave-browser#10269

Submitter Checklist:

Test Plan:

  1. Open brave browser with a clean profile with the flag --use-dev-goupdater-url
  2. Navigate to https://bitmovin.com/demos/drm
  3. Download widevine
  4. Request for widevine should go through update.googleapis.com
  5. Download should either be trigged from *.gvt1.com and *.dl.google.com

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@jumde jumde requested a review from iefremov as a code owner June 15, 2020 19:26
@jumde jumde self-assigned this Jun 15, 2020
@jumde jumde requested review from bbondy and diracdeltas June 15, 2020 19:29
@jumde jumde changed the title [WIP] Fix 10269: Do not proxy downloads for Widevine Fix 10269: Do not proxy downloads for Widevine Jun 15, 2020
@jumde jumde added the CI/run-network-audit Run network-audit label Jun 15, 2020
auto request_info = std::make_shared<brave::BraveRequestInfo>(url);
int rc =
OnBeforeURLRequest_StaticRedirectWork(ResponseCallback(), request_info);
EXPECT_EQ(request_info->new_url_spec, base::EmptyString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer just "" instead of base::EmptyString, there is an explanation

// Therefore, DO NOT USE THESE AS A GENERAL-PURPOSE SUBSTITUTE FOR DEFAULT
// CONSTRUCTORS. There is only one case where you should use these: functions
// which need to return a string by reference (e.g. as a class member
// accessor), and don't have an empty string to use (e.g. in an error case).

@@ -74,6 +74,13 @@ int OnBeforeURLRequest_StaticRedirectWorkForGURL(
URLPattern::SCHEME_HTTP | URLPattern::SCHEME_HTTPS,
"*://dl.google.com/*");

static URLPattern widevineGvt1_pattern(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

most of names here don't match CC, I think it should be widevine_gvt1_pattern

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filed this issue: brave/brave-browser#10314 - Will try to address this soon. Added a to-do to the code.

@kjozwiak
Copy link
Member

Verification PASSED on macOS 10.15.5 x64 using the following build:

Brave | 1.12.31 Chromium: 83.0.4103.106 (Official Build) nightly (64-bit)
-- | --
Revision | ce7134bb3d95141cd18f1e65772a4247f282d950-refs/branch-heads/4103@{#694}
OS | macOS Version 10.15.5 (Build 19F101)
  • verified the STR outlined via Fix 10269: Do not proxy downloads for Widevine #5846 (comment)
  • ensured playing media using Widevine via Netflix, Disney+ & YT worked
  • ensured Widevine notification are being displayed when attempting to play media via Netflix, Disney+ & YT
  • ensured that Widevine is being downloaded via http://*.gvt1.com and not through redirector.brave.com when installing via Netflix, Disney+ & YT

Screen Shot 2020-06-17 at 12 49 31 AM

kjozwiak@Kamils-MBP ~ % curl -Ls -o /dev/null -w '%{url_effective}' -d '{"request":{"@os":"mac","@updater":"","acceptformat":"crx2,crx3","app":[{"appid":"oimompecagnajdejgnnjijobebaeigek","enabled":true,"installsource":"ondemand","ping":{"r":-2},"updatecheck":{},"version":"0.0.0.0"}],"arch":"x64","dedup":"cr","domainjoined":false,"hw":{"physmemory":16},"lang":"","nacl_arch":"x86-64","os":{"arch":"x86_64","platform":"Mac OS X","version":"10.15.5"},"prodchannel":"stable","prodversion":"83.1.12.20","protocol":"3.1","requestid":"{67dd796c-ac2d-46f9-a56d-6495e7c92640}","sessionid":"{fd35f289-fb9a-4a7c-8acf-9576b1d0c5eb}","updaterchannel":"stable","updaterversion":"83.1.12.20"}}' -H "Content-Type: application/json" -X POST http://localhost:8192/extensions https://update.googleapis.com/service/update2/json
http://localhost:8192/extensions)]}'

{"response":{"server":"prod","protocol":"3.1","daystart":{"elapsed_seconds":78917,"elapsed_days":4915},"app":[{"appid":"oimompecagnajdejgnnjijobebaeigek","cohort":"","status":"ok","cohortname":"","ping":{"status":"ok"},"updatecheck":{"status":"ok","urls":{"url":[{"codebase":"http://redirector.gvt1.com/edgedl/chromewebstore/L2Nocm9tZV9leHRlbnNpb24vYmxvYnMvYjYxQUFXaFBmeUtPbVFUYUhmRGV0MS1Wdw/"},{"codebase":"http://dl.google.com/chromewebstore/L2Nocm9tZV9leHRlbnNpb24vYmxvYnMvYjYxQUFXaFBmeUtPbVFUYUhmRGV0MS1Wdw/"},{"codebase":"https://dl.google.com/chromewebstore/L2Nocm9tZV9leHRlbnNpb24vYmxvYnMvYjYxQUFXaFBmeUtPbVFUYUhmRGV0MS1Wdw/"},{"codebase":"http://www.google.com/dl/chromewebstore/L2Nocm9tZV9leHRlbnNpb24vYmxvYnMvYjYxQUFXaFBmeUtPbVFUYUhmRGV0MS1Wdw/"},{"codebase":"https://www.google.com/dl/chromewebstore/L2Nocm9tZV9leHRlbnNpb24vYmxvYnMvYjYxQUFXaFBmeUtPbVFUYUhmRGV0MS1Wdw/"}]},"manifest":{"version":"4.10.1610.0","packages":{"package":[{"hash_sha256":"64d94786dc21a0b8328770c487d2544517da680280d123cb2e5bdbcf071d1126","size":4370363,"name":"4.10.1610.0_oimompecagnajdejgnnjijobebaeigek.crx","fp":"1.64d94786dc21a0b8328770c487d2544517da680280d123cb2e5bdbcf071d1126","required":true}]}}}}]}}https://update.googleapis.com/service/update2/json%
kjozwiak@Kamils-MBP ~ %

@srirambv
Copy link
Contributor

srirambv commented Jun 17, 2020

Verification passed on

Brave 1.12.31 Chromium: 83.0.4103.106 (Official Build) nightly (64-bit)
Revision ce7134bb3d95141cd18f1e65772a4247f282d950-refs/branch-heads/4103@{# 694}
OS Windows 10 OS Version 1909 (Build 18363.900)
  • Verified steps from PR description
  • Verified able to play media after Widevine is downloaded
  • Verified request is via http://redirector.gvt1.com
    image
Request JSON
{
  "request": {
    "@os": "win",
    "@updater": "",
    "acceptformat": "crx2,crx3",
    "app": [
      {
        "appid": "oimompecagnajdejgnnjijobebaeigek",
        "event": [
          {
            "download_time_ms": 56269,
            "downloaded": 5205379,
            "downloader": "direct",
            "eventresult": 1,
            "eventtype": 14,
            "nextversion": "4.10.1610.0",
            "previousversion": "0.0.0.0",
            "total": 5205379,
            "url": "http://redirector.gvt1.com/edgedl/chromewebstore/L2Nocm9tZV9leHRlbnNpb24vYmxvYnMvNTQ2QUFXaFBmeHYtb0xmZjM3YU5MN0FwZw/4.10.1610.0_oimompecagnajdejgnnjijobebaeigek.crx"
          },
          {
            "eventresult": 1,
            "eventtype": 3,
            "nextfp": "1.3a7afafe965da76cea59344022f69dcb9eaaef382ce56eb1daa8d20f9ec80c16",
            "nextversion": "4.10.1610.0",
            "previousversion": "0.0.0.0"
          }
        ],
        "version": "4.10.1610.0"
      }
    ],
    "arch": "x64",
    "dedup": "cr",
    "hw": {
      "physmemory": 16
    },
    "lang": "",
    "nacl_arch": "x86-64",
    "os": {
      "arch": "x86_64",
      "platform": "Windows",
      "version": "10.0.18363.900"
    },
    "prodchannel": "stable",
    "prodversion": "83.1.12.31",
    "protocol": "3.1",
    "requestid": "{b76fc663-6970-45b7-b036-7faa9b3285a0}",
    "sessionid": "{f57142d3-f09d-42bd-acee-ae61417c1723}",
    "updaterchannel": "stable",
    "updaterversion": "83.1.12.31"
  }
}
Response JSON
{
  "response": {
    "server": "prod",
    "protocol": "3.1",
    "daystart": {
      "elapsed_seconds": 84987,
      "elapsed_days": 4915
    },
    "app": [
      {
        "appid": "oimompecagnajdejgnnjijobebaeigek",
        "status": "ok",
        "event": [
          {
            "status": "ok"
          },
          {
            "status": "ok"
          }
        ]
      }
    ]
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/run-network-audit Run network-audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Desktop] Do not proxy downloads for widevine
7 participants