Fix objects equal to null not being detected as null

[rafern]

This PR fixes flutter/flutter#37681 by allowing objects equal to null to be used in the StandardMessageCodec, since JSONObject.NULL.equals(null) is true

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it!) and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it!) and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[rafern]

@googlebot I signed it!

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[collinjackson]

Would it be possible to include a test for this?

[rafern]

@collinjackson Sorry for the late response. Would a minimal flutter app that triggers the bug be enough as a test?

If so, I made an integration test on https://github.com/rafern/flutter_11283_test/tree/master/example. It should fail with the old branch and succeed with the new one

[Fallenstedt]

hi,

is it ridiculous of me to ask when this could get reviewed?

[cbracken]

Thanks for your contribution! LGTM for the change. It would be good for us to check in a test that verifies we don't regress this in future.

Given the simplicity of this patch and the effect of not having it, I'm fine with landing as-is and following up with a separate patch to land a unit test in the engine to entire we don't regress this.

@mklim can provide details on how to write such a test.

[mklim]

LGTM, thank you for the fix!

I'm not 100% sure, but at a first look I think testing this should be relatively straightforward. Since the tests are new, I expect the most tedious part will be adding a new test file for StandardMessageCodec.java, following the steps described under "Adding a new test".

Then you'll want to create tests that get the main case of this PR, that null equivalent input values are written to the output ByteBuffer in encodeMessage as NULL. I'd create a test case passing in a value equaling null to encodeMessage and test that the output ByteBuffer had NULL written to it, like expected. I'd also double check while writing the test that it was red without the fix here, then green with it, just to make sure I was testing the right case.

There's much more in this class, but just that one minimal test case is enough to cover the fix in this PR.

[cbracken]

Merged. @rafern if you do get a moment to write a unit test, please send it along. If you don't think you'll have time to, let us know and I can try to find someone to author one so we don't regress this.

[rafern]

@cbracken Thank you for merging the PR. I will try to write a unit test tomorrow and inform you in case I fail to figure out how it works

[Fallenstedt]

Thank you so much everyone!

[rafern]

@mklim @cbracken Would it be possible to include this patch as a hotfix for stable? Currently, the only way for my app to not crash is to use the dev channel

[Fallenstedt]

Hello,

I am sorry to ping you all again, however, I agree with @rafern. Is there a way we can include this in a hotfix? I'm seeing this same issue arise in community packages. Developers are using firebase_auth with github/twitter etc to authenticate with firebase. The problem is these developers are using the stable channel while developing with Android, and are unable to build and test with Android.

See a discussion at: roughike/flutter_twitter_login#28