jdk icon indicating copy to clipboard operation
jdk copied to clipboard

8372661: Add a null-safe static factory method to "jdk.test.lib.net.SimpleSSLContext"

Open vy opened this issue 1 month ago • 11 comments

Overhauls SimpleSSLContext to remove the need for null checks at the call site, and to accept a key store file search path, which removes the need to copy-paste SimpleSSLContext just to change the search path.


Progress

  • [x] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • [x] Change must not contain extraneous whitespace
  • [x] Commit message must refer to an issue

Issue

  • JDK-8372661: Add a null-safe static factory method to "jdk.test.lib.net.SimpleSSLContext" (Enhancement - P4)

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 28765

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

Using diff file

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

Using Webrev

Link to Webrev Comment

vy avatar Dec 11 '25 09:12 vy

:wave: Welcome back vyazici! 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.

bridgekeeper[bot] avatar Dec 11 '25 09:12 bridgekeeper[bot]

@vy 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:

8372661: Add a null-safe static factory method to "jdk.test.lib.net.SimpleSSLContext"

Reviewed-by: dfuchs, weijun

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 90 new commits pushed to the master branch:

  • e75726ee03ca4664827ca5d680c02bcf2a96f4ea: 8373832: Test java/lang/invoke/TestVHInvokerCaching.java tests nothing
  • f3a48560b5e3a280f6f76031eb3d475ff9ee49f4: 8373807: test/jdk/java/net/httpclient/websocket/DummyWebSocketServer.java getURI() uses "localhost"
  • 4e05748f0899cabb235c71ecdf4256d4ad137a0d: 8373716: Refactor further java/util tests from TestNG to JUnit
  • ... and 87 more: https://git.openjdk.org/jdk/compare/6a6ff876c515eba6cc89320e02dc5739d4540316...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.

openjdk[bot] avatar Dec 11 '25 09:12 openjdk[bot]

@vy The following labels will be automatically applied to this pull request:

  • javadoc
  • net
  • security

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

openjdk[bot] avatar Dec 11 '25 09:12 openjdk[bot]

Tier1-2 is clear on 326fd60e352.

vy avatar Dec 11 '25 12:12 vy

Webrevs

mlbridge[bot] avatar Dec 11 '25 12:12 mlbridge[bot]

I would prefer to split this PR into two fixes:

  • a first fix that simply adds the new API to SimpleSSLContext, without removing the old API.
  • a second fix that do all the rest: remove the old API and update the tests.

This would allow us to easily backport the first fix, and new tests would not need adaptation when they are later being backported provided that the first fix has been backported first.

You could enjoy using dependent PRs for this :-)

dfuch avatar Dec 11 '25 13:12 dfuch

@dfuch, I've updated the ticket and this PR as follows:

  1. Pushed d4c9b3129fc reverting all changes and only keeping SimpleSSLContext enhancements, and updated the parent ticket description (i.e., JDK-8372661) to reflect this.
  2. Created JDK-8373515 for migrating test/jdk/java/net/httpclient/
  3. Created JDK-8373537 for migrating test/jdk/com/sun/net/httpserver/
  4. Created JDK-8373538 for migrating the rest, and removing the unused SimpleSSLContext methods

vy avatar Dec 11 '25 18:12 vy

/label remove javadoc,security

vy avatar Dec 11 '25 18:12 vy

@vy The javadoc label was successfully removed.

The security label was successfully removed.

openjdk[bot] avatar Dec 11 '25 18:12 openjdk[bot]

@vy Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

openjdk[bot] avatar Dec 12 '25 18:12 openjdk[bot]

@vy security has been added to this pull request based on files touched in new commit(s).

openjdk[bot] avatar Dec 12 '25 18:12 openjdk[bot]

@dfuch, @wangweij, thanks so much for reviews. Tier 1-2 are clear.

/integrate

vy avatar Dec 18 '25 12:12 vy

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

  • 2c0d9a79b8197d88a104bd77026dd45b83a11f8a: 8373396: Min and Max Ideal missing AddNode::Ideal optimisations
  • 2ba423db9925355348106fc9fcf84450123d2605: 8370200: Crash: assert(outer->outcnt() >= phis + 2 - be_loads && outer->outcnt() <= phis + 2 + stores + 1) failed: only phis
  • 4f283f188c43cb25c4eafcdf22eb7f58eae286cc: 8373820: C2: Robust Node::uncast_helper infinite loop check
  • ... and 105 more: https://git.openjdk.org/jdk/compare/6a6ff876c515eba6cc89320e02dc5739d4540316...master

Your commit was automatically rebased without conflicts.

openjdk[bot] avatar Dec 18 '25 12:12 openjdk[bot]

@vy Pushed as commit 629e4ac6f45c87898f6a014f28a443c800413869.

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

openjdk[bot] avatar Dec 18 '25 12:12 openjdk[bot]