Bug #6202
closedInvalid attribute names in configuration files are ignored
100%
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.
Updated by Michael Kay about 1 year 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.
Updated by Michael Kay about 1 year ago
The fix was incorrect. Setting a property to null fails with a NullPointerException. Should use Properties.remove().
Updated by Debbie Lockett about 1 year 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).
Updated by Debbie Lockett about 1 year 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.
Updated by Michael Kay about 1 year 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.
Updated by Norm Tovey-Walsh about 1 year ago
- Assignee changed from Debbie Lockett to Norm Tovey-Walsh
Yes. I'll see about setting that up.
Updated by Debbie Lockett about 1 year 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.
Updated by Debbie Lockett about 1 year 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.
Updated by Debbie Lockett about 1 year 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.)
Updated by Debbie Lockett about 1 year 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 inConfigurationReader
. - Otherwise, obsolete properties are labelled as
OBSOLETE_PROPERTY
inConfigurationReader
. 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: updatingConfigurationReader
to add thatglobal/@displayByteCode
,global/@maxCompiledClasses
,global/@thresholdForHotspotByteCode
, andxslt/@messageEmitter
are obsolete; and updating the documentation in FeatureKeys forglobal/@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.
Updated by Michael Kay about 1 year 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
Updated by Debbie Lockett about 1 year 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).
Updated by Debbie Lockett about 1 year ago
- Status changed from In Progress to Resolved
I believe all issues have been resolved now.
Updated by O'Neil Delpratt about 1 year 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