Skip to content
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

pretty printed config.xml breaks plugin due to a regex #139

Closed
santicalvo opened this issue Sep 10, 2019 · 1 comment
Closed

pretty printed config.xml breaks plugin due to a regex #139

santicalvo opened this issue Sep 10, 2019 · 1 comment
Labels
bug Something isn't working properly build issue An issue related to build process legacy Related to inherited code/functionality

Comments

@santicalvo
Copy link

santicalvo commented Sep 10, 2019

Bug report edited

I am not 100% sure this is a bug or just a feature request. Is just that it took me a lot to debug an issue.

I have some hooks and they rewrite config.xml and pretty print it...
If we pretty print the config.xml file and set the "name" this way:

<name>
    my-app-name
</name>

The app breaks after adding platform with the error:

Cannot read property 'includes' of null

There is a quick solution to avoid this: use a library to indent that can indent "inline" elements.
In my case, I did the pretty printing this way, using the xml-formatter library:

const xml = format(configXml.write({encoding:'utf-8', xml_declaration:true}));

With this library, I could correct it is as easyly as this:

const xml = format(configXml.write({encoding:'utf-8', xml_declaration:true}), {collapseContent:true});

However, I leave this issue open for reference. If you think it is not an issue or should be remove, please do it!

Steps to reproduce:
You can test it this way, create an empty app and edit the config.xml file to pretty print it as mentioned before.

$ cordova create hello com.hello.world hello
Creating a new cordova project.
user@host$ cd hello
user@host$ vim config.xml
user@host$ cordova plugin add cordova-plugin-firebasex
Adding cordova-plugin-firebasex to package.json
user@host$ cordova platform add android
Using cordova-fetch for cordova-android@^8.0.0
Adding android project...
Creating Cordova project for the Android platform:
	Path: platforms/android
	Package: com.hello.world
	Name: hello
	Activity: MainActivity
	Android target: android-28
Subproject Path: CordovaLib
Subproject Path: app
Android project created with cordova-android@8.0.0
Installing "cordova-plugin-firebasex" for android
Installing "cordova-plugin-androidx" for android
Installing "cordova-plugin-androidx-adapter" for android
Subproject Path: CordovaLib
Subproject Path: app
Plugin 'cordova-plugin-whitelist' found in config.xml... Migrating it to package.json
Discovered saved plugin "cordova-plugin-whitelist". Adding it to the project
Installing "cordova-plugin-whitelist" for android
Adding cordova-plugin-whitelist to package.json
cordova-plugin-androidx: Updated gradle.properties to enable AndroidX
cordova-plugin-androidx-adapter: Processed 8 Java source files in 230ms
Cannot read property 'includes' of null
$

I debug and see in the file project-root/plugins/cordova-plugin-firebasex/scripts/after_prepare.js, line 17, that we are searching for the string <name>blabla</name>

var config = fs.readFileSync('config.xml').toString();
var name = utilities.getValue(config, 'name');
if (name.includes('&amp;')) {
    name = name.replace(/&amp;/g, '&');
}

The utilities.getValue(config, 'name') calls the following function on utilities.js, wich returns null:

getValue: function (config, name) {
    var value = config.match(new RegExp('<' + name + '(.*?)>(.*?)</' + name + '>', 'i'));
    if (value && value[2]) {
      return value[2]
    } else {
      return null
    }
  }

I see the regex expression doesn't work multiline. I am not very good at regex and I am short of time now.... If I find a second I will try to come up with a regex that supports it...

@dpa99c
Copy link
Owner

dpa99c commented Sep 12, 2019

Regex parsing of XML is pretty fragile - this is code inherited from cordova-plugin-firebase.
This should be replaced with an XML parser to read the name from config.xml which will resolve this issue.

@dpa99c dpa99c added bug Something isn't working properly build issue An issue related to build process legacy Related to inherited code/functionality TODO Something needs doing labels Sep 12, 2019
@dpa99c dpa99c added ready for release Something has been implemented and is awaiting release to npm and removed TODO Something needs doing labels Nov 7, 2019
@dpa99c dpa99c closed this as completed in 71c896f Nov 9, 2019
@dpa99c dpa99c removed the ready for release Something has been implemented and is awaiting release to npm label Nov 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working properly build issue An issue related to build process legacy Related to inherited code/functionality
Projects
None yet
Development

No branches or pull requests

2 participants