IGNITE-2849: BinaryObjectBuilder doesn't properly check metadata
Project: http://git-wip-us.apache.org/repos/asf/ignite/repo Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/d36a2e51 Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/d36a2e51 Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/d36a2e51 Branch: refs/heads/gridgain-7.5.11-vk Commit: d36a2e51e3aa3105dff73839c84e52a531fbd918 Parents: 9e84e50 Author: Denis Magda <[email protected]> Authored: Thu Mar 24 20:07:48 2016 +0300 Committer: Denis Magda <[email protected]> Committed: Mon Mar 28 17:34:19 2016 +0300 ---------------------------------------------------------------------- .../cache/store/jdbc/CacheJdbcPojoStore.java | 2 +- .../ignite/internal/binary/BinaryUtils.java | 16 --- .../binary/builder/BinaryObjectBuilderImpl.java | 107 ++++++++------ .../BinaryObjectBuilderAdditionalSelfTest.java | 144 +++++++++++++++++-- ...naryObjectBuilderDefaultMappersSelfTest.java | 2 +- 5 files changed, 197 insertions(+), 74 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ignite/blob/d36a2e51/modules/core/src/main/java/org/apache/ignite/cache/store/jdbc/CacheJdbcPojoStore.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/cache/store/jdbc/CacheJdbcPojoStore.java b/modules/core/src/main/java/org/apache/ignite/cache/store/jdbc/CacheJdbcPojoStore.java index 200aa0f..b9a3118 100644 --- a/modules/core/src/main/java/org/apache/ignite/cache/store/jdbc/CacheJdbcPojoStore.java +++ b/modules/core/src/main/java/org/apache/ignite/cache/store/jdbc/CacheJdbcPojoStore.java @@ -248,7 +248,7 @@ public class CacheJdbcPojoStore<K, V> extends CacheAbstractJdbcStore<K, V> { Object colVal = getColumnValue(rs, colIdx, field.getJavaFieldType()); - builder.setField(field.getJavaFieldName(), colVal); + builder.setField(field.getJavaFieldName(), colVal, (Class<Object>)field.getJavaFieldType()); if (calcHash) hashValues.add(colVal); http://git-wip-us.apache.org/repos/asf/ignite/blob/d36a2e51/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java index 4a79f22..eefdc3f 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java @@ -317,22 +317,6 @@ public class BinaryUtils { } /** - * @param typeName Field type name. - * @return Field type ID; - */ - @SuppressWarnings("StringEquality") - public static int fieldTypeId(String typeName) { - for (int i = 0; i < FIELD_TYPE_NAMES.length; i++) { - String typeName0 = FIELD_TYPE_NAMES[i]; - - if (typeName.equals(typeName0)) - return i; - } - - throw new IllegalArgumentException("Invalid metadata type name: " + typeName); - } - - /** * @param typeId Field type ID. * @return Field type name. */ http://git-wip-us.apache.org/repos/asf/ignite/blob/d36a2e51/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryObjectBuilderImpl.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryObjectBuilderImpl.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryObjectBuilderImpl.java index 9043a8b..16c51b0 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryObjectBuilderImpl.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryObjectBuilderImpl.java @@ -195,6 +195,10 @@ public class BinaryObjectBuilderImpl implements BinaryObjectBuilder { Set<Integer> remainsFlds = null; + BinaryType meta = ctx.metadata(typeId); + + Map<String, Integer> fieldsMeta = null; + if (reader != null) { BinarySchema schema = reader.schema(); @@ -204,9 +208,15 @@ public class BinaryObjectBuilderImpl implements BinaryObjectBuilder { assignedFldsById = U.newHashMap(assignedVals.size()); for (Map.Entry<String, Object> entry : assignedVals.entrySet()) { - int fieldId = ctx.fieldId(typeId, entry.getKey()); + String name = entry.getKey(); + Object val = entry.getValue(); + + int fieldId = ctx.fieldId(typeId, name); - assignedFldsById.put(fieldId, entry.getValue()); + assignedFldsById.put(fieldId, val); + + if (val != REMOVED_FIELD_MARKER) + fieldsMeta = checkMetadata(meta, fieldsMeta, val, name); } remainsFlds = assignedFldsById.keySet(); @@ -280,10 +290,6 @@ public class BinaryObjectBuilderImpl implements BinaryObjectBuilder { } } - BinaryType meta = ctx.metadata(typeId); - - Map<String, Integer> fieldsMeta = null; - if (assignedVals != null && (remainsFlds == null || !remainsFlds.isEmpty())) { for (Map.Entry<String, Object> entry : assignedVals.entrySet()) { Object val = entry.getValue(); @@ -302,43 +308,9 @@ public class BinaryObjectBuilderImpl implements BinaryObjectBuilder { serializer.writeValue(writer, val); - String oldFldTypeName = meta == null ? null : meta.fieldTypeName(name); - - boolean nullObjField = false; - - int newFldTypeId; - - if (val instanceof BinaryValueWithType) { - newFldTypeId = ((BinaryValueWithType)val).typeId(); - - if (newFldTypeId == GridBinaryMarshaller.OBJ && ((BinaryValueWithType)val).value() == null) - nullObjField = true; - } - else - newFldTypeId = BinaryUtils.typeByClass(val.getClass()); - - String newFldTypeName = BinaryUtils.fieldTypeName(newFldTypeId); - - if (oldFldTypeName == null) { - // It's a new field, we have to add it to metadata. - if (fieldsMeta == null) - fieldsMeta = new HashMap<>(); - - fieldsMeta.put(name, BinaryUtils.fieldTypeId(newFldTypeName)); - } - else if (!nullObjField) { - String objTypeName = BinaryUtils.fieldTypeName(GridBinaryMarshaller.OBJ); - - if (!objTypeName.equals(oldFldTypeName) && !oldFldTypeName.equals(newFldTypeName)) { - throw new BinaryObjectException( - "Wrong value has been set [" + - "typeName=" + (typeName == null ? meta.typeName() : typeName) + - ", fieldName=" + name + - ", fieldType=" + oldFldTypeName + - ", assignedValueType=" + newFldTypeName + ']' - ); - } - } + if (reader == null) + // Metadata has already been checked. + fieldsMeta = checkMetadata(meta, fieldsMeta, val, name); } } @@ -386,6 +358,55 @@ public class BinaryObjectBuilderImpl implements BinaryObjectBuilder { } } + /** + * Checks metadata when a BinaryObject is being serialized. + * + * @param meta Current metadata. + * @param fieldsMeta Map holding metadata information that has to be updated. + * @param newVal Field value being serialized. + * @param name Field name. + */ + private Map<String, Integer> checkMetadata(BinaryType meta, Map<String, Integer> fieldsMeta, Object newVal, + String name) { + String oldFldTypeName = meta == null ? null : meta.fieldTypeName(name); + + boolean nullFieldVal = false; + + int newFldTypeId; + + if (newVal instanceof BinaryValueWithType) { + newFldTypeId = ((BinaryValueWithType)newVal).typeId(); + + if (((BinaryValueWithType)newVal).value() == null) + nullFieldVal = true; + } + else + newFldTypeId = BinaryUtils.typeByClass(newVal.getClass()); + + if (oldFldTypeName == null) { + // It's a new field, we have to add it to metadata. + if (fieldsMeta == null) + fieldsMeta = new HashMap<>(); + + fieldsMeta.put(name, newFldTypeId); + } + else if (!nullFieldVal) { + String newFldTypeName = BinaryUtils.fieldTypeName(newFldTypeId); + + if (!F.eq(newFldTypeName, oldFldTypeName)) { + throw new BinaryObjectException( + "Wrong value has been set [" + + "typeName=" + (typeName == null ? meta.typeName() : typeName) + + ", fieldName=" + name + + ", fieldType=" + oldFldTypeName + + ", assignedValueType=" + newFldTypeName + ']' + ); + } + } + + return fieldsMeta; + } + /** {@inheritDoc} */ @Override public BinaryObjectBuilderImpl hashCode(int hashCode) { this.hashCode = hashCode; http://git-wip-us.apache.org/repos/asf/ignite/blob/d36a2e51/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderAdditionalSelfTest.java ---------------------------------------------------------------------- diff --git a/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderAdditionalSelfTest.java b/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderAdditionalSelfTest.java index 804c060..e3e538b 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderAdditionalSelfTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderAdditionalSelfTest.java @@ -26,6 +26,7 @@ import org.apache.ignite.IgniteBinary; import org.apache.ignite.IgniteCheckedException; import org.apache.ignite.binary.BinaryObject; import org.apache.ignite.binary.BinaryObjectBuilder; +import org.apache.ignite.binary.BinaryObjectException; import org.apache.ignite.binary.BinaryType; import org.apache.ignite.configuration.BinaryConfiguration; import org.apache.ignite.configuration.CacheConfiguration; @@ -234,7 +235,7 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes public void testDateArrayModification() { GridBinaryTestClasses.TestObjectAllTypes obj = new GridBinaryTestClasses.TestObjectAllTypes(); - obj.dateArr = new Date[] {new Date(11111), new Date(11111), new Date(11111)}; + obj.dateArr = new Date[] {new Date(11111), new Date(11111), new Date(11111)}; BinaryObjectBuilderImpl mutObj = wrap(obj); @@ -498,7 +499,7 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes Object[] createdArr = {mutObj, "a", 1, new String[] {"s", "s"}, new byte[] {1, 2}, new UUID(3, 0)}; - mutObj.setField("foo", createdArr.clone()); + mutObj.setField("foo", createdArr.clone(), Object.class); GridBinaryTestClasses.TestObjectContainer res = mutObj.build().deserialize(); @@ -555,7 +556,7 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes ArrayList<Object> list = Lists.newArrayList(mutObj, "a", Lists.newArrayList(1, 2)); - mutObj.setField("foo", list); + mutObj.setField("foo", list, Object.class); GridBinaryTestClasses.TestObjectContainer res = mutObj.build().deserialize(); @@ -653,7 +654,7 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes List<Object> list = Lists.newLinkedList(Arrays.asList(mutObj, "a", Lists.newLinkedList(Arrays.asList(1, 2)))); - mutObj.setField("foo", list); + mutObj.setField("foo", list, Object.class); GridBinaryTestClasses.TestObjectContainer res = mutObj.build().deserialize(); @@ -736,7 +737,7 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes Set<Object> c = Sets.newHashSet(mutObj, "a", Sets.newHashSet(1, 2)); - mutObj.setField("foo", c); + mutObj.setField("foo", c, Object.class); GridBinaryTestClasses.TestObjectContainer res = mutObj.build().deserialize(); @@ -815,7 +816,7 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes Map<Object, Object> map = Maps.newHashMap(ImmutableMap.of(mutObj, "a", "b", mutObj)); - mutObj.setField("foo", map); + mutObj.setField("foo", map, Object.class); GridBinaryTestClasses.TestObjectContainer res = mutObj.build().deserialize(); @@ -995,6 +996,123 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes /** * */ + public void testWrongMetadataNullField() { + BinaryObjectBuilder builder = binaries().builder("SomeType"); + + builder.setField("dateField", null); + + builder.setField("objectField", null, Integer.class); + + builder.build(); + + try { + builder = binaries().builder("SomeType"); + + builder.setField("dateField", new Date()); + + builder.build(); + } + catch (BinaryObjectException ex) { + assertTrue(ex.getMessage().startsWith("Wrong value has been set")); + } + + builder = binaries().builder("SomeType"); + + try { + builder.setField("objectField", new GridBinaryTestClasses.Company()); + + builder.build(); + + fail("BinaryObjectBuilder accepted wrong metadata"); + } + catch (BinaryObjectException ex) { + assertTrue(ex.getMessage().startsWith("Wrong value has been set")); + } + } + + /** + * + */ + public void testWrongMetadataNullField2() { + BinaryObjectBuilder builder = binaries().builder("SomeType1"); + + builder.setField("dateField", null); + + builder.setField("objectField", null, Integer.class); + + BinaryObject obj = builder.build(); + + try { + builder = binaries().builder(obj); + + builder.setField("dateField", new Date()); + + builder.build(); + } + catch (BinaryObjectException ex) { + assertTrue(ex.getMessage().startsWith("Wrong value has been set")); + } + + builder = binaries().builder(obj); + + try { + builder.setField("objectField", new GridBinaryTestClasses.Company()); + + builder.build(); + + fail("BinaryObjectBuilder accepted wrong metadata"); + } + catch (BinaryObjectException ex) { + assertTrue(ex.getMessage().startsWith("Wrong value has been set")); + } + } + + /** + * + */ + public void testCorrectMetadataNullField() { + BinaryObjectBuilder builder = binaries().builder("SomeType2"); + + builder.setField("dateField", null, Date.class); + + builder.setField("objectField", null, GridBinaryTestClasses.Company.class); + + builder.build(); + + builder = binaries().builder("SomeType2"); + + builder.setField("dateField", new Date()); + + builder.setField("objectField", new GridBinaryTestClasses.Company()); + + builder.build(); + + } + + /** + * + */ + public void testCorrectMetadataNullField2() { + BinaryObjectBuilder builder = binaries().builder("SomeType3"); + + builder.setField("dateField", null, Date.class); + + builder.setField("objectField", null, GridBinaryTestClasses.Company.class); + + BinaryObject obj = builder.build(); + + builder = binaries().builder(obj); + + builder.setField("dateField", new Date()); + + builder.setField("objectField", new GridBinaryTestClasses.Company()); + + builder.build(); + } + + /** + * + */ public void testDateInObjectField() { GridBinaryTestClasses.TestObjectContainer obj = new GridBinaryTestClasses.TestObjectContainer(); @@ -1053,9 +1171,9 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes BinaryObjectBuilderImpl mutableObj = wrap(obj); - Date[] arr = { new Date() }; + Date[] arr = {new Date()}; - mutableObj.setField("foo", arr); + mutableObj.setField("foo", arr, Object.class); GridBinaryTestClasses.TestObjectContainer res = mutableObj.build().deserialize(); @@ -1072,9 +1190,9 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes BinaryObjectBuilderImpl mutableObj = wrap(obj); - Timestamp[] arr = { new Timestamp(100020003) }; + Timestamp[] arr = {new Timestamp(100020003)}; - mutableObj.setField("foo", arr); + mutableObj.setField("foo", arr, Object.class); GridBinaryTestClasses.TestObjectContainer res = mutableObj.build().deserialize(); @@ -1264,7 +1382,7 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes CacheObjectBinaryProcessorImpl processor = (CacheObjectBinaryProcessorImpl)( (IgniteBinaryImpl)binaries()).processor(); - return new BinaryObjectBuilderImpl(processor.binaryContext(), processor.typeId(aCls.getName()), + return new BinaryObjectBuilderImpl(processor.binaryContext(), processor.typeId(aCls.getName()), processor.binaryContext().userTypeName(aCls.getName())); } @@ -1305,7 +1423,7 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes root.setField("linkedHashSet", linkedHashSet); root.setField("singletonList", Collections.singletonList(Integer.MAX_VALUE), Collection.class); - root.setField("singletonSet", Collections.singleton(Integer.MAX_VALUE), Collection.class); + root.setField("singletonSet", Collections.singleton(Integer.MAX_VALUE), Collection.class); // maps root.setField("hashMap", hashMap); @@ -1319,7 +1437,7 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes root.setField("asMap", Collections.singletonMap("key", "val")); root.setField("asListHint", Collections.singletonList(Integer.MAX_VALUE), List.class); root.setField("asSetHint", Collections.singleton(Integer.MAX_VALUE), Set.class); - root.setField("asMapHint", (AbstractMap) Collections.singletonMap("key", "val"), AbstractMap.class); + root.setField("asMapHint", (AbstractMap)Collections.singletonMap("key", "val"), AbstractMap.class); BinaryObject binaryObj = root.build(); http://git-wip-us.apache.org/repos/asf/ignite/blob/d36a2e51/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderDefaultMappersSelfTest.java ---------------------------------------------------------------------- diff --git a/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderDefaultMappersSelfTest.java b/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderDefaultMappersSelfTest.java index 61b2731..fcf4413 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderDefaultMappersSelfTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderDefaultMappersSelfTest.java @@ -151,7 +151,7 @@ public class BinaryObjectBuilderDefaultMappersSelfTest extends GridCommonAbstrac builder = builder(obj); - builder.setField("objField", "value"); + builder.setField("objField", "value", Object.class); builder.setField("otherField", (Object)null); obj = builder.build();
