Skip to content

[camerax] Explicitly remove READ_EXTERNAL_STORAGE permission #4700

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.

Sign up for GitHub

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

Jump to bottom
Closed
camsim99 wants to merge 30 commits into from
Closed

[camerax] Explicitly remove READ_EXTERNAL_STORAGE permission #4700

camsim99 wants to merge 30 commits into from

Conversation

@camsim99
Copy link
Contributor

@camsim99 camsim99 commented Aug 14, 2023 •
edited
Loading

Specifies that READ_EXTERNAL_STORAGE permission should not be implied from WRITE_EXTERNAL_STORAGE and thus removed from any merged manifests.

Fixes flutter/flutter#131116.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

camsim99 added 25 commits May 1, 2023 16:46
@camsim99
Merge remote-tracking branch 'upstream/main' into camx_occ
dec3d69
@camsim99
Merge remote-tracking branch 'upstream/main'
0e0333b
@camsim99
Merge remote-tracking branch 'upstream/main'
bd7ac99
@camsim99
Merge remote-tracking branch 'upstream/main'
5c3363b
@camsim99
Undo changes
fed9621
@camsim99
Merge remote-tracking branch 'upstream/main'
5aabe34
@camsim99
Merge remote-tracking branch 'upstream/main'
2b9a352
@camsim99
Merge remote-tracking branch 'upstream/main'
a1173da
@camsim99
Merge remote-tracking branch 'upstream/main'
cbc3d6b
@camsim99
Merge remote-tracking branch 'upstream/main'
cae5a4c
@camsim99
Merge remote-tracking branch 'upstream/main'
72283db
@camsim99
Merge remote-tracking branch 'upstream/main'
166a77c
@camsim99
Merge remote-tracking branch 'upstream/main'
399780e
@camsim99
Merge remote-tracking branch 'upstream/main'
8d5d0e7
@camsim99
Merge remote-tracking branch 'upstream/main'
084d960
@camsim99
Merge remote-tracking branch 'upstream/main'
d2a59ac
@camsim99
Merge remote-tracking branch 'upstream/main'
a1422bf
@camsim99
Merge remote-tracking branch 'upstream/main'
bdd87a6
@camsim99
Merge remote-tracking branch 'upstream/main'
137a28b
@camsim99
Merge remote-tracking branch 'upstream/main'
bc0db5a
@camsim99
Merge remote-tracking branch 'upstream/main'
d04b466
@camsim99
Merge remote-tracking branch 'upstream/main'
a9cfe87
@camsim99
Merge remote-tracking branch 'upstream/main'
a32def1
@camsim99
Merge remote-tracking branch 'upstream/main'
4785148
@camsim99
Remove permission
b13dd3c
@camsim99 camsim99 marked this pull request as ready for review August 14, 2023 22:32
@camsim99 camsim99 requested a review from a team August 14, 2023 22:32
@camsim99 camsim99 closed this Aug 15, 2023
@camsim99 camsim99 mentioned this pull request Aug 15, 2023
Merged
11 tasks
auto-submit bot pushed a commit that referenced this pull request Aug 15, 2023
@camsim99
[camerax] Explicitly remove READ_EXTERNAL_STORAGE permission (#4716)
b8573f6 
Lands #4700.
@rohansohonee1
Copy link

rohansohonee1 commented Oct 3, 2024

The following change causes this permission to be removed from any merged manifests. So was this the intended behavior?

Because this change gives an issue i.e if another plugin defines this permission of READ_EXTERNAL_STORAGE then it also gets removed from the merged manifest.

Eg: Using this plugin along with file_picker plugin.
file_picker requires this permission & it has been defined in it's manifest file as following:

<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" android:maxSdkVersion="32"/>

In the merged manifest logs it rejects the file_picker permission.

uses-permission#android.permission.READ_EXTERNAL_STORAGE
ADDED from [:camera_android_camerax] /Users/appleapple/StudioProjects/file_picker_api_30_issue/build/camera_android_camerax/intermediates/merged_manifest/debug/AndroidManifest.xml:17:5-19:31
REJECTED from [:file_picker] /Users/appleapple/StudioProjects/file_picker_api_30_issue/build/file_picker/intermediates/merged_manifest/debug/AndroidManifest.xml:9:5-11:38
	tools:node
		ADDED from [:camera_android_camerax] /Users/appleapple/StudioProjects/file_picker_api_30_issue/build/camera_android_camerax/intermediates/merged_manifest/debug/AndroidManifest.xml:19:9-28
	android:name
		ADDED from [:camera_android_camerax] /Users/appleapple/StudioProjects/file_picker_api_30_issue/build/camera_android_camerax/intermediates/merged_manifest/debug/AndroidManifest.xml:18:9-64

@camsim99
Please let me know if I am missing something?

@rohansohonee1 rohansohonee1 mentioned this pull request Oct 4, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@reidbaker reidbaker reidbaker approved these changes

@gmackall gmackall Awaiting requested review from gmackall

@bparrishMines bparrishMines Awaiting requested review from bparrishMines

@tarrinneal tarrinneal Awaiting requested review from tarrinneal

Assignees

No one assigned

Labels

p: camera platform-android

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[camera] camera plugin declares READ_EXTERNAL_STORAGE permission by default

3 participants

@camsim99 @rohansohonee1 @reidbaker