8263031: HttpClient throws Exception if it receives a Push Promise that is too large

[martinuy]

Hi,

Can I have a review for the backport of JDK-8263031 to 17u?

The JDK main line patch did not apply cleanly because JDK-8295944 was backported to 17u already and tests were moved.

In addition, I had to make changes to PushPromiseContinuation.java to adapt the test to its new location (see jtreg header diff) and context (see the invocations to conn.addToOutputQ). With these changes, the test is passing.

No regressions observed in the jdk/java/net/httpclient tests category.

Thanks,
Martin.-

---

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
• JDK-8263031 needs maintainer approval

Warnings

 ⚠️ Patch contains a binary file (test/jdk/java/util/zip/ZipFile/crash.jar)
 ⚠️ Patch contains a binary file (test/jdk/java/util/zip/ZipFile/input.jar)
 ⚠️ Patch contains a binary file (test/jdk/java/util/zip/ZipFile/input.zip)

Issue

• JDK-8263031: HttpClient throws Exception if it receives a Push Promise that is too large (Bug - P4)

Reviewers

• Matthias Baesken (@MBaesken - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 400

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk17u/pull/400.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back mbalao! 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]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

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

[mlbridge]

Webrevs

• 01: Full - Incremental (7b122c13)
• 00: Full (cdd40840)

[GoeLin]

Please enable the GHA action tests!

[gnu-andrew]

Please enable the GHA action tests!

I think the problem here is this is against 17u, not 17u-dev. 17u is closed for the October release: https://wiki.openjdk.org/display/JDKUpdates/JDK+17u

[martinuy]

I forgot to enable the GHA when forking. They're disabled by default. Running manually now.

…

On Thursday, September 26, 2024, Andrew Hughes ***@***.***> wrote: Please enable the GHA action tests! I think the problem here is this is against 17u, not 17u-dev. 17u is closed for the October release: https://wiki.openjdk.org/ display/JDKUpdates/JDK+17u — Reply to this email directly, view it on GitHub <#400 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFID3EE7YLE3DOVQNTJIKPLZYQP6JAVCNFSM6AAAAABO4JUI2WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNZXGI2TKMBRGI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[gnu-andrew]

I forgot to enable the GHA when forking. They're disabled by default. Running manually now.

The PR should be against 17u-dev.

[alexeybakhtin]

I think the module section is not required anymore as soon as JDK-8295944 in the JDK17 already

[martinuy]

Done. Thanks.

[martinuy]

I forgot to enable the GHA when forking. They're disabled by default. Running manually now.

The PR should be against 17u-dev.

It's my understanding that @RealCLanger prefers PRs against stable repositories when they are for critical fixes in the last part of the cycle. While I tend to agree with that choice, I'm okay with anything as long as there is a unified criteria.

[alexeybakhtin]

Looks good to me. Unfortunately, I have no review status and can not officially approve

[openjdk]

⚠️ @martinuy 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.

[bridgekeeper]

@martinuy 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]

@martinuy this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout JDK-8263031
git fetch https://git.openjdk.org/jdk17u.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

[bridgekeeper]

@martinuy 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]

@martinuy 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]

@martinuy 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.