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

Youtube video publisher is not added to auto-contribute when video ID contains an underscore #1277

Closed
jasonrsadler opened this issue Sep 26, 2018 · 1 comment · Fixed by brave/brave-core#522

Comments

@jasonrsadler
Copy link

jasonrsadler commented Sep 26, 2018

Description

When viewing a youtube video, an interval query is sent to get information about viewing times, channel, media ID, etc.

When adding to ledger, the publisher ID is split from the video ID with an underscore delimiter. However, IDs can contain an underscore and will cause 404s when trying to query for publisher.
If video ID begins with an underscore, we will have an empty mediaID, DCHECK will fail, and Brave will crash in debug mode.

Steps to Reproduce

  1. Open Brave and have Rewards enabled
  2. View video at https://www.youtube.com/watch?v=_FQJEzJ_cQw
    Result: Brave will fail DCHECK and crash in debug mode
  1. Open Brave and have Rewards enabled
  2. View video at https://www.youtube.com/watch?v=iwprDM-K_3w
    Result: Youtube frontend proxy returns 404s in terminal and publisher is not added to autocontribute

Expected result:

Publishers should be added to ledger. Brave should not crash.

Reproduces how often:

100% not added to autocontribute when video id contains an underscore.
100% crashes in debug when video id begins with underscore

Brave version (chrome://version info)

Brave 0.55.2 Chromium: 70.0.3538.22 (Developer Build) (64-bit)
Revision ac9418ba9c3bd7f6baaffa0b055dfe147e0f8364-refs/branch-heads/3538@{#468}
OS Mac OS X

Reproducible on current release:

Untested

Website problems only:

  • Does the issue resolve itself when disabling Brave Shields? No
  • Is the issue reproducible on the latest version of Chrome? NA

Additional Information

Video IDs will need to be split properly with [-_0-9A-Za-z]{11} as a mandatory constraint as the first part.

@jasonrsadler jasonrsadler added this to the Releasable builds 0.55.x milestone Sep 26, 2018
@jasonrsadler jasonrsadler self-assigned this Sep 26, 2018
@jasonrsadler jasonrsadler changed the title Youtube video publisher is not added to auto-contribute is video ID contains an underscore Youtube video publisher is not added to auto-contribute when video ID contains an underscore Sep 26, 2018
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Sep 26, 2018
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Sep 26, 2018
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Sep 26, 2018
linting

Correcting build (initialization order)
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Sep 26, 2018
linting

Correcting build (initialization order)

linting
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Sep 26, 2018
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Sep 26, 2018
linting

Correcting build (initialization order)

linting
@NejcZdovc NejcZdovc reopened this Oct 2, 2018
@btlechowski
Copy link

btlechowski commented Oct 9, 2018

Verification passed on

Brave 0.55.11 Chromium: 70.0.3538.35 (Official Build) beta (64-bit)
Revision 28dcb499844fa40c28d5f62e337876cb936f79f5-refs/branch-heads/3538@{#678}
OS Windows 7

Checked all scenarios from brave/brave-core#522
Checked STR from this issue.

Verification Passed on

Brave 0.55.11 Chromium: 70.0.3538.35 (Official Build) beta (64-bit)
Revision 28dcb499844fa40c28d5f62e337876cb936f79f5-refs/branch-heads/3538@{#678}
OS Linux

Verified passed with

Brave 0.55.12 Chromium: 70.0.3538.45 (Official Build) beta(64-bit)
Revision cbdc32e4334458954e9def214d7e5fa1ca1960eb-refs/branch-heads/3538@{#830}
OS Mac OS X

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment