Skip to content

Conversation

Stypox
Copy link
Member

@Stypox Stypox commented Jul 6, 2025

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

This PR resolves a security-related TODO from #12044: making sure only packages with the right permissions can access NewPipe's MediaBrowser through onGetRoot(). The PackageValidator file is taken from https://github.com/android/uamp/blob/329a21b63c247e9bd35f6858d4fc0e448fa38603/common/src/main/java/com/example/android/uamp/media/PackageValidator.kt, as suggested in the Android docs here.

Furthermore, this PR fixes #12400, which was caused by an invalid media resumption notification being created by the system after closing the player. The notification was being created because the system was querying onGetRoot() to get information about resumption, and our code returned a BrowserRoot instead of returning null, indicating that we supported media resumption as described here. Now the code only returns a BrowserRoot if rootHints?.getBoolean(EXTRA_RECENT) != true, since the EXTRA_RECENT hint is what identifies a media resumption request. This is also how UAMP and the Android docs identify it.

I tested the fix for #12400 on an Android 10 emulator, and while I could reproduce before this commit, now I can't anymore. I also tested the Android Auto implementation with android-head-unit and it still works (i.e. the Android Auto app passes the PackageValidator tests).

For reference, VLC's implementation is quite different (no validation checks, package check to identify Android Auto), though I think it's buggy as I get ghost notifications with VLC too: https://github.com/videolan/vlc-android/blob/13567cd0ddc3ae93b853229a532694eee25a526d/application/vlc-android/src/org/videolan/vlc/PlaybackService.kt#L1909

I've learned something about MediaBrowser tonight ;-)

Fixes the following issue(s)

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.

Due diligence

Fixes TeamNewPipe#12400, see there for explanation. Citing from there:

So apparently the problem is onGetRoot always returning a BrowserRoot instance. Making it return null solved the issue (but again, breaks Android Auto compatibility). It turns out (see https://stackoverflow.com/q/63818988/) that onGetRoot is also used for media resumption https://developer.android.com/media/implement/surfaces/mobile#mediabrowserservice_implementation, which causes a new notification to pop up (in this case a useless notification because our onGetRoot does not return something that can be used for resumption). So what needs to be done is to check if rootHints?.getBoolean(EXTRA_RECENT) == true and if that's the case not return anything (as EXTRA_RECENT is used by the system for resumption).

The PackageValidator file is taken from https://github.com/android/uamp/blob/329a21b63c247e9bd35f6858d4fc0e448fa38603/common/src/main/java/com/example/android/uamp/media/PackageValidator.kt .
@github-actions github-actions bot added the size/large PRs with less than 750 changed lines label Jul 6, 2025
@Stypox Stypox added bug Issue is related to a bug ASAP Issue needs to be fixed as soon as possible player Issues related to any player (main, popup and background) player notification Anything to do with the MediaStyle notification labels Jul 6, 2025
@Stypox Stypox added this to v0.28.x Jul 6, 2025
@Stypox Stypox moved this to In Progress in v0.28.x Jul 6, 2025
@absurdlylongusername
Copy link
Member

Unable to reproduce this bug on Android 10 emulator, but I can on Android 11 emulator. I can confirm this fix works for Android 11.

@Stypox
Copy link
Member Author

Stypox commented Jul 14, 2025

Merging for now, we can always revert it if people report issues on Android Auto.

@Stypox Stypox merged commit 4ddc064 into TeamNewPipe:dev Jul 14, 2025
8 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in v0.28.x Jul 14, 2025
@Stypox Stypox deleted the fix-ghost-notifications branch July 16, 2025 13:53
@Stypox Stypox mentioned this pull request Jul 17, 2025
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ASAP Issue needs to be fixed as soon as possible bug Issue is related to a bug player notification Anything to do with the MediaStyle notification player Issues related to any player (main, popup and background) size/large PRs with less than 750 changed lines

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Nightly] Closed videos are stuck in notifications

3 participants