Defensive guards against invalid code points in preferences.

[mbien]

The code point '\u0000' breaks the Preferences storage as demonstrated in https://bugs.openjdk.org/browse/JDK-8068373, as preventive measure it can't be used in keys since https://bugs.openjdk.org/browse/JDK-8084823.

There are many reports of windows users running into seemingly unrelated issues, but closer investigation showed that they all had at some point "invalid code point" exceptions in the log.

this usually happens in a code pattern like this:

for (String k : pref.keys()) { // contains an invalid key
    ... = pref.get(key, null);  // throws IAE
}

It is not clear what causes the corruption (and why it seems to only occur on windows), this PR adds some defensive code to cover some common cases, logs it and makes sure no corrupted properties files are imported from old configs on first launch.

the jackpot rule:

$pref.keys() :: $pref instanceof java.util.prefs.Preferences;;

had 70 hits - this PR covers only common cases + adds the import filter.

org.apache.tools.ant.module.bridge.impl.BridgeImpl$PostRun
java.lang.IllegalArgumentException: Key contains code point U+0000
	at java.prefs/java.util.prefs.AbstractPreferences.get(AbstractPreferences.java:296)
	at org.apache.tools.ant.module.api.IntrospectedInfo$3.load(IntrospectedInfo.java:681)
	at org.apache.tools.ant.module.AntSettings.getCustomDefs(AntSettings.java:119)
	at org.apache.tools.ant.module.bridge.impl.BridgeImpl$PostRun.run(BridgeImpl.java:443)
	at org.openide.util.RequestProcessor$Task.run(RequestProcessor.java:1418)
	at org.netbeans.modules.openide.util.GlobalLookup.execute(GlobalLookup.java:45)
	at org.openide.util.lookup.Lookups.executeWith(Lookups.java:287)

java.lang.IllegalArgumentException: Key contains code point U+0000
       at java.util.prefs.AbstractPreferences.get(AbstractPreferences.java:296)
       at java.util.prefs.AbstractPreferences.getBoolean(AbstractPreferences.java:531)
       at org.netbeans.core.windows.TopComponentTracker.load(TopComponentTracker.java:99)
       at org.netbeans.core.windows.PersistenceHandler.load(PersistenceHandler.java:126)
       at org.netbeans.core.windows.WindowSystemImpl.load(WindowSystemImpl.java:81)
       at org.netbeans.core.GuiRunLevel$InitWinSys.run(GuiRunLevel.java:229)

java.lang.IllegalArgumentException: Key contains code point U+0000
      at java.prefs/java.util.prefs.AbstractPreferences.get(AbstractPreferences.java:296)
      at org.netbeans.modules.maven.queries.MavenFileOwnerQueryImpl.registerCoordinates(MavenFileOwnerQueryImpl.java:153)
      at org.netbeans.modules.maven.ProjectOpenedHookImpl.registerWithSubmodules(ProjectOpenedHookImpl.java:431)
      at org.netbeans.modules.maven.ProjectOpenedHookImpl.projectOpened(ProjectOpenedHookImpl.java:138)
      at org.netbeans.spi.project.ui.ProjectOpenedHook$1.projectOpened(ProjectOpenedHook.java:60)
  [catch] at org.netbeans.modules.project.ui.OpenProjectList.notifyOpened(OpenProjectList.java:1273)
      at org.netbeans.modules.project.ui.OpenProjectList.doOpenProject(OpenProjectList.java:1354)
      at org.netbeans.modules.project.ui.OpenProjectList.open(OpenProjectList.java:798)
      at org.netbeans.modules.project.ui.OpenProjectList$6.run(OpenProjectList.java:650)
      at org.openide.util.RequestProcessor$Task.run(RequestProcessor.java:1418)
      at org.netbeans.modules.openide.util.GlobalLookup.execute(GlobalLookup.java:45)
      at org.openide.util.lookup.Lookups.executeWith(Lookups.java:278)
      at org.openide.util.RequestProcessor$Processor.run(RequestProcessor.java:2033)

closes #6076
closes #5147
closes #5633
closes #5634
https://bz.apache.org/netbeans/show_bug.cgi?id=271652
https://issues.apache.org/jira/browse/NETBEANS-3284

[neilcsmith-net]

Generally a good move. Thanks for picking up!

On the protecting on import side -

• there are .properties files outside of config/Preferences. Are we 100% sure there is no valid reason for \u0000 in them? Or need to filter by preferences root?
• does the import protection also need to be considered in - https://github.com/apache/netbeans/blob/master/platform/options.api/src/org/netbeans/modules/options/export/OptionsExportModel.java#L293 ? (yes, I love we have two code paths with somewhat different implementations of similar things!)

[mbien]

there are .properties files outside of config/Preferences. Are we 100% sure there is no valid reason for \u0000 in them? Or need to filter by preferences root?

I scanned my config folder which had NB configs reaching back to NB 5. And none of them had this code point in properties files. #6076 (comment)

does the import protection also need to be considered in .. (yes, I love we have two code paths with somewhat different implementations of similar things!)

oh boy. No I have not. Going to take a look.

OptionsExportModel isn't closing a single ZipFile instance...

[mbien]

@neilcsmith-net good that you pointed me to OptionsExportModel, since it left several streams open, and all ZipFiles.

I never used the zip export feature before. Lets test this a bit before merging.

edit: tested config export/import and also config migration, everything worked fine I believe. Corrupted properties were also detected and successfully ignored.

forgot to squash, will force push

[matthiasblaesing]

Not sure about the \0 in properties, but the changes look sane and code is cleaner now.

[mbien]

thanks for the reviews and sorry for the delay.

I am also not sure about the null code point as property value, but https://bugs.openjdk.org/browse/JDK-8068373 for example describes it being illegal in XML files. Given that a) there was no such value in any property in my config backups and b) properties could end up in xml at some point which makes it probably safer to filter them out during config migration.

-> merging