JSON-java icon indicating copy to clipboard operation
JSON-java copied to clipboard

> JSONObject(Map): Throws NPE if key is null

Open stleary opened this issue 2 years ago • 3 comments

JSONObject(Map): Throws NPE if key is null

A bit late here, but I would consider throwing an exception for an input that used to be accepted a breaking API change. In my company's codebase we had some tests that verified this behavior that were broken as a result, and a currently unknown amount of code that could actually be depending on this behavior.

Obviously this ship has already sailed (and rejecting null keys makes much more sense), but can we at least point out the breaking change in the release history documentation? Would be very useful as a heads-up for anyone trying to upgrade from an old version.

Originally posted by @data-enabler in https://github.com/stleary/JSON-java/issues/405#issuecomment-1113714068

stleary avatar Apr 30 '22 14:04 stleary

I think it would be helpful to have a Breaking Changes section or page that includes the above comment. Will look into it.

stleary avatar Apr 30 '22 14:04 stleary

Okay On Fri, Oct 7, 2022, 9:58 PM Yunki Kim @.***> wrote:

I would like to work on this issue.

— Reply to this email directly, view it on GitHub https://github.com/stleary/JSON-java/issues/678#issuecomment-1272207630, or unsubscribe https://github.com/notifications/unsubscribe-auth/AY3ID3D7UUI6GOKEF3CXUYDWCDPMZANCNFSM5UYONSGQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

JayseMayne avatar Oct 08 '22 12:10 JayseMayne

@stleary - I'd like to help out here (if it's still relevant). What exactly are you looking for?

TomerPacific avatar Oct 20 '22 17:10 TomerPacific

@TomerPacific This is a documentation issue. docs/RELEASES.md should include text when potentially breaking changes are introduced. In this issue, #405 was updated to throw an NPE from JSONObject(Map), and was released in 20180813. I have already updated the releases page, but the corresponding entry in RELEASES.md should have a similar change.

stleary avatar Oct 22 '22 13:10 stleary

@stleary, I opened a PR for this.

TomerPacific avatar Oct 28 '22 09:10 TomerPacific