Failed to install '<plugin>': TypeError: Cannot read properties of undefined (reading 'id')

[GiovanniBattista]

Bug Report

Problem

I have created an internal cordova plugin at our company which adds the android:networkSecurityConfig attribute to the /manifest/application/ tag in AndroidManifest.xml via the following <edit-config:

<edit-config file="AndroidManifest.xml" mode="merge" target="/manifest/application">
   <application android:networkSecurityConfig="@xml/uq_network_security_config" />
</edit-config>

This works flawlessly if the plugin is added the first time. But if the plugin is removed and then re-added again an error occurrs.

What is expected to happen?

The plugin should be added again without any issues.

What does actually happen?

If the plugin is re-added, the following error is produced:

Failed to install 'uq-cordova-plugin-settings': TypeError: Cannot read properties of undefined (reading 'id')
    at registerConflict (D:\CordovaApp\node_modules\cordova-common\src\ConfigChanges\ConfigChanges.js:255:43)
    at D:\CordovaApp\node_modules\cordova-common\src\ConfigChanges\ConfigChanges.js:283:34
    at Array.forEach (<anonymous>)
    at PlatformMunger._is_conflicting (D:\CordovaApp\node_modules\cordova-common\src\ConfigChanges\ConfigChanges.js:259:21)
    at PlatformMunger.add_plugin_changes (D:\CordovaApp\node_modules\cordova-common\src\ConfigChanges\ConfigChanges.js:106:59)
    at D:\CordovaApp\node_modules\cordova-common\src\PluginManager.js:120:33
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
Cannot read properties of undefined (reading 'id')

Information

The problem is that, upon removal, the platforms/android/android.json looks like the following:

"AndroidManifest.xml": {
  "parents": {
    "/*": [
      {
        "xml": "<uses-permission android:name=\"android.permission.WRITE_EXTERNAL_STORAGE\" />",
        "count": 1
      },
      {
        "xml": "<queries><intent><action android:name=\"android.media.action.IMAGE_CAPTURE\" /></intent><intent><action android:name=\"android.intent.action.GET_CONTENT\" /></intent><intent><action android:name=\"android.intent.action.PICK\" /></intent><intent><action android:name=\"com.android.camera.action.CROP\" /><data android:mimeType=\"image/*\" android:scheme=\"content\" /></intent></queries>",
        "count": 1
      },
      ...
    ],
    "application": [],
    "/manifest/application": []
  }
}

Notice the empty array for /manifest/application.

The code in ConfigChanges.js registerConflict accesses the first entry in the array, which does not exist in this case. Therefore, the above error is being thrown.

With a slight adjustment to the code the above error could be avoided, but it leads to another error ("There was a conflict trying to modify attributes with") which is a different issue, I guess.

Command or Code

Create a new cordova project, create a new plugin directory with the <edit-config lines from the beginning of the issue.

cordova create hello com.example.hello HelloWorld
mkdir cordova-plugin-test
# add plugin.xml and package.json to cordova-plugin-test
cd hello
cordova platform add android@12.0.0
cordova plugin add ..\cordova-plugin-test
cordova plugin remove cordova-plugin-test
cordova plugin add ..\cordova-plugin-test
# => produces the error

A minimum viable plugin.xml looks like this:

<?xml version="1.0" encoding="UTF-8"?>
<plugin xmlns="http://apache.org/cordova/ns/plugins/1.0"
        id="cordova-plugin-test" version="0.0.1">
    <name>Test</name>
    <description>Cordova Device Test</description>
    <license>Apache 2.0</license>
    <keywords>cordova,test</keywords>

    <platform name="android">
      <edit-config file="AndroidManifest.xml" mode="merge" target="/manifest/application">
            <application android:networkSecurityConfig="@xml/uq_network_security_config" />
        </edit-config>
    </platform>
</plugin>

The package.json looks like this:

{
  "name": "cordova-plugin-test",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC"
}

Environment, Platform, Device

Environment: Windows 10 with PowerShell

Version information

Cordova-Cli: 12.0.0 (cordova-lib@12.0.1) Cordova Platform Android: 12.0.0 Windows 10 PowerShell 7

Checklist

• [x] I searched for existing GitHub issues
• [x] I updated all Cordova tooling to most recent version
• [x] I included all the necessary information above

[GiovanniBattista]

I may have spoken too soon. The guard condition in registerConflict only leads to another error further down the road:

Failed to install 'uq-cordova-plugin-settings': TypeError: Cannot read properties of undefined (reading 'charAt')
    at SAXParser.write (D:\CordovaApp\node_modules\sax\lib\sax.js:986:17)
    at XMLParser.feed (D:\CordovaApp\node_modules\elementtree\lib\parsers\sax.js:48:15)
    at ElementTree.parse (D:\CordovaApp\node_modules\elementtree\lib\elementtree.js:271:10)
    at exports.XML (D:\CordovaApp\node_modules\elementtree\lib\elementtree.js:606:13)
    at ConfigFile.prune_child (D:\CordovaApp\node_modules\cordova-common\src\ConfigChanges\ConfigFile.js:135:46)
    at PlatformMunger.apply_file_munge (D:\CordovaApp\node_modules\cordova-common\src\ConfigChanges\ConfigChanges.js:76:43)  
    at PlatformMunger._munge_helper (D:\CordovaApp\node_modules\cordova-common\src\ConfigChanges\ConfigChanges.js:173:18)    
    at PlatformMunger.add_plugin_changes (D:\CordovaApp\node_modules\cordova-common\src\ConfigChanges\ConfigChanges.js:115:18)
    at D:\CordovaApp\node_modules\cordova-common\src\PluginManager.js:120:33
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

It only works if you delete the two empty arrays in platforms/android/android.json:

[breautek]

The entire system that powers the <config-file> and <edit-config> doesn't work very well, and there is a lot of known issues surrounding it.

If you're able to determine a solution, even if it's not perfect then a PR would be appreciated...

Most issues starts to occur when there are more than 1 thing trying to modify the same node, or part of the node tree. Your issue doesn't appear to be that however given that it looks like you have a sample project with your sample plugin.

The issue appears to be more on the fact that on plugin uninstall, it isn't "cleaning" up after itself.

If I recall correctly, when you have a munge operation:

it will add a data point like this:

{
     "xml": "<uses-permission android:name=\"android.permission.WRITE_EXTERNAL_STORAGE\" />",
    "count": 1
},

and it's suppose to increment the count. This I believe is to support 2 plugins potentially trying to make hte same modification. e.g. if you add plugin A and plugin B that has the same edit-config, it should have a count of 2. If you remove plugin B, then the count I think is suppose to decrement. If the count reaches 0, it suppose to signal to remove the modification from the target file. At least I believe that is how it's suppose to work.

Perhaps on plugin add, it's incrementing more than once? If so that might the root cause of your issue.

[GiovanniBattista]

Sorry for the late response, I was on a short vacation.

Unfortunately, my proposed fix didn't work correctly and led to a subsequent error.

Yeah, it also seams to me, that the cleanup upon plugin removal doesn't work properly. In my case, the resulting data point looks like this:

"/manifest/application": [
{
  "xml": "<application android:networkSecurityConfig=\"@xml/uq_network_security_config\" />",
  "count": 1,
  "mode": "merge",
  "plugin": "uq-cordova-plugin-settings",
  "oldAttrib": {
    "android:hardwareAccelerated": "true",
    "android:icon": "@mipmap/ic_launcher",
    "android:label": "@string/app_name",
    "android:networkSecurityConfig": "@xml/uq_network_security_config",
    "android:supportsRtl": "true"
  }
},

Upon removal, it actually looks like this which produces the error when the plugin is re-added again.

"/manifest/application": []

If I remove this line, then the re-adding works.

It also works, if at least one other plugin adds something to "/manifest/application", i.e. if the array isn't empty when the plugin is re-added again.

[PrasannaKumarChalla]

I am having the same issue, is there any update or workaroung for this?.

[AhmedAyachi]

@GiovanniBattista @breautek I think the idea of using an xml string to identify the code added to AndroidManifest is not as good as it seems to be, bacause in some cases you can end up adding something twice, for ex in my case, adding a permission tag from one plugin and the same permission with a maxSdkVersion attribute with another, will result on adding the same permission twice which means build failure, I think using a definition object inside parents is better be like below:

{
    tag:"uses-permission",
    attributes:[
        {
            name:"android:name",
            value:"android.permission.READ_EXTERNAL_STORAGE",
            plugins:["plugin_1","plugin_2"],
        },
        {
            name:"android:maxSdkVersion",
            value:"30",
            plugins:["plugin_2"],
        }
    ]
}

and then if you remove a plugin , if the plugin exists in plugins and plugins.length>1, you keep the tag and just remove the plugin from plugins and if the plugins.length==1 then remove from AndroidManifest. I don't know it seems legit, I'm a bit busy these days, but i'll definitely give it a shot, I just wrote this comment to someone who can work on it sooner.

[breautek]

@GiovanniBattista @breautek I think the idea of using an xml string to identify the code added to AndroidManifest is not as good as it seems to be, bacause in some cases you can end up adding something twice, for ex in my case, adding a permission tag from one plugin and the same permission with a maxSdkVersion attribute with another, will result on adding the same permission twice which means build failure, I think using a definition object inside parents is better be like below:

{
    tag:"uses-permission",
    attributes:[
        {
            name:"android:name",
            value:"android.permission.READ_EXTERNAL_STORAGE",
            plugins:["plugin_1","plugin_2"],
        },
        {
            name:"android:maxSdkVersion",
            value:"30",
            plugins:["plugin_2"],
        }
    ]
}

and then if you remove a plugin , if the plugin exists in plugins and plugins.length>1, you keep the tag and just remove the plugin from plugins and if the plugins.length==1 then remove from AndroidManifest. I don't know it seems legit, I'm a bit busy these days, but i'll definitely give a shot, I just wrote this comment to someone who can work on it sooner.

I do agree to this and something similar to this is already being done. The codebase calls it munging. But it's not as simple as this structure. This in itself doesn't handle conflicts, and operations needs to support targeting nodes and attributes so they can be manipulated, or deleted, etc.

Personally I think what Cordova has needs to be rewritten, but I haven't found the time to provide a PoC either.

[AhmedAyachi]

A conflict is when two definition objects with the same name have two different values, and that will happen a lot when the project is using more and more plugins. By saying "This in itself doesn't handle conflicts", you mean: when plugin_2 overwrites the value provided by plugin_1 (with the --force flag let's say), when plugin_2 is removed, the value should get back to the one provided by plugin_1 ? I did not think of that and I totally agree, but what if we change the schema to this :

{
    tag:"uses-permission",
    attributes:[
        {
            name:"android:name",
            plugins:[
                {id:"plugin_1",value:"android.permission.READ_EXTERNAL_STORAGE"},
                {id:"plugin_2",value:"android.permission.READ_EXTERNAL_STORAGE"}
            ],
        },
        {
            name:"android:maxSdkVersion",
            plugins:[
                {id:"plugin_1",value:"29"},
                {id:"plugin_2",value:"30"},
            ],
        }
    ]
}

And in this case, the value of each attribute is determined by the last plugin item .value. Let's say plugin_2 was removed, the value will fallback automatically to the plugin_1's. I don't know, I'm just sharing ideas with you, so when I start contributing, I actually have something to work with/on.