Issue #240 - added support for "reject duplicate keys" config option

jakartaee / jsonp-api

Jakarta JSON Processing

https://eclipse.org/ee4j/jsonp

Other

141 stars 61 forks source link

Issue #240 - added support for "reject duplicate keys" config option #241

Closed JohnTimm closed 3 years ago

JohnTimm commented 4 years ago

Signed-off-by: John T.E. Timm johntimm@us.ibm.com

leadpony commented 4 years ago

@JohnTimm @lukasj

JsonReaferFactory#getConfigInUse() must return all configuration properties it recognized.
I feel FORBID_DUPLICATE_KEYS is better naming than ALLOW_DUPLICATE_KEYS, because the property does not exist in the configuration properties by default and such default state means allow duplicate keys.

JohnTimm commented 4 years ago

@JohnTimm @lukasj

JsonReaderFactory#getConfigInUse() must return all configuration properties it recognized.

I have updated the code to pass the configuration object into JsonReaderFactoryImpl (in a similar fashion to JsonGeneratorFactoryImpl).

I feel FORBID_DUPLICATE_KEYS is better naming than ALLOW_DUPLICATE_KEYS, because the property does not exist in the configuration properties by default and such default state means allow duplicate keys.

I renamed ALLOW_DUPLICATE_KEYS to FORBID_DUPLICATE_KEYS and updated the logic accordingly. Please see my latest commit.

JohnTimm commented 4 years ago

Make sure that the copyright year is updated to 2020 in all files you changed.

Updated copyright date on JsonObjectBuilderImpl. Please see my latest commit.

JohnTimm commented 4 years ago

@lukasj @m0mus Is there a chance for this PR to make it into the next release candidate?

JohnTimm commented 4 years ago

Make sure that the copyright year is updated to 2020 in all files you changed.

@m0mus Your requested changes have been addressed. What do we need to do to move this PR forward?

nmittles commented 4 years ago

@m0mus Hi Dmitry. Is there anything holding this PR out? It looks like John has made all the requested changes.

JohnTimm commented 4 years ago

@lukasj @m0mus This has been sitting here for months. Please let me know if more needs to be done before it can be merged.

lukasj commented 4 years ago

We did not want this in 2.0.0 and currently waiting for master to open up for integrations (expect ~2-3 weeks)

JohnTimm commented 4 years ago

We did not want this in 2.0.0 and currently waiting for master to open up for integrations (expect ~2-3 weeks)

Sounds good. Thanks for the quick response! I appreciate it.

JohnTimm commented 3 years ago

@lukasj Now that Jakarta EE 8 has shipped, where do we stand on getting this PR pulled into master?

lmsurpre commented 3 years ago

Bumped into this interesting article and it reminded me that we've been waiting on this pull request for 9 months: https://labs.bishopfox.com/tech-blog/an-exploration-of-json-interoperability-vulnerabilities

JohnTimm commented 3 years ago

@lukasj Everything is done through JsonObjectBuilder now so that the behavior should be consistent. Please take a look at my recent commits. We'd like to get this functionality merged sooner than later.