Project

Profile

Help

Bug #6202

closed

Invalid attribute names in configuration files are ignored

Added by Michael Kay 7 months ago. Updated 5 months ago.

Status:
Closed
Priority:
Low
Category:
Diagnostics
Sprint/Milestone:
-
Start date:
2023-09-19
Due date:
% Done:

100%

Estimated time:
Legacy ID:
Applies to branch:
12, trunk
Fix Committed on Branch:
12, trunk
Fixed in Maintenance Release:
Platforms:
.NET, Java

Description

Unless you validate your configuration file against the published schema (which most users are unlikely to do), incorrectly-spelled attribute names (in the global section and other similar sections) will go undetected.

The code iterates over the valid attribute names and for each one tests to see if the relevant attribute exists. This simply ignores any invalid attributes.

Unfortunately configuration files are allowed in Saxon-HE so we can't simply run the file through schema validation.

A solution would be for ConfigurationReader.applyProperty() to delete the attribute from the Properties object, and then at the end if the Properties object is non-empty, it indicates that unrecognised attributes were present in the file.

Actions #1

Updated by Michael Kay 7 months ago

  • Category changed from Configuration to Diagnostics
  • Status changed from New to Resolved
  • Applies to branch 12, trunk added
  • Fix Committed on Branch 12, trunk added
  • Platforms .NET, Java added

Fixed for 12.x and 13.x. Not fixing this for earlier releases.

Actions #2

Updated by Michael Kay 6 months ago

The fix was incorrect. Setting a property to null fails with a NullPointerException. Should use Properties.remove().

Actions #3

Updated by Debbie Lockett 6 months ago

  • Status changed from Resolved to In Progress
  • Assignee changed from Michael Kay to Debbie Lockett

Reopening. The changes are causing a compile time failure for SaxonJS nodejs unit test iss5925:

"Unrecognized configuration property global/@xpathVersionForXslt"

We need to check that the ConfigurationReader really checks for all properties that are allowed (as defined in FeatureKeys.xml).

Actions #4

Updated by Debbie Lockett 6 months ago

I think that we should be generating a sample configuration file from FeatureKeys.xml, as well as the java classes, schema, and documentation; and testing this. Note that validating against the schema may be insufficient; it seems that we also need to check it can be read by the ConfgurationReader.

Currently the sample configuration files that we distribute in the saxon-resources are just samples - they do not contain all properties. The run_config_samples test just checks the samples are valid against the generated schema. Similarly the example configuration file included in the documentation (https://www.saxonica.com/documentation12/index.html#!configuration/configuration-file) is only manually edited, and may be incomplete.

Actions #5

Updated by Michael Kay 6 months ago

I've checked the list of global/xsl/xquery/xsd properties read by ConfigurationReader.java against the list in samples/config/config-raw.xsd and found several omissions -- thresholdForFunctionInlining, xpathVersionForXsd, xpathVersionForXslt , multipleModuleImports, allowUnresolvedSchemaComponents -- which I have fixed.

There seems to be some inconsistency in how we handle properties that no longer have any meaning. These should probably result in a warning. I've changed ConfigurationReader so it issues a warning for certain properties that are still in the schema but no longer have any effect (for example, those related to bytecode generation).

It would definitely help if both the schema and the ConfigurationReader were driven directly from the FeatureKeys database.

Actions #6

Updated by Norm Tovey-Walsh 6 months ago

  • Assignee changed from Debbie Lockett to Norm Tovey-Walsh

Yes. I'll see about setting that up.

Actions #7

Updated by Debbie Lockett 6 months ago

Note that we do check that config-raw.xsd is consistent with FeatureKeys.xml: the stylesheet tools/FeatureKeys/FeatureKeysToSchema.xsl which is used to generate config.xsd (provided in the saxon-resources) from config-raw.xsd checks the properties "The stylesheet also checks the schema for consistency with FeatureKeys.xml; the set of attributes defined in both should agree".

But yes, currently there are no checks for consistency with ConfigurationReader.

Actions #8

Updated by Debbie Lockett 5 months ago

I have updated the samples testing gradle task run_config_samples to add tests which actually use the sample configuration files (as well as the existing tests that they are valid against the schema).

These tests (for the sample configuration files config.xml and config-HE.xml) currently fail. It looks like the ConfigurationReader is still throwing an error, as well as producing warnings for obsolete options. Testing config.xml:

Warning 
   Obsolete configuration property global/@lazyConstructionMode
Warning 
   Unrecognized configuration property global/@lazyConstructionMode
Warning 
   Invalid configuration property xsd/@schemaUriResolver. Supplied value:
  'com.saxonica.ee.config.StandardSchemaResolver'; required: 'Cannot use
  com.saxonica.ee.config.StandardSchemaResolver as the value of
  http://saxon.sf.net/feature/schemaURIResolverClass. Failed to instantiate class
  com.saxonica.ee.config.StandardSchemaResolver (it has no public zero-argument constructor)'
Warning 
   Unrecognized configuration property xsd/@schemaUriResolver
Unrecognized configuration property global/@lazyConstructionMode

Testing config-HE.xml:

Warning 
   Obsolete configuration property global/@lazyConstructionMode
Warning 
   Unrecognized configuration property global/@lazyConstructionMode
Warning 
   Obsolete configuration property xslt/@versionWarning
Warning 
   Unrecognized configuration property xslt/@versionWarning
Unrecognized configuration property global/@lazyConstructionMode

Also note that there is no indication in the FeatureKeys.xml source that lazyConstructionMode is now obsolete; so this is currently not documented.

Actions #9

Updated by Debbie Lockett 5 months ago

I've committed a fix in the ConfigurationReader so that obsolete options just produce warnings and not an error.

I also discovered another missing option - global/@regexBacktrackingLimit - which is now added.

(I'm currently working on generating configuration files from the FeatureKeys source, to test for complete consistency.)

Actions #10

Updated by Debbie Lockett 5 months ago

I've committed some further work:

  • PREFER_JAXP_PARSER was fully removed from FeatureKeys for 11.0 (rather than just being labelled as obsolete); I have removed some remaining instances of global/@preferJaxpParser in sample configuration files and in ConfigurationReader.
  • Otherwise, obsolete properties are labelled as OBSOLETE_PROPERTY in ConfigurationReader. I have added mark up to FeatureKeys (i.e. the <obsolete> element) to label features as obsolete. I have aligned these so that they are now consistent: updating ConfigurationReader to add that global/@displayByteCode, global/@maxCompiledClasses, global/@thresholdForHotspotByteCode, and xslt/@messageEmitter are obsolete; and updating the documentation in FeatureKeys for global/@lazyConstructionMode).
  • In tools/featureKeys I've added a new stylesheet "FeatureKeysToConfigFile.xsl" which can be used to generate sample configuration files, referencing FeatureKeys.xml and AdditionalFeatures.xml to include all possible options. The stylesheet should be run against the source document config-base.xml, which includes sample values for options to ensure that generated configuration files are valid, and "usable". Stylesheet parameters can be used to configure the generated output: $edition, $platform, $include-obsolete - to include relevant options only.

It would be good to add testing for generated configuration files which will help us to keep FeatureKeys and ConfigurationReader aligned; but I have not yet configured this.

Actions #11

Updated by Michael Kay 5 months ago

There were JUnit test failures in ConfigTest caused by invalid options in the testdata/config.xml file; I have corrected these errors and the corresponding tests now pass. The invalid options were

stripWhitespace - should be stripSpace
collectionUriResolver - should be collectionFinder
preEvaluateDocFunction - should be preEvaluateDoc
Actions #12

Updated by Debbie Lockett 5 months ago

Updates committed to FeatureKeysToConfigFile.xsl, and testing for generated configuration files now added in run-samples.gradle - see the run_config_tests task (which is added as a dependency for run_config_samples, so we will at least run this testing whenever we run these samples tests).

Actions #13

Updated by Debbie Lockett 5 months ago

  • Status changed from In Progress to Resolved

I believe all issues have been resolved now.

Actions #14

Updated by O'Neil Delpratt 5 months ago

  • Status changed from Resolved to Closed
  • % Done changed from 0 to 100
  • Fixed in Maintenance Release 12.4 added

Bug fix applied in the Saxon 12.4 maintenance release

Please register to edit this issue

Also available in: Atom PDF