Skip to content

Fix NPE for null values in Map and List collection attribute converters#6782

Open
S-Saranya1 wants to merge 1 commit intomasterfrom
somepal/fix-dynamoDB-enhanced-double-float-converter-npe
Open

Fix NPE for null values in Map and List collection attribute converters#6782
S-Saranya1 wants to merge 1 commit intomasterfrom
somepal/fix-dynamoDB-enhanced-double-float-converter-npe

Conversation

@S-Saranya1
Copy link
Copy Markdown

@S-Saranya1 S-Saranya1 commented Mar 11, 2026

MapAttributeConverter and ListAttributeConverter pass null collection values/elements directly to element converters without null-checking. For converters like DoubleAttributeConverter and FloatAttributeConverter, this causes a NullPointerException due to auto-unboxing in ConverterUtils.validateDouble()/validateFloat(). This occurs when users persist a Map<String, Double> or List containing null values.

Motivation and Context

DynamoDB has a first-class NULL type for representing absent values. When a Java collection contains null elements, the SDK should convert them to DynamoDB NULL(nul(true)) rather than passing them to element converters that aren't designed to handle null. The existing pattern in ResolvedImmutableAttribute already does this for top-level attributes — this fix applies the same approach to collection converters.

Modifications

  • MapAttributeConverter.Delegate.toAttributeValue(): null map values are now converted to AttributeValues.nullAttributeValue() before delegating to the value converter
  • ListAttributeConverter.Delegate.transformFrom(): null list elements are now converted to AttributeValues.nullAttributeValue() before delegating to the element converter

Testing

  • Added parameterized CollectionNullValueConverterTest covering null handling for both MapAttributeConverter and ListAttributeConverter across 8 element converter types(String, Boolean, Integer, Long, Float, Double, BigDecimal, Instant)
    • Existing tests continue to pass — no behavioral change for non-null values

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed
  • I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
  • My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

  • I confirm that this pull request can be released under the Apache 2 license

@S-Saranya1 S-Saranya1 requested a review from a team as a code owner March 11, 2026 22:04
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
63.6% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@joviegas
Copy link
Copy Markdown
Contributor

I think one more option is to fix in the Map and list AttributeConverter to handle the null values and keys and null list objects

public EnhancedAttributeValue toAttributeValue(T input) {
Map<String, AttributeValue> result = new LinkedHashMap<>();
input.forEach((k, v) -> result.put(keyConverter.toString(k), valueConverter.transformFrom(v)));
return EnhancedAttributeValue.fromMap(result);
}

and

public AttributeValue transformFrom(T input) {
return EnhancedAttributeValue.fromListOfAttributeValues(input.stream()
.map(elementConverter::transformFrom)
.collect(toList()))
.toAttributeValue();
}

@S-Saranya1
Copy link
Copy Markdown
Author

I think one more option is to fix in the Map and list AttributeConverter to handle the null values and keys and null list objects

public EnhancedAttributeValue toAttributeValue(T input) {
Map<String, AttributeValue> result = new LinkedHashMap<>();
input.forEach((k, v) -> result.put(keyConverter.toString(k), valueConverter.transformFrom(v)));
return EnhancedAttributeValue.fromMap(result);
}

and

public AttributeValue transformFrom(T input) {
return EnhancedAttributeValue.fromListOfAttributeValues(input.stream()
.map(elementConverter::transformFrom)
.collect(toList()))
.toAttributeValue();
}

Considered fixing at the Map/List converter level, but it would introduce a behavioral change for
StringAttributeConverter and BooleanAttributeConverter. Both currently accept null values without throwing. Adding a null check at the Map/List level would break existing customers who rely on that behavior (e.g., Map<String, String> with null values). So, fixing in ConverterUtils.validateDouble()/validateFloat().

@joviegas
Copy link
Copy Markdown
Contributor

joviegas commented Mar 18, 2026

I think one more option is to fix in the Map and list AttributeConverter to handle the null values and keys and null list objects

public EnhancedAttributeValue toAttributeValue(T input) {
Map<String, AttributeValue> result = new LinkedHashMap<>();
input.forEach((k, v) -> result.put(keyConverter.toString(k), valueConverter.transformFrom(v)));
return EnhancedAttributeValue.fromMap(result);
}

and

public AttributeValue transformFrom(T input) {
return EnhancedAttributeValue.fromListOfAttributeValues(input.stream()
.map(elementConverter::transformFrom)
.collect(toList()))
.toAttributeValue();
}

Considered fixing at the Map/List converter level, but it would introduce a behavioral change for StringAttributeConverter and BooleanAttributeConverter. Both currently accept null values without throwing. Adding a null check at the Map/List level would break existing customers who rely on that behavior (e.g., Map<String, String> with null values). So, fixing in ConverterUtils.validateDouble()/validateFloat().

No I think it will not introduce behavioural change if we do as below

       public EnhancedAttributeValue toAttributeValue(T input) {
            Map<String, AttributeValue> result = new LinkedHashMap<>();
            input.forEach((k, v) -> result.put(keyConverter.toString(k),
                                                v == null ? AttributeValue.fromNul(true)
                                                          : valueConverter.transformFrom(v)));
            return EnhancedAttributeValue.fromMap(result);
        }

Here if its a null then we set to AttributeValue.fromNul(true) which is fine since user explicitly sets the value as Null and its fine sending this as Null Attributes to DDB

We can write paramaterized test to make sure this is backward compatible with all types.

@S-Saranya1 S-Saranya1 force-pushed the somepal/fix-dynamoDB-enhanced-double-float-converter-npe branch from 20eed83 to cf36aa0 Compare April 13, 2026 14:24
@S-Saranya1 S-Saranya1 changed the title Fix NPE in DoubleAttributeConverter and FloatAttributeConverter for null input Fix NPE for null values in Map and List collection attribute converters Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants