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

8179503: Java should support GET OCSP calls #1917

Closed
wants to merge 1 commit into from

Conversation

apavlyutkin
Copy link
Contributor

@apavlyutkin apavlyutkin commented May 31, 2023

Hi, here is backport of JDK-8179503 that adds support of GET OCSP calls for parity with Oracle: in spite of the fact that JBS issue is not labelled with any Oracle release, this one is mandatory for JDK-8274471 released to 11.0.18-oracle

Original patch applied with the only change

src/java.base/share/classes/sun/security/provider/certpath/OCSP.java

  • resolved baseline conflict related to revokation checking

Verification (amd64/20.04): newly added test/jdk/java/security/cert/CertPathValidator/OCSP/GetAndPostTests.java
Regression (amd64/20.04): jdk_security

@TheRealMDoerr I raised this one instead of already reviewed #847. Please check it out


Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • JDK-8179503 needs maintainer approval
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue

Issue

  • JDK-8179503: Java should support GET OCSP calls (Enhancement - P3)

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk11u-dev.git pull/1917/head:pull/1917
$ git checkout pull/1917

Update a local copy of the PR:
$ git checkout pull/1917
$ git pull https://git.openjdk.org/jdk11u-dev.git pull/1917/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1917

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk11u-dev/pull/1917.diff

Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented May 31, 2023

👋 Welcome back apavlyutkin! 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.

@openjdk openjdk bot changed the title Backport f5ee356540d7aa4a7663c0d5d74f5fdb0726b426 8179503: Java should support GET OCSP calls May 31, 2023
@openjdk
Copy link

openjdk bot commented May 31, 2023

This backport pull request has now been updated with issue from the original commit.

@openjdk openjdk bot added backport rfr Pull request is ready for review labels May 31, 2023
@mlbridge
Copy link

mlbridge bot commented May 31, 2023

Webrevs

Copy link
Contributor

@TheRealMDoerr TheRealMDoerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still good.

@openjdk
Copy link

openjdk bot commented May 31, 2023

@apavlyutkin This change is no longer ready for integration - check the PR body for details.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label May 31, 2023
@bridgekeeper
Copy link

bridgekeeper bot commented Jul 13, 2023

@apavlyutkin This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

@apavlyutkin
Copy link
Contributor Author

Please don't close this, bot.

@bridgekeeper
Copy link

bridgekeeper bot commented Aug 21, 2023

@apavlyutkin This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

@openjdk
Copy link

openjdk bot commented Sep 26, 2023

⚠️ @apavlyutkin This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

@openjdk openjdk bot removed the ready Pull request is ready to be integrated label Sep 26, 2023
@apavlyutkin
Copy link
Contributor Author

There is no a blocker anymore

@openjdk openjdk bot added the approval label Oct 2, 2023
@GoeLin
Copy link
Member

GoeLin commented Oct 5, 2023

Please also request the follow up change in PR 1920. I will approve both.

@apavlyutkin
Copy link
Contributor Author

apavlyutkin commented Oct 5, 2023

Please also request the follow up change in PR 1920. I will approve both.

This is a chain of 4 dependent PR's: 1917 (this one), 1918 (not yet reviewed), 1920 (reviewed), and 1921 (clean), and 1920 only makes sense on top of 1918.

@GoeLin
Copy link
Member

GoeLin commented Oct 8, 2023

Hi, I think it would be good to merge head into these, maybe into the topmost change of the chain of dependent fixes.
This would cause the tests to run again.

@alexey-pavlyutkin
Copy link

alexey-pavlyutkin commented Oct 9, 2023

Hi, Goetz, let's make it AFTER all the changes get reviewed. Once I rebased the original chain #847 (this one is just a copy) through a year of master and Github went crazy, it correctly displayed the delta as a patch, but at the same time review pane also showed all the commits to master for this year making review just impossible, and exactly that is the reason why I created this copy of the original chain. Thank you

@GoeLin
Copy link
Member

GoeLin commented Oct 16, 2023

Hi, ok, I am waiting for PR 1918 being reviewed.
I ran 1917, 1918 and 1920 through our testing. I had to grep the patches and do some simple resolves. Unfortunately, 1918 does not compile:
java.base/share/classes/sun/security/x509/X509CertImpl.java:1087: error: unreported exception IOException; must be caught or declared to be thrown
return algId == null ? null : algId.getEncodedParams();
Actually there was a row of other patches in the nightbuild, but after removing these three it worked.

@apavlyutkin
Copy link
Contributor Author

I do not observe the problem

Creating jdk image
Stopping sjavac server
Finished building target 'images' in configuration 'linux-x86_64-normal-server-release'
alex@alex-vbox:~/jdk11u-dev$ git status
On branch 8274471_
Your branch is up to date with 'origin/8274471_'.

nothing to commit, working tree clean
alex@alex-vbox:~/jdk11u-dev$ git log
commit 6192658292ae314236fcd63591438f8b5ed17db0 (HEAD -> 8274471_, origin/8274471_)
Author: Alexey Pavlyutkin <apavlyutkin@azul.com>
Date:   Wed May 31 13:46:34 2023 +0300

    Backport f63c4a832a1aea451f47aaf86d5361e970c6a28f

commit 2355e23d467afc603867d87c8820149d20727c46 (origin/8179503_, 8179503_)
Author: Alexey Pavlyutkin <apavlyutkin@azul.com>
Date:   Wed May 31 12:04:56 2023 +0300

    Backport f5ee356540d7aa4a7663c0d5d74f5fdb0726b426

commit b2faa3567e3b934689845025333c41bcbfe18b1e (origin/master, origin/HEAD, master)

Author: Goetz Lindenmaier <goetz@openjdk.org>
Date:   Tue May 30 16:40:22 2023 +0000

    8309108: Bump update version for OpenJDK: jdk-11.0.21

    Reviewed-by: mdoerr

It looks like for some reason your nigthbuild didn't incorporate all the changes

https://github.com/openjdk/jdk11u-dev/pull/1918/files#diff-bfcfa1bb8e1022116232bea13c3b15a51955d853a5a06c2b8fd5b264a07e8d6dL294

@openjdk openjdk bot added approval and removed approval labels Oct 24, 2023
@bridgekeeper
Copy link

bridgekeeper bot commented Nov 14, 2023

@apavlyutkin This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

@bridgekeeper
Copy link

bridgekeeper bot commented Dec 12, 2023

@apavlyutkin This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

@bridgekeeper bridgekeeper bot closed this Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport rfr Pull request is ready for review
Development

Successfully merging this pull request may close these issues.

4 participants