Skip to content

2.6 jetty http2 http3#1457

Merged
thboileau merged 7 commits into2.6from
2.6_jetty-http2-http3
Apr 26, 2025
Merged

2.6 jetty http2 http3#1457
thboileau merged 7 commits into2.6from
2.6_jetty-http2-http3

Conversation

@thboileau
Copy link
Copy Markdown
Contributor

The aim

Currently only the support of http2

Check-list

  • PR size
    • Under 300 lines ✅
    • Can't be split without complicating the process even more
  • Tests
    • Added
    • Not applicable (why?)
  • Doc
    • Added
    • Not applicable
  • Reviewer
    • Asked for a review
    • Added label DO NOT REVIEW

@thboileau thboileau requested a review from jlouvel April 6, 2025 14:28
@thboileau thboileau changed the base branch from 2.5 to 2.6 April 6, 2025 14:28
@thboileau thboileau force-pushed the 2.6_jetty-http2-http3 branch from e15ec0d to 7d30753 Compare April 6, 2025 14:43
Copy link
Copy Markdown
Collaborator

@jlouvel jlouvel left a comment

Choose a reason for hiding this comment

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

Looks great overall, a few minor comments added

* <tr>
* <td>http.transport.mode</td>
* <td>string</td>
* <td>http1</td>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@thboileau Could we use "HTTP1_1" and "HTTP2" for consistency with the protocol version and client-side parameter?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure, thanks

* <tr>
* <td>http.transport.protocols</td>
* <td>string</td>
* <td>http1</td>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same comment about "HTTP1_1"

* @return a keystore.
* @throws Exception
*/
private static KeyStore loadKeyStore(String path, String provider, String type, char[] password) throws Exception {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could this method be non static and protected to allow overridding?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure, thanks


@Override
public void stop() throws Exception {
getLogger().info("Stopping the Jetty " + getProtocols() + " server on port " + getHelped().getPort());
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hi @jlouvel I see that the number of spaces for a tab is 8 with my Eclipse installation.
Do you see the same?

Copy link
Copy Markdown
Collaborator

@jlouvel jlouvel left a comment

Choose a reason for hiding this comment

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

Looks good to me @thboileau

@thboileau thboileau merged commit 69da540 into 2.6 Apr 26, 2025
1 check passed
@thboileau thboileau deleted the 2.6_jetty-http2-http3 branch April 26, 2025 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants