Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CUSTOM-54: NullPointerException when JMX attribute is null #4365

Merged
merged 3 commits into from Dec 6, 2019

Conversation

@rdebusscher
Copy link
Contributor

rdebusscher commented Dec 4, 2019

CUSTOM-54: NullPointerException when JMX attribute is null within the rest-monitoring endpoint

Description

This is a bug fix

When JMX attributes are null, it results in NPE when accessed from the rest-monitoring endpoint.

The fix checks if the attribute value is null and return JsonValue.NULL in that case instead of the 'generic' method who tries to convert the instance to a JsonValue.

Testing

Testing Performed

Manual testing

Testing Environment

Oracle 1.8.0_181 on Mac 10.14.6 with Maven 3.5.4

Documentation

N/A

… rest-monitoring endpoint
@rdebusscher rdebusscher requested review from Pandrex247 and dmatej Dec 4, 2019
@rdebusscher

This comment has been minimized.

Copy link
Contributor Author

rdebusscher commented Dec 5, 2019

@MarkWareham Added Unit tests

@rdebusscher rdebusscher requested a review from MarkWareham Dec 5, 2019
@dmatej
dmatej approved these changes Dec 5, 2019
Copy link
Contributor

dmatej left a comment

Test was successful, not sure with AssertJ usage, but ok.

<dependency>
<groupId>org.glassfish</groupId>
<artifactId>javax.json</artifactId>
<version>1.1</version>

This comment has been minimized.

Copy link
@pzygielo

pzygielo Dec 5, 2019

Contributor

Why not use the same version and from jakarta as

Payara/pom.xml

Line 186 in 2e6edd2

<jsonp.version>1.1.5</jsonp.version>

seems to be used for example in
<dependency>
<groupId>org.glassfish</groupId>
<artifactId>jakarta.json</artifactId>
<version>${jsonp.version}</version>
</dependency>

This comment has been minimized.

Copy link
@rdebusscher

rdebusscher Dec 5, 2019

Author Contributor

I though the org.glassfish:jakarta.json pointed to the API, and not an actual implementation., as only the API artifacts should be 'jakartified'.
Fixed in new commit.

@rdebusscher rdebusscher requested review from MarkWareham and dmatej Dec 5, 2019
@rdebusscher

This comment has been minimized.

Copy link
Contributor Author

rdebusscher commented Dec 5, 2019

jenkins test please

@rdebusscher rdebusscher merged commit c8666a2 into payara:master Dec 6, 2019
21 of 25 checks passed
21 of 25 checks passed
security/snyk - appserver/distributions/pom.xml (payara-ci) Test in progress
security/snyk - appserver/jms/pom.xml (payara-ci) Test in progress
security/snyk - nucleus/core/pom.xml (payara-ci) Test in progress
security/snyk - nucleus/grizzly/pom.xml (payara-ci) Test in progress
Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - api/payara-api/pom.xml (payara-ci) No new issues
Details
security/snyk - api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admingui/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/common/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ejb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/extras/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/featuresets/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/installer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/load-balancer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/orb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/registration/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/security/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/transaction/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/cluster/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/test-utils/pom.xml (payara-ci) No new issues
Details
@rdebusscher rdebusscher deleted the rdebusscher:CUSTCOM-54 branch Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.