1EdTech / basiclti-util-java

Apache License 2.0
57 stars 44 forks source link

NPE when calling @Lti annotated spring controller method with a null argument #28

Closed janeklb closed 7 years ago

janeklb commented 7 years ago

To reproduce:

create a spring controller with a method that contains a @RequestParam(required = false) parameter. Eg.

    @Lti
    @RequestMapping(method = RequestMethod.POST)
    public String post(
            final LtiVerificationResult ltiResult,
            final HttpServletRequest request,
            final ModelMap modelMap,
            @RequestParam(required = false) final Integer someId) throws IOException, OAuthException, GeneralSecurityException {

Results in:

java.lang.NullPointerException
    at org.imsglobal.aspect.LtiLaunchVerifier.verifyLtiLaunch(LtiLaunchVerifier.java:53)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethodWithGivenArgs(AbstractAspectJAdvice.java:621)
    at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethod(AbstractAspectJAdvice.java:610)
    at org.springframework.aop.aspectj.AspectJAroundAdvice.invoke(AspectJAroundAdvice.java:68)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:168)
    at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:92)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
    at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:653)
    at com.intrallect.dalo.spring.controller.LtiController$$EnhancerBySpringCGLIB$$866b786.post(<generated>)
pfgray commented 7 years ago

This looks good. I want to merge this, but you'll need to sign a contributer's agreement before I am allowed to. You can contact @LMattson for details on how to do that.

janeklb commented 7 years ago

How would I go about doing that? I don't see any contact details on @LMattson's profile page.

pfgray commented 7 years ago

@janeklb, sorry this has taken so long.

I need you to sign a Contributor's License Agreement form before I can accept this PR.

The CLA form can be requested by sending an email to info@imsglobal.org

janeklb commented 7 years ago

I'm no longer active in this space and don't plan on contributing any more in the near future, so I'd prefer not to sign the CLA. More than happy to take down the MR and for you to apply the patch manually / offline, if you'd prefer though.

pfgray commented 7 years ago

Completely understandable, I'll patch this myself. Thanks for your efforts

janeklb commented 7 years ago

Cheers :)