search

aws / aws-sdk-java-v2

The official AWS SDK for Java - Version 2
Apache License 2.0
2.19k stars 845 forks source link

AdminCreateUserRequest.class not filtering out null #3861

Open jun7343 opened 1 year ago

jun7343 commented 1 year ago

Describe the bug

I tried to request the creation of an account using "AdminCreateRequest" to create an account. When creating "AdminCreateRequest" with a builder, if a null value is received for the DeliveryMediumType parameter, a Null Point Exception occurs.

Expected Behavior

If the DeliveryMediumType value is received as null from the parameter, the default value specified in the API documentation, which is "SMS", should be executed.

Current Behavior

As explained above, if you receive a DeliveryMediumType parameter value of null and build and execute an AdminCreateUserRequest, a NullPointException like the one below will occur.

java.lang.NullPointerException
    at software.amazon.awssdk.services.cognitoidentityprovider.model.DeliveryMediumListTypeCopier.lambda$copyEnumToString$1(DeliveryMediumListTypeCopier.java:47)
    at java.base/java.util.Arrays$ArrayList.forEach(Arrays.java:4390)
    at software.amazon.awssdk.services.cognitoidentityprovider.model.DeliveryMediumListTypeCopier.copyEnumToString(DeliveryMediumListTypeCopier.java:46)
    at software.amazon.awssdk.services.cognitoidentityprovider.model.AdminCreateUserRequest$BuilderImpl.desiredDeliveryMediums(AdminCreateUserRequest.java:1524)
    at software.amazon.awssdk.services.cognitoidentityprovider.model.AdminCreateUserRequest$BuilderImpl.desiredDeliveryMediums(AdminCreateUserRequest.java:1531)
    at kr.co.motov.tmo.helper.CognitoTest.test(CognitoTest.java:40)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:725)
    at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
    at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
    at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
    at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84)
    at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:214)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:210)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:135)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:66)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
    at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
    at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
    at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:107)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52)
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86)
    at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86)
    at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:53)
    at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.processAllTestClasses(JUnitPlatformTestClassProcessor.java:99)
    at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.access$000(JUnitPlatformTestClassProcessor.java:79)
    at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor.stop(JUnitPlatformTestClassProcessor.java:75)
    at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:62)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
    at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
    at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
    at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
    at com.sun.proxy.$Proxy2.stop(Unknown Source)
    at org.gradle.api.internal.tasks.testing.worker.TestWorker$3.run(TestWorker.java:193)
    at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129)
    at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100)
    at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60)
    at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
    at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:113)
    at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:65)
    at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
    at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)

Reproduction Steps

The issue occurs when calling the adminCreateUser method in the createUser method with null being passed as the DeliveryMediumType value, which is a variable argument parameter.

public AdminCreateUserResponse createUser(String username, String temporaryPassword, DeliveryMediumType deliveryMediumType) {
     return this.adminCreateUser(username, temporaryPassword, null);
}

public AdminCreateUserResponse adminCreateUser(String username, String temporaryPassword, DeliveryMediumType deliveryMediumType) {

  try(CognitoIdentityProviderClient client = CognitoIdentityProviderClient.builder()
                    .region(Region.AP_NORTHEAST_2)
                    .credentialsProvider(ProfileCredentialsProvider.create())
                    .build()) {
            AdminCreateUserRequest request = AdminCreateUserRequest.builder()
                    .username(username)
                    .userPoolId("asdjqwjdklaslkdjalskjdk;ljasdjas")
                    .temporaryPassword(temporaryPassword)
                    .desiredDeliveryMediums(deliveryMediumType)
                    .build();

            client.adminCreateUser(request);
        } catch (Exception e) {
            e.printStackTrace();
        }
}

Possible Solution

The following code is a portion of the content within "AdminCreateUserRequest.java".

@Override
@SafeVarargs
public final Builder desiredDeliveryMediumsWithStrings(String... desiredDeliveryMediums) {
    desiredDeliveryMediumsWithStrings(Arrays.asList(desiredDeliveryMediums));
    return this;
}

...

@Override
@SafeVarargs
public final Builder desiredDeliveryMediums(DeliveryMediumType... desiredDeliveryMediums) {
    desiredDeliveryMediums(Arrays.asList(desiredDeliveryMediums));
    return this;
}

In my opinion, it would be better to replace Array.asList() with Arrays.nonNullElementsIn() which can filter out null values.

The following code is a portion of the content within "DeliveryMediumListTypeCopier.java

static List<String> copyEnumToString(Collection<DeliveryMediumType> deliveryMediumListTypeParam) {
      List<String> list;
      if (deliveryMediumListTypeParam == null || deliveryMediumListTypeParam instanceof SdkAutoConstructList) {
          list = DefaultSdkAutoConstructList.getInstance();
      } else {
          List<String> modifiableList = new ArrayList<>();
          deliveryMediumListTypeParam.forEach(entry -> {
              String result = entry.toString();
              modifiableList.add(result);
          });
          list = Collections.unmodifiableList(modifiableList);
      }
      return list;
  }

It seems that the condition should be added to include cases where the value of the Collection parameter is empty in the above code. This is because a NullPointException occurs when iterating through a forEach loop with an empty Collection.

Additional Information/Context

No response

AWS Java SDK version used

2.20.31

JDK version used

11

Operating System and version

Apple Mac M1 Pro Ventura 13.0.1

Sap004 commented 1 year ago

@jun7343 can you please assign this issue to me?

jun7343 commented 1 year ago

@jun7343 can you please assign this issue to me?

I cannot do it, I don't have the authority.

Sap004 commented 1 year ago

@jun7343 can you please assign this issue to me?

I cannot do it, I don't have the authority.

okay I will open a pr in next 24hrs please review and approve

debora-ito commented 1 year ago

If the DeliveryMediumType value is received as null from the parameter, the default value specified in the API documentation, which is "SMS", should be executed.

DeliveryMediumType will default to SMS if no attribute is provided. This is different than providing null.

Will bring this up to the rest of the team. As a workaround, you can add null check to your code.

debora-ito commented 1 year ago

We'll add a better error message than the NullPointerException.

jun7343 commented 1 year ago

We'll add a better error message than the NullPointerException.

ok. thanks

TheMarvelFan commented 1 year ago

Hi is anyone working on this issue? If not, I would like to wrok on it.