Update RecaptchaService.groovy

[jameskleeh]

An NPE is thrown if the param is not included in the request

[iamthechad]

This PR is marked as having a conflict. Did you fetch from upstream before submitting?

Also, can you add a unit test to cover this scenario?

Thanks.

[jameskleeh]

I actually just used github's built in editor. I didn't do anything local

[iamthechad]

So then it's not actually a situation you ran into, but something you surmised could happen from looking at the code?

[jameskleeh]

Can you point me to the existing service tests? I can't seem to find it

[jameskleeh]

No, I actually ran into the issue. Can you not see it as a possibility?

[iamthechad]

It's never been reported, and this code was ported straight from the legacy grails-recaptcha plugin, where it's been like that for almost a year. I can see it as a possibility perhaps if something isn't configured correctly, but that parameter is automatically generated by the ReCaptcha JavaScript and it should always be present.

The tests for this class are located in src/test/groovy/com/megatome/grails/RecaptchaServiceTests.groovy

[KerchumA222]

I have run into this issue a few times before. When params doesn't contain that key everything blows up. It's somewhat difficult to debug from there because of dynamic types, etc.

[jameskleeh]

I am using the renderParameters and handling it client side with angularjs.

What do you want the unit test to test? A null passed into recap.checkAnswer?

[iamthechad]

Ideally, you'd write a test first to easily demonstrate that a missing g-recaptcha-response causes an exception. Then apply your change and make sure that the test passes.

[jameskleeh]

Once I apply the change the test would fail

[iamthechad]

• Revert your change temporarily.
• Write a test that demonstrates the issue. This test will fail.
• Apply your change.
• Verify that the test passes with your change.

[jameskleeh]

My point is that it results in no added tests to the source code. I'll add a test that it doesn't throw an NPE