From 9d93cfd359db71e30cfaa5518ddcdfd30e7b2539 Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Mon, 28 Oct 2024 19:28:23 -0400 Subject: [PATCH] Improved error handling for data type conformance issues. Related to GSA/fedramp-automation#823. --- core/pom.xml | 6 +-- .../datatype/AbstractDataTypeAdapter.java | 54 +++++++++++++------ .../core/datatype/IDataTypeAdapter.java | 2 + .../io/json/MetaschemaJsonReader.java | 49 +++++++++++++---- .../databind/io/xml/MetaschemaXmlReader.java | 41 +++++++++++--- .../info/IFeatureScalarItemValueHandler.java | 24 +++++++++ 6 files changed, 138 insertions(+), 38 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index 41528b3aa..876449181 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -22,9 +22,9 @@ - ${scm.url}/tree/develop/core - HEAD - + ${scm.url}/tree/develop/core + HEAD + diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/datatype/AbstractDataTypeAdapter.java b/core/src/main/java/gov/nist/secauto/metaschema/core/datatype/AbstractDataTypeAdapter.java index 5e3c64079..138662802 100644 --- a/core/src/main/java/gov/nist/secauto/metaschema/core/datatype/AbstractDataTypeAdapter.java +++ b/core/src/main/java/gov/nist/secauto/metaschema/core/datatype/AbstractDataTypeAdapter.java @@ -10,7 +10,9 @@ import gov.nist.secauto.metaschema.core.metapath.function.InvalidValueForCastFunctionException; import gov.nist.secauto.metaschema.core.metapath.item.atomic.IAnyAtomicItem; +import gov.nist.secauto.metaschema.core.model.util.JsonUtil; import gov.nist.secauto.metaschema.core.model.util.XmlEventUtil; +import gov.nist.secauto.metaschema.core.util.ObjectUtils; import org.codehaus.stax2.XMLEventReader2; import org.codehaus.stax2.XMLStreamWriter2; @@ -39,7 +41,10 @@ */ public abstract class AbstractDataTypeAdapter implements IDataTypeAdapter { - public static final String DEFAULT_JSON_FIELD_NAME = "STRVALUE"; + /** + * The default JSON property name to use for a field value. + */ + public static final String DEFAULT_JSON_FIELD_VALUE_NAME = "STRVALUE"; @NonNull private final Class clazz; @@ -77,7 +82,7 @@ public boolean canHandleQName(QName nextQName) { @Override public String getDefaultJsonValueKey() { - return DEFAULT_JSON_FIELD_NAME; + return DEFAULT_JSON_FIELD_VALUE_NAME; } @Override @@ -93,24 +98,31 @@ public boolean isXmlMixed() { @Override public TYPE parse(XMLEventReader2 eventReader) throws IOException { StringBuilder builder = new StringBuilder(); + XMLEvent nextEvent; try { - XMLEvent nextEvent; while (!(nextEvent = eventReader.peek()).isEndElement()) { - if (nextEvent.isCharacters()) { - Characters characters = nextEvent.asCharacters(); - builder.append(characters.getData()); - // advance past current event - eventReader.nextEvent(); - } else { + if (!nextEvent.isCharacters()) { throw new IOException(String.format("Invalid content '%s' at %s", XmlEventUtil.toString(nextEvent), XmlEventUtil.toString(nextEvent.getLocation()))); } + Characters characters = nextEvent.asCharacters(); + builder.append(characters.getData()); + // advance past current event + eventReader.nextEvent(); } // trim leading and trailing whitespace - @SuppressWarnings("null") - @NonNull String value = builder.toString().trim(); - return parse(value); + String value = ObjectUtils.notNull(builder.toString().trim()); + try { + return parse(value); + } catch (IllegalArgumentException ex) { + throw new IOException( + String.format("Malformed data '%s'%s. %s", + value, + XmlEventUtil.generateLocationMessage(nextEvent), + ex.getLocalizedMessage()), + ex); + } } catch (XMLStreamException ex) { throw new IOException(ex); } @@ -124,11 +136,20 @@ public TYPE parse(XMLEventReader2 eventReader) throws IOException { public TYPE parse(JsonParser parser) throws IOException { String value = parser.getValueAsString(); if (value == null) { - throw new IOException("Unable to parse field value as text"); + throw new IOException("Unable to null value as text"); } // skip over value parser.nextToken(); - return parse(value); + try { + return parse(value); + } catch (IllegalArgumentException ex) { + throw new IOException( + String.format("Malformed data '%s'%s. %s", + value, + JsonUtil.generateLocationMessage(parser), + ex.getLocalizedMessage()), + ex); + } } @SuppressWarnings("null") @@ -172,10 +193,11 @@ public void writeJsonValue(Object value, JsonGenerator generator) throws IOExcep @Override public ITEM_TYPE cast(IAnyAtomicItem item) { - ITEM_TYPE retval; if (item == null) { throw new InvalidValueForCastFunctionException("item is null"); - } else if (getItemClass().isAssignableFrom(item.getClass())) { + } + ITEM_TYPE retval; + if (getItemClass().isAssignableFrom(item.getClass())) { @SuppressWarnings("unchecked") ITEM_TYPE typedItem = (ITEM_TYPE) item; retval = typedItem; } else { diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/datatype/IDataTypeAdapter.java b/core/src/main/java/gov/nist/secauto/metaschema/core/datatype/IDataTypeAdapter.java index 42a1ae21b..85e48aa23 100644 --- a/core/src/main/java/gov/nist/secauto/metaschema/core/datatype/IDataTypeAdapter.java +++ b/core/src/main/java/gov/nist/secauto/metaschema/core/datatype/IDataTypeAdapter.java @@ -234,6 +234,8 @@ default boolean isAtomic() { * @return a supplier that will provide new instances of the parsed data * @throws IOException * if an error occurs while parsing + * @throws IllegalArgumentException + * if the provided value is invalid based on the data type * @see #parse(String) */ @NonNull diff --git a/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.java b/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.java index 6018eb3b6..16ed4ccef 100644 --- a/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.java +++ b/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.java @@ -316,7 +316,17 @@ public IBoundObject readItemField(IBoundObject parentItem, IBoundDefinitionModel @Override public Object readItemFieldValue(IBoundObject parentItem, IBoundFieldValue fieldValue) throws IOException { // read the field value's value - return readScalarItem(fieldValue); + return checkMissingFieldValue(readScalarItem(fieldValue)); + } + + @NonNull + private Object checkMissingFieldValue(Object value) throws IOException { + if (value == null && LOGGER.isWarnEnabled()) { + LOGGER.atWarn().log("Missing property value{}", + JsonUtil.generateLocationMessage(getReader())); + } + // TODO: change nullness annotations to be @Nullable + return ObjectUtils.notNull(value); } @Override @@ -491,8 +501,17 @@ public void accept( // the field will be the JSON key String key = ObjectUtils.notNull(parser.currentName()); - Object value = jsonKey.getDefinition().getJavaTypeAdapter().parse(key); - jsonKey.setValue(parent, ObjectUtils.notNull(value.toString())); + try { + Object value = jsonKey.getDefinition().getJavaTypeAdapter().parse(key); + jsonKey.setValue(parent, ObjectUtils.notNull(value.toString())); + } catch (IllegalArgumentException ex) { + throw new IOException( + String.format("Malformed data '%s'%s. %s", + key, + JsonUtil.generateLocationMessage(parser), + ex.getLocalizedMessage()), + ex); + } // skip to the next token parser.nextToken(); @@ -626,14 +645,14 @@ private final class JsomValueKeyProblemHandler implements IJsonProblemHandler { @NonNull private final IJsonProblemHandler delegate; @NonNull - private final IBoundInstanceFlag jsonValueKyeFlag; + private final IBoundInstanceFlag jsonValueKeyFlag; private boolean foundJsonValueKey; // false private JsomValueKeyProblemHandler( @NonNull IJsonProblemHandler delegate, - @NonNull IBoundInstanceFlag jsonValueKyeFlag) { + @NonNull IBoundInstanceFlag jsonValueKeyFlag) { this.delegate = delegate; - this.jsonValueKyeFlag = jsonValueKyeFlag; + this.jsonValueKeyFlag = jsonValueKeyFlag; } @Override @@ -656,9 +675,17 @@ public boolean handleUnknownProperty( } else { // handle JSON value key String key = ObjectUtils.notNull(parser.currentName()); - Object keyValue = jsonValueKyeFlag.getJavaTypeAdapter().parse(key); - jsonValueKyeFlag.setValue(ObjectUtils.notNull(parentItem), keyValue); - + try { + Object keyValue = jsonValueKeyFlag.getJavaTypeAdapter().parse(key); + jsonValueKeyFlag.setValue(ObjectUtils.notNull(parentItem), keyValue); + } catch (IllegalArgumentException ex) { + throw new IOException( + String.format("Malformed data '%s'%s. %s", + key, + JsonUtil.generateLocationMessage(parser), + ex.getLocalizedMessage()), + ex); + } // advance past the field name JsonUtil.assertAndAdvance(parser, JsonToken.FIELD_NAME); @@ -672,7 +699,6 @@ public boolean handleUnknownProperty( } return retval; } - } private class ModelInstanceReadHandler @@ -719,7 +745,8 @@ public Map readMap() throws IOException { IBoundInstanceModel instance = getCollectionInfo().getInstance(); - @SuppressWarnings("PMD.UseConcurrentHashMap") Map items = new LinkedHashMap<>(); + @SuppressWarnings("PMD.UseConcurrentHashMap") + Map items = new LinkedHashMap<>(); // A map value is always wrapped in a START_OBJECT, since fields are used for // the keys diff --git a/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java b/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java index e9fd1d80f..c9357a1a0 100644 --- a/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java +++ b/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java @@ -30,6 +30,8 @@ import gov.nist.secauto.metaschema.databind.model.info.IItemReadHandler; import gov.nist.secauto.metaschema.databind.model.info.IModelInstanceCollectionInfo; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.codehaus.stax2.XMLEventReader2; import java.io.IOException; @@ -56,6 +58,7 @@ public class MetaschemaXmlReader implements IXmlParsingContext { + private static final Logger LOGGER = LogManager.getLogger(MetaschemaXmlReader.class); @NonNull private final XMLEventReader2 reader; @NonNull @@ -184,11 +187,20 @@ protected void readFlagInstances( XmlEventUtil.generateLocationMessage(attribute))); } } else { - // get the attribute value - Object value = instance.getDefinition().getJavaTypeAdapter().parse(ObjectUtils.notNull(attribute.getValue())); - // apply the value to the parentObject - instance.setValue(targetObject, value); - flagInstanceMap.remove(qname); + try { + // get the attribute value + Object value = instance.getDefinition().getJavaTypeAdapter().parse(ObjectUtils.notNull(attribute.getValue())); + // apply the value to the parentObject + instance.setValue(targetObject, value); + flagInstanceMap.remove(qname); + } catch (IllegalArgumentException ex) { + throw new IOException( + String.format("Malformed data '%s'%s. %s", + attribute.getValue(), + XmlEventUtil.generateLocationMessage(start), + ex.getLocalizedMessage()), + ex); + } } } @@ -453,6 +465,7 @@ private IBoundObject readDefinitionEl public Object readItemFlag( IBoundObject parent, IBoundInstanceFlag flag) throws IOException { + // should never be called throw new UnsupportedOperationException("handled by readFlagInstances()"); } @@ -481,7 +494,7 @@ public Object readItemField( XmlEventUtil.requireStartElement(getReader(), wrapper); } - Object retval = readScalarItem(instance); + Object retval = checkMissingFieldValue(readScalarItem(instance)); if (wrapper != null) { XmlEventUtil.skipWhitespace(getReader()); @@ -534,7 +547,19 @@ public IBoundObject readItemField( public Object readItemFieldValue( IBoundObject parent, IBoundFieldValue fieldValue) throws IOException { - return readScalarItem(fieldValue); + return checkMissingFieldValue(readScalarItem(fieldValue)); + } + + @SuppressWarnings("null") + @NonNull + private Object checkMissingFieldValue(Object value) throws IOException { + if (value == null && LOGGER.isWarnEnabled()) { + StartElement start = getStartElement(); + LOGGER.atWarn().log("Missing property value{}", + XmlEventUtil.generateLocationMessage(start)); + } + // TODO: change nullness annotations to be @Nullable + return value; } private void handleAssemblyDefinitionBody( @@ -578,7 +603,7 @@ public IBoundObject readItemAssembly( this::handleAssemblyDefinitionBody); } - @NonNull + @Nullable private Object readScalarItem(@NonNull IFeatureScalarItemValueHandler handler) throws IOException { return handler.getJavaTypeAdapter().parse(getReader()); diff --git a/databind/src/main/java/gov/nist/secauto/metaschema/databind/model/info/IFeatureScalarItemValueHandler.java b/databind/src/main/java/gov/nist/secauto/metaschema/databind/model/info/IFeatureScalarItemValueHandler.java index bc2cd0edc..df1b96790 100644 --- a/databind/src/main/java/gov/nist/secauto/metaschema/databind/model/info/IFeatureScalarItemValueHandler.java +++ b/databind/src/main/java/gov/nist/secauto/metaschema/databind/model/info/IFeatureScalarItemValueHandler.java @@ -16,6 +16,20 @@ public interface IFeatureScalarItemValueHandler extends IItemValueHandler, IValuedMutable { + /** + * Apply the string value. + *

+ * This first parses the value using the underlying data type implementation and + * then applies the parsed value. + * + * @param parent + * the parent object to apply the value to + * @param text + * the value to parse + * @throws IllegalArgumentException + * if the text was malformed + * @see #getJavaTypeAdapter() + */ default void setValue(@NonNull Object parent, @NonNull String text) { Object item = getValueFromString(text); setValue(parent, item); @@ -27,6 +41,16 @@ default String toStringFromItem(@NonNull Object parent) { return item == null ? null : getJavaTypeAdapter().asString(item); } + /** + * Parse a string value using the underlying data type implementation. + * + * @param text + * the value to parse + * @return the parsed value + * @throws IllegalArgumentException + * if the text was malformed + * @see #getJavaTypeAdapter() + */ default Object getValueFromString(@NonNull String text) { return getJavaTypeAdapter().parse(text); }