Skip to content

[camerax]: Read External Storage defined in another plugin gets rejected from mergedManifest #156198

Closed
flutter/packages
#7826

Description

@rohansohonee1

Steps to reproduce

  1. Build the android app from the below code sample.
  2. Inspect the manifest merger logs at build/outputs/logs/manifest-merger-debug-report.txt.

Expected results

The permission should not get REJECTED because it is a required permission for the file_picker plugin.

Actual results

The permission is REJECTED.

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

For reference flutter/packages#4700 (comment)

Code sample

Code sample
dependencies:
  flutter:
    sdk: flutter
  flutter_keyboard_visibility: 6.0.0
  shared_preferences: 2.2.3
  image_picker: 1.1.2
  image_cropper: 5.0.1
  flutter_cache_manager: 3.4.1
  device_info_plus: 10.1.2
  share_plus: 10.0.0
  connectivity_plus: 6.0.5
  flutter_udid: 3.0.0
  flutter_local_notifications: 17.2.2
  permission_handler: 11.3.1
  in_app_purchase: 3.2.0
  package_info_plus: 8.0.2
  mobile_scanner: 3.0.0-beta.1
  in_app_review: 2.0.9
  open_mail_app: 0.4.5
  webview_flutter: 4.8.0
  facebook_app_events: 0.19.3
  app_settings: 5.1.1
  camera: 0.11.0
  file_picker: 8.0.5

dependency_overrides:
  mobile_scanner:
    git:
      url: https://github.com/agendaboa/mobile_scanner
      ref: e6bc28f66f6d266177b80daf898acabae980283b
import 'package:file_picker/file_picker.dart';
import 'package:flutter/material.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        // This is the theme of your application.
        //
        // TRY THIS: Try running your application with "flutter run". You'll see
        // the application has a purple toolbar. Then, without quitting the app,
        // try changing the seedColor in the colorScheme below to Colors.green
        // and then invoke "hot reload" (save your changes or press the "hot
        // reload" button in a Flutter-supported IDE, or press "r" if you used
        // the command line to start the app).
        //
        // Notice that the counter didn't reset back to zero; the application
        // state is not lost during the reload. To reset the state, use hot
        // restart instead.
        //
        // This works for code too, not just values: Most code changes can be
        // tested with just a hot reload.
        colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
        useMaterial3: true,
      ),
      home: const MyHomePage(title: 'Flutter Demo Home Page'),
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({super.key, required this.title});

  // This widget is the home page of your application. It is stateful, meaning
  // that it has a State object (defined below) that contains fields that affect
  // how it looks.

  // This class is the configuration for the state. It holds the values (in this
  // case the title) provided by the parent (in this case the App widget) and
  // used by the build method of the State. Fields in a Widget subclass are
  // always marked "final".

  final String title;

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  int _counter = 0;

  void _incrementCounter() async {
    // pick the .p12 or .pfx extension file
    FilePickerResult? result = await FilePicker.platform.pickFiles(
      type: FileType.custom,
      allowedExtensions: ['p12', 'pfx'],
      withData: true,
    );
    print(result);
    setState(() {
      // This call to setState tells the Flutter framework that something has
      // changed in this State, which causes it to rerun the build method below
      // so that the display can reflect the updated values. If we changed
      // _counter without calling setState(), then the build method would not be
      // called again, and so nothing would appear to happen.
      _counter++;
    });
  }

  @override
  Widget build(BuildContext context) {
    // This method is rerun every time setState is called, for instance as done
    // by the _incrementCounter method above.
    //
    // The Flutter framework has been optimized to make rerunning build methods
    // fast, so that you can just rebuild anything that needs updating rather
    // than having to individually change instances of widgets.
    return Scaffold(
      appBar: AppBar(
        // TRY THIS: Try changing the color here to a specific color (to
        // Colors.amber, perhaps?) and trigger a hot reload to see the AppBar
        // change color while the other colors stay the same.
        backgroundColor: Theme.of(context).colorScheme.inversePrimary,
        // Here we take the value from the MyHomePage object that was created by
        // the App.build method, and use it to set our appbar title.
        title: Text(widget.title),
      ),
      body: Center(
        // Center is a layout widget. It takes a single child and positions it
        // in the middle of the parent.
        child: Column(
          // Column is also a layout widget. It takes a list of children and
          // arranges them vertically. By default, it sizes itself to fit its
          // children horizontally, and tries to be as tall as its parent.
          //
          // Column has various properties to control how it sizes itself and
          // how it positions its children. Here we use mainAxisAlignment to
          // center the children vertically; the main axis here is the vertical
          // axis because Columns are vertical (the cross axis would be
          // horizontal).
          //
          // TRY THIS: Invoke "debug painting" (choose the "Toggle Debug Paint"
          // action in the IDE, or press "p" in the console), to see the
          // wireframe for each widget.
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            const Text(
              'You have pushed the button this many times:',
            ),
            Text(
              '$_counter',
              style: Theme.of(context).textTheme.headlineMedium,
            ),
          ],
        ),
      ),
      floatingActionButton: FloatingActionButton(
        onPressed: _incrementCounter,
        tooltip: 'Increment',
        child: const Icon(Icons.add),
      ), // This trailing comma makes auto-formatting nicer for build methods.
    );
  }
}

Screenshots or Video

Screenshots / Video demonstration

[Upload media here]

Logs

manifest-merger-debug-report.txt

Flutter Doctor output

Doctor output
[✓] Flutter (Channel stable, 3.19.6, on macOS 12.7.6 21H1320 darwin-x64, locale en-US)
    • Flutter version 3.19.6 on channel stable at /Users/appleapple/fvm/versions/3.19.6
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 54e66469a9 (6 months ago), 2024-04-17 13:08:03 -0700
    • Engine revision c4cd48e186
    • Dart version 3.3.4
    • DevTools version 2.31.1

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
    • Android SDK at /Users/appleapple/Library/Android/sdk
    • Platform android-34, build-tools 34.0.0
    • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.11+0-17.0.11b1207.24-11852314)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.2)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14C18
    • CocoaPods version 1.15.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2024.1)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.11+0-17.0.11b1207.24-11852314)

[✓] VS Code (version 1.93.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.98.0

[✓] Connected device (3 available)            
    • sdk gphone x86 64 (mobile) • emulator-5554 • android-x64    • Android 11 (API 30) (emulator)
    • macOS (desktop)            • macos         • darwin-x64     • macOS 12.7.6 21H1320 darwin-x64
    • Chrome (web)               • chrome        • web-javascript • Google Chrome 129.0.6668.90

[✓] Network resources
    • All expected network resources are available.

• No issues found!

Activity

  1. rohansohonee1 commented on Oct 4, 2024

    @rohansohonee1
    Author
    1. Maybe the plugin should not perform the tools:node="remove".
    2. Or the documentation must mention that Read external storage permission will be removed from the merged manifest.
  2. added
    in triagePresently being triaged by the triage team
    on Oct 4, 2024
  3. huycozy commented on Oct 4, 2024

    @huycozy
    Member

    I can reproduce this issue by following steps

    1. Create new Flutter project
    2. Add two plugins:
    camera_android_camerax: ^0.6.9+1
    file_picker: ^8.1.2
    1. Build apk:
    flutter build apk
    1. Check the REJECTED permission on build/app/outputs/logs/manifest-merger-release-report.txt as mentioned above

    Reproduced this on Flutter stable 3.24.3 and master 3.26.0-1.0.pre.353

  4. added
    platform-androidAndroid applications specifically
    packageflutter/packages repository. See also p: labels.
    has reproducible stepsThe issue has been confirmed reproducible and is ready to work on
    a: pluginsSupport for writing, building, and running plugin packages
    team-androidOwned by Android platform team
    fyi-ecosystemFor the attention of Ecosystem team
    and removed
    in triagePresently being triaged by the triage team
    on Oct 4, 2024
  5. rohansohonee1 commented on Oct 4, 2024

    @rohansohonee1
    Author

    @huycozy
    Can you suggest a temporary/immediate solution for this issue?

  6. huycozy commented on Oct 4, 2024

    @huycozy
    Member

    I tried explicitly adding READ_EXTERNAL_STORAGE permission to project's AndroidManifest file with tools:node="merge" or tools:node="replace" but didn't work.

    Another idea that you can try is to fork camera_android_camerax and modify permission for your use temporarily (add it back?)

  7. 1 remaining item

  8. rohansohonee1 commented on Oct 4, 2024

    @rohansohonee1
    Author
  9. huycozy commented on Oct 4, 2024

    @huycozy
    Member

    Yeah, app level should have higher priority than libraries.

    I have tested by excluding this merge rules tools:node="merge" or tools:node="replace".
    I declared it in the project's AndroidManifest file in the same manner as it has been declared in file_picker plugin:
    It is working for me.

    Adding permission like that or with tools:node="merge" or tools:node="replace" indeed works. Perhaps I misread or the build was cached on my report at #156198 (comment).

    You can confirm this by using Android Studio > Build > Analyze APK, selecting the output APK built from Flutter and checking the permission on the final AndroidManifest, it should be there.

  10. added
    P1High-priority issues at the top of the work list
    triaged-androidTriaged by Android platform team
    on Oct 4, 2024
  11. gmackall commented on Oct 4, 2024

    @gmackall
    Member

    From Android triage: @camsim99 , talk to @reidbaker

  12. camsim99 commented on Oct 8, 2024

    @camsim99
    Contributor

    Ok I'm understanding that https://github.com/flutter/packages/blob/c6377679226b5ee00c46154f3f72f1b1a4c17c03/packages/camera/camera_android_camerax/android/src/main/AndroidManifest.xml#L10 was not quite the right fix for READ_EXTERNAL_STORAGE being unnecessarily added. And based on the comments, sounds like ideally we could have the manifest of any other plugin take precedent over CameraX's, but based on https://developer.android.com/build/manage-manifests#merge_priorities it doesn't sound like this is possible since they are all libraries?

    Assuming it's not possible: FWIW, I'm not sure that needing to manually include READ_EXTERNAL_STORAGE in an app that does need it is worse than having to remove it from an app that does not need it, but maybe the recent changes to scoped storage (and the fact that it doesn't do anything on API 33+) make the consequences less severe for the latter case.

    @reidbaker I hear I should talk to you. Any insights?

  13. camsim99 commented on Oct 8, 2024

    @camsim99
    Contributor

    Talked offline with @reidbaker and decided to revert flutter/packages#4716 and add documentation on how to remove READ_EXTERNAL_STORAGE manually in your Flutter app.

    The docs that explain why WRITE_EXTERNAL_STORAGE (which may bring in READ_EXTERNAL_STORAGE by default in the merged Android manifest) is needed in the plugin is already linked in the README that my PR edits.

  14. added
    r: fixedIssue is closed as already fixed in a newer version
    on Oct 10, 2024
  15. github-actions commented on Oct 24, 2024

    @github-actions

    This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

  16. locked as resolved and limited conversation to collaborators on Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

P1High-priority issues at the top of the work lista: pluginsSupport for writing, building, and running plugin packagesfound in release: 3.24Found to occur in 3.24found in release: 3.26Found to occur in 3.26fyi-ecosystemFor the attention of Ecosystem teamhas reproducible stepsThe issue has been confirmed reproducible and is ready to work onp: cameraThe camera pluginpackageflutter/packages repository. See also p: labels.platform-androidAndroid applications specificallyr: fixedIssue is closed as already fixed in a newer versionteam-androidOwned by Android platform teamtriaged-androidTriaged by Android platform team

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions