Skip to content

fix(dynamodb-enhanced): Fix NPE in EnhancedType for null and wildcard types#6745

Merged
alextwoods merged 8 commits intoaws:masterfrom
abhu85:fix/dynamodb-enhanced-null-checks-6639-5890/2026-02-20
Mar 17, 2026
Merged

fix(dynamodb-enhanced): Fix NPE in EnhancedType for null and wildcard types#6745
alextwoods merged 8 commits intoaws:masterfrom
abhu85:fix/dynamodb-enhanced-null-checks-6639-5890/2026-02-20

Conversation

@abhu85
Copy link
Contributor

@abhu85 abhu85 commented Feb 20, 2026

Summary

This PR fixes NullPointerException bugs in the DynamoDB Enhanced Client's EnhancedType class when dealing with wildcard types like List<?>.

Issue #5890: EnhancedType.hashCode(), equals(), and toString() throw NPE when dealing with wildcard types where rawClass is null.

Root Cause

For wildcard types (e.g., List<?>), EnhancedType stores rawClass = null. The following methods called methods on rawClass without null checks:

  • hashCode() called rawClass.hashCode()
  • equals() called rawClass.equals()
  • innerToString() called rawClass.getTypeName()

Changes

  • EnhancedType.java: Add null checks for rawClass in hashCode(), equals(), and innerToString() methods
  • EnhancedTypeTest.java: Add 3 tests for wildcard type handling in hashCode(), equals(), and toString()

Test Plan

  • All 26 EnhancedTypeTest tests pass (including 3 new tests)
  • Tests verify: wildcard hashCode, wildcard equality, wildcard toString

Test Commands

```bash
./mvnw test -pl services-custom/dynamodb-enhanced -Dtest=EnhancedTypeTest

Tests run: 26, Failures: 0, Errors: 0, Skipped: 0

```

Fixes #5890


Note: The ConverterUtils fix for #6639 has been moved to a separate PR: #6761

…r null values and wildcard types

- Add null checks to ConverterUtils.validateDouble() and validateFloat() to prevent NPE when input is null
- Add null check for rawClass in EnhancedType.hashCode() and equals() to support wildcard types (List<?>)
- Add comprehensive unit tests for null handling

Fixes aws#6639
Fixes aws#5890

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@abhu85 abhu85 requested a review from a team as a code owner February 20, 2026 09:49
@bhoradc
Copy link

bhoradc commented Mar 4, 2026

Hi @abhu85

Thanks for the PR! If you don't mind, could you remove the ConverterUtils changes (#6639) from this PR and submit those as a separate PR?

Also on the EnhancedType wildcard issue, we noticed that toString() has the same NPE when processing wildcard type parameters. Would you mind including a fix for that as well in this PR?

Thanks again for the contribution!

Regards,
Chaitanya

@bhoradc bhoradc added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Mar 4, 2026
abhu85 and others added 2 commits March 5, 2026 07:32
…ils changes

- Add null check in innerToString() for wildcard types (rawClass is null)
- Add test for toString() with wildcard types
- Remove ConverterUtils changes (to be submitted as separate PR per maintainer request)
- Update changelog to reflect EnhancedType-only changes
@abhu85
Copy link
Contributor Author

abhu85 commented Mar 5, 2026

Hi @bhoradc,

Thanks for the feedback! I've addressed both requests:

  1. Split the PR: Removed the ConverterUtils changes from this PR and submitted them separately as fix(dynamodb-enhanced): Fix NPE in ConverterUtils for null input #6761.

  2. Added toString() fix: Added null check in innerToString() for wildcard types, plus a test that verifies:

    • wildcardParam.toString() returns "EnhancedType(?)"
    • listWithWildcard.toString() returns "EnhancedType(java.util.List<?>)"

Summary of changes in this PR now:

  • EnhancedType.java: Null checks in hashCode(), equals(), and toString()/innerToString() for wildcard types
  • EnhancedTypeTest.java: 3 tests for wildcard handling (hashCode, equals, toString)
  • Updated changelog to reflect EnhancedType-only changes

Please let me know if any further changes are needed!

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Mar 5, 2026
@bhoradc
Copy link

bhoradc commented Mar 5, 2026

@abhu85 - Thanks for the quick updates.

One more thing - could you update the PR description to reflect the current scope? It still references the ConverterUtils changes in all sections. Just to keep it clean and ensure we don't accidently close that issue once this merges. Thanks.

@bhoradc bhoradc added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Mar 5, 2026
@github-actions
Copy link

It looks like this PR has not been active for more than five days. In the absence of more information, we will be closing this PR soon. Please add a comment to prevent automatic closure, or if the PR is already closed please feel free to open a new one.

@github-actions github-actions bot added the closing-soon This issue will close in 4 days unless further comments are made. label Mar 15, 2026
@abhu85 abhu85 changed the title fix(dynamodb-enhanced): Fix NPE in ConverterUtils and EnhancedType for null and wildcard types fix(dynamodb-enhanced): Fix NPE in EnhancedType for null and wildcard types Mar 16, 2026
@abhu85
Copy link
Contributor Author

abhu85 commented Mar 16, 2026

Hi @bhoradc,

Apologies for the delay! I've updated the PR description to reflect the current scope - it now only references the EnhancedType changes for #5890.

The ConverterUtils changes for #6639 remain in the separate PR #6761.

Please let me know if there's anything else needed!

@github-actions github-actions bot removed closing-soon This issue will close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. labels Mar 16, 2026
abhu85 and others added 2 commits March 17, 2026 03:07
Addresses review feedback to cover the ternary logic in equals() when
comparing a wildcard type (rawClass=null) against a non-wildcard type
(rawClass!=null). Tests both comparison directions.
@abhu85
Copy link
Contributor Author

abhu85 commented Mar 16, 2026

Thanks for the feedback @alextwoods! I've added a test case wildcardType_equals_notEqualToNonWildcard that covers the wildcard != non-wildcard comparison, testing both directions:

@Test
public void wildcardType_equals_notEqualToNonWildcard() {
    // A wildcard type (rawClass=null) should not be equal to a non-wildcard type (rawClass!=null)
    EnhancedType<List<?>> listWithWildcard = new EnhancedType<List<?>>(){};
    EnhancedType<?> wildcardType = listWithWildcard.rawClassParameters().get(0);
    EnhancedType<String> nonWildcardType = EnhancedType.of(String.class);

    // Wildcard vs non-wildcard should not be equal (tests both comparison directions)
    assertThat(wildcardType).isNotEqualTo(nonWildcardType);
    assertThat(nonWildcardType).isNotEqualTo(wildcardType);
}

This exercises the ternary logic in equals() line 572 when this.rawClass == null but other.rawClass != null, and vice versa.

@abhu85 abhu85 requested a review from alextwoods March 17, 2026 03:50
@abhu85
Copy link
Contributor Author

abhu85 commented Mar 17, 2026

@alextwoods The CI is failing on the API surface area check because this PR modifies public methods in EnhancedType.java (equals(), hashCode(), toString()).

Could you please add one of the following labels after review?

  • api-surface-area-reviewed - if the API changes are approved
  • no-api-surface-area-change - if this is considered a false positive (the behavioral contract is unchanged, just null-safety improvements)

Thanks!

@sonarqubecloud
Copy link

@alextwoods alextwoods added the no-api-surface-area-change Indicate there is no API surface area change and thus API surface area review is not required label Mar 17, 2026
@alextwoods
Copy link
Contributor

@abhu85 - Added the no-api-surface-area-change label since this doesn't have API changes. CI looks good other than that.

@alextwoods alextwoods enabled auto-merge March 17, 2026 17:33
@alextwoods alextwoods added this pull request to the merge queue Mar 17, 2026
Merged via the queue into aws:master with commit 2cde02b Mar 17, 2026
22 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-api-surface-area-change Indicate there is no API surface area change and thus API surface area review is not required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NPE when calling hashCode() on wildcard DynamoDB EnhancedType

3 participants