8290367: Update default value and extend the scope of com.sun.jndi.ldap.object.trustSerialData system property

[AlekseiEfimov]

Summary of the change

The LDAP Naming Service Provider implementation's default settings are changed to disallow deserialization and reconstruction of Java objects from different LDAP attributes (RFC 2713). Currently, only the deserialization is controlled by the com.sun.jndi.ldap.object.trustSerialData system property, and it is allowed by default.
The change proposed here switches the default value of thecom.sun.jndi.ldap.object.trustSerialDatasystem property to "false", and also extends its scope to cover the reconstruction of RMI remote objects from the javaRemoteLocation LDAP attribute.

CSR for this change can be viewed here.

List of code changes

• Switch the default value of the 'com.sun.jndi.ldap.object.trustSerialData' system property to "false".

• Extend the scope of the property to also cover the reconstruction of RMI remote objects from the deprecated 'javaRemoteLocation' LDAP attribute.

• Document the support for javaRemoteLocation and the javaReferenceAddress LDAP attributes in java.naming's module-info.

Test changes

• New test/jdk/com/sun/jndi/ldap/objects/RemoteLocationAttributeTest.java test has been added to test that com.sun.jndi.ldap.object.trustSerialData system property can be used to control reconstruction of RMI objects from the javaRemoteLocation LDAP attribute.

• test/jdk/javax/naming/module/RunBasic.java was modified to pass com.sun.jndi.ldap.object.trustSerialData=true to the sub-tests that rely on reconstruction/deserialization from LDAP attributes.

• During the update for test/jdk/javax/naming/module/RunBasic.java, it was spotted that sub-tests apps launched in separate processes were returning the '0' exit value irrelevant to their execution status. All these sub-tests were modified to throw an exception when failure is observed. It helps to ensure that the exit value of launched process is not '0' for failed sub-tests.

Testing

tier1-tier3 and JNDI regression/JCK tests not showing any failures related to this change.
No failures observed for the modified regression tests.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change requires a CSR request to be approved

Issues

• JDK-8290367: Update default value and extend the scope of com.sun.jndi.ldap.object.trustSerialData system property
• JDK-8290369: Update default value and extend the scope of com.sun.jndi.ldap.object.trustSerialData system property (CSR)

Reviewers

• Daniel Fuchs (@dfuch - Reviewer) ⚠️ Review applies to faec04e6
• Jaikiran Pai (@jaikiran - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/10228/head:pull/10228
$ git checkout pull/10228

Update a local copy of the PR:
$ git checkout pull/10228
$ git pull https://git.openjdk.org/jdk pull/10228/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10228

View PR using the GUI difftool:
$ git pr show -t 10228

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10228.diff

[bridgekeeper]

👋 Welcome back aefimov! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[AlekseiEfimov]

/csr needed

[openjdk]

@AlekseiEfimov an approved CSR request is already required for this pull request.

[openjdk]

@AlekseiEfimov The following label will be automatically applied to this pull request:

• core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 02: Full - Incremental (7f16da07)
• 01: Full - Incremental (faec04e6)
• 00: Full (40868838)

[dfuch]

@AlekseiEfimov The CSR is well written, and the update to the module-info.java look good to me. The code changes and tests changes look good. I'm glad to see this change.

[jaikiran]

Thank you for the changes, Aleksei. They look fine to me.

[AlekseiEfimov]

/integrate

[openjdk]

@AlekseiEfimov This pull request has not yet been marked as ready for integration.

[openjdk]

@AlekseiEfimov This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8290367: Update default value and extend the scope of com.sun.jndi.ldap.object.trustSerialData system property

Reviewed-by: dfuchs, jpai

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 123 new commits pushed to the master branch:

• 11e7d53: 8293819: sun/util/logging/PlatformLoggerTest.java failed with "RuntimeException: Retrieved backing PlatformLogger level null is not the expected CONFIG"
• 141d5f5: 8293767: AWT test TestSinhalaChar.java has old SCCS markings
• 3beca2d: 8291600: [vectorapi] vector cast op check is not always needed for vector mask cast
• 9a40b76: 8293842: IPv6-only systems throws UnsupportedOperationException for several socket/TCP options
• bb9aa4e: 8293813: ProblemList com/sun/jdi/JdbLastErrorTest.java on windows-x64 in Xcomp mode
• 4cec141: 8291509: Minor cleanup could be done in sun.security
• 6beeb84: 8293875: ProblemList sun/management/jmxremote/bootstrap/RmiBootstrapTest.java#id1 on linux-x64
• bf79f99: 8292989: Avoid dynamic memory in AsyncLogWriter
• 2028ec7: 8289608: Change com/sun/jdi tests to not use Thread.suspend/resume
• ecb456a: 8293779: redundant checking in AESCrypt.makeSessionKey() method
• ... and 113 more: https://git.openjdk.org/jdk/compare/dbb2c4b6ac01d2a3367a2354213d3b4230dfbb96...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[AlekseiEfimov]

/integrate

[openjdk]

Going to push as commit 7765942.
Since your change was applied there have been 123 commits pushed to the master branch:

• 11e7d53: 8293819: sun/util/logging/PlatformLoggerTest.java failed with "RuntimeException: Retrieved backing PlatformLogger level null is not the expected CONFIG"
• 141d5f5: 8293767: AWT test TestSinhalaChar.java has old SCCS markings
• 3beca2d: 8291600: [vectorapi] vector cast op check is not always needed for vector mask cast
• 9a40b76: 8293842: IPv6-only systems throws UnsupportedOperationException for several socket/TCP options
• bb9aa4e: 8293813: ProblemList com/sun/jdi/JdbLastErrorTest.java on windows-x64 in Xcomp mode
• 4cec141: 8291509: Minor cleanup could be done in sun.security
• 6beeb84: 8293875: ProblemList sun/management/jmxremote/bootstrap/RmiBootstrapTest.java#id1 on linux-x64
• bf79f99: 8292989: Avoid dynamic memory in AsyncLogWriter
• 2028ec7: 8289608: Change com/sun/jdi tests to not use Thread.suspend/resume
• ecb456a: 8293779: redundant checking in AESCrypt.makeSessionKey() method
• ... and 113 more: https://git.openjdk.org/jdk/compare/dbb2c4b6ac01d2a3367a2354213d3b4230dfbb96...master

Your commit was automatically rebased without conflicts.

[openjdk]

@AlekseiEfimov Pushed as commit 7765942.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.