-
-
Notifications
You must be signed in to change notification settings - Fork 937
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(share_plus): Set exported=false for BroadcastReceiver on Android #1613
Conversation
cc91935
to
5a425e0
Compare
@@ -14,7 +14,7 @@ | |||
</provider> | |||
<!-- This manifest declared broadcast receiver allows us to use an explicit | |||
Intent when creating a PendingItent to be informed of the user's choice --> | |||
<receiver android:name=".SharePlusPendingIntent" android:exported="true"> | |||
<receiver android:name=".SharePlusPendingIntent" android:exported="false"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know why it was exported ? Was it on purpose or just a miss / was created exported by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it that big deal? You can ping author, who initially added this receiver and ask. I guess, it it because the author isn't a mobile dev, but I don't see why we should care about it, considering that the issue is resolved.
…for security reasons
5a425e0
to
4b7c5a4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it was added during the Android v2 migration.
Also, it's true by default.
Are we sure users won't run into issues like this?
It was added while adding share result functionality. As to default or not - I have attached the docs in the PR description about why it is Ok to set to
Yes, I am sure. The issue you linked is about not having |
thanks @vbuberen for PR ! |
Description
Addressing an issue reported in #1608. After looking at the documentation and code saw that we indeed don't need to have receiver exposed as only catch the intent that we launch from the app that uses
share_plus
Docs on topic:
Tested the change - no issues with getting result of share in example app.
Related Issues
Closes #1608
Checklist
CHANGELOG.md
nor thepubspec.yaml
files.flutter analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?
!
in the title as explained in Conventional Commits).