Community
Participate
Working Groups
Build Identifier: The MetaType service implementation will trim whitespace from the value being validated, which is passed as an argument to AttributeDefinition.validate(String). Whitespace is trimmed from the beginning and end of the value. If the value is a comma-delimited string, whitespace will be trimmed from the beginning and end of each token as well. The implementation behaves this way because it uses the ValuesTokenizer to process both default values specified within the XML as well as those passed directly to validate(). R4V42, section 105.6.1, table 105.1, page 135, states default values must have whitespace trimmed in the manner done by the implementation. It's not clear this should also apply to arguments passed to the validate() method. At least one issue with trimming whitespace from arguments passed to the validate() method occurs when using the CHAR attribute definition type. Any characters passed that represent whitespace as defined by trim() will not actually be validated since an empty string gets substituted in their place. Reproducible: Always
The same issue applies to the default value specified in the XML. There is no way to specify a space char as a default value. We may need a spec clarification here. It seems strange that we would ignore whitespace for default values but pay attention to whitespace for validation.
I opened an OSGi bug asking for clarification. See https://www.osgi.org/members/bugzilla/show_bug.cgi?id=1843.
The decision from CPEG was that whitespace stripping should remain for backwards compatibility. The spec should clarify the same whitespace stripping requirements for the "default" XML attribute value should also apply to arguments passed to the AttributeDefinition.validate(String) method. Any whitespace character may be escaped with a backslash if it has special meaning. Tom, please assign this to me.
Created attachment 191519 [details] Proposed Patch The main idea of this defect was support for the Char attribute definition type. The specification does not limit the Char type to non-whitespace characters only. However, the whitespace stripping rules imposed on implementations in terms of the 'default' XML attribute on the <AD> element and the AttributeDefinition.getDefaultValue() method made supporting whitespace characters impossible. CPEG did not want to remove the whitespace stripping rules out of concern for backwards compatibility. Consequently, it was decided that significant whitespace should be escaped with '\', like the backslash when not being used as an escape and comma when not being used as a delimiter. Furthermore, the same whitespace stripping and escape rules should apply to the AttributeDefinition.validate(String) method. This decision, of course, had ramifications extending beyond the resolution of the primary issue, which are reflected in the list below. This patch employs the following strategy. * (1) Significant whitespace at the beginning or end of the 'default' XML * attribute within the <AD> element must be escaped; otherwise, it will be * stripped. * (2) Significant whitespace at the beginning or end of each comma delimited * token within the 'default' attribute must be escaped; otherwise, it will * be stripped. * (3) Significant whitespace at the beginning or end of the argument passed to * the validate() method must be escaped; otherwise, it will be stripped. * (4) Significant whitespace at the beginning or end of the 'value' XML * attribute within the <Option> element must be escaped; otherwise, it * will be stripped. * (5) Escaping whitespace between two non-whitespace characters is permitted * but not required. In other words, whitespace between two non-whitespace * characters will never be stripped. * (6) An escape character occurring as the last character in the sequence will * be treated the same as insignificant whitespace. * (7) Escape characters will not be preserved in the results of * AttributeDefinition.getDefaultValue() or * AttributeDefinition.getOptionValues(). This has the nonintuitive * consequence that * AttributeDefinition.validate(AttributeDefinition.getDefaultValue()[i]) * and * AttributeDefinition.validate(AttributeDefinition.getOptionValues()[i]) * will not necessarily pass validation. However, preserving escape * characters in the result would probably be even more nonintuitive. * Moreover, this approach is not inconsistent with the requirement on * clients to escape certain characters (',', '\', and leading or trailing * significant whitespace) on other parameters to the validate() method. * Finally, the two operations referenced above are completely superfluous * since it must be the case that any declared default or option value is * valid. * (8) Null parameters passed to AttributeDefinition.validate(String) are * always invalid. * (9) Empty string parameters passed to AttributeDefinition.validate(String) * are always valid for the String type, even for cardinality zero * (required by the CT), unless restricted by options. * Furthermore, a sequence of comma-delimited empty strings is valid for * cardinality < -1 and cardinality > 1. For example, given a * cardinality of 5, AttributeDefinition.validate(",,,,") would pass. *(10) In order to be valid, a value must pass all of the following tests. * (a) The value must not be null. * (b) The value must be convertible into the attribute definition's * type, unless it's an empty string and cardinality != 0. * (c) The following relation must hold: min <= value <= max, if * either min or max is specified. * (d) If options were specified, the value must be equal to one of * them. * Note this approach means validation will always be present since the type * compatibility check can always be performed (i.e. the Equinox * implementation will never return null indicating no validation exists). *(11) An invalid option value will simply be ignored and not result in the * entire metadata being rejected (this is based on the previous behavior). *(12) Similarly, an invalid default value will simply be ignored. *(13) When specifying 'min' or 'max' values for type Char, escapes must not be * used. For example, <AD id="1" max="" min="	" type="Char"/> * not <AD id="1" max="\" min="\	" type="Char"/>. */
Note that I also snuck in two additional fixes. First, there's an API filter to suppress the warning for implementing LogService. Second, the Metatype bundle manifest has version ranges.
(In reply to comment #4) (10) (b) should read "The value must be convertible into the attribute definition's type" period. There are no exceptions.
Thanks for the hard work on this one John! I have released the patch with a minor change to the logging in ValueTokenizer. You had it logging a message as an ERROR when validating a type throw an unexpected exception. This can happen if you cannot convert the string into the proper type. The problem is that you then log the validate message as Warning again when setting the default or option value from the XML input so we will get duplicate messages. This is also problematic to the AD.validate() method because if invalid data is passed in then we will log an error and return a validate error string. I don't think the callers of the method would want an error logged in that case. I changed the log type to debug. For the record, the more I think about this issue the more I think the AD.validate() method never intended to take a ',' comma separated string as input. I think the original intent was to validate single value against some set of criteria (e.g. the min, max and perhaps type). But it is unclear to me that the original intention of AD.validate(String value) was to ever allow a client to pass a comma separated list of values to be validated. Otherwise it seems the method should take a String[] not a simple String. At any rate, our implementation has always attempted to split the validate input string into multiple comma separated list of values and I am not really interested in changing that now without getting a spec clarification stating that the input would never be a comma separated list. Again, thanks to John for putting up with my constant hounding about allowing validate to take a comma separated list ;-) What John done is likely the best we can do without completely removing the ability to parse the validate string input as a comma separated string.
