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

Issue #4691 - Use MethodHandles.lookup() consistently in WebSocket code. #4692

Merged

Conversation

sbordet
Copy link
Contributor

@sbordet sbordet commented Mar 22, 2020

  • Using MethodHandles.publicLookup().in(...) instead of privateLookupIn(...).
  • Moved test WebSocket EndPoint classes into the proper package, otherwise
    they would not be accessible because the test package is not exported.

Signed-off-by: Simone Bordet simone.bordet@gmail.com

#4691

* Using MethodHandles.publicLookup().in(...) instead of privateLookupIn(...).
* Moved test WebSocket EndPoint classes into the proper package, otherwise
  they would not be accessible because the test package is not exported.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet requested a review from lachlan-roberts March 22, 2020 15:42
@sbordet
Copy link
Contributor Author

sbordet commented Mar 22, 2020

@lachlan-roberts please review. Note that this is the minimal change needed for 10.0.0.alpah2, but the issue is not completed because MethodHandles.lookup() usages really need to be consistent and well documented.

You want to keep the issue open, merge this PR, but create another PR to fix the issue.

Copy link
Contributor

@lachlan-roberts lachlan-roberts left a comment

Choose a reason for hiding this comment

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

LGTM.
I will remove the throws InvalidWebSocketException and then merge.

@@ -706,16 +706,7 @@ private void assertSignatureValid(Class<?> endpointClass, Method method, Class<?

private MethodHandles.Lookup getMethodHandleLookup(Class<?> endpointClass) throws InvalidWebSocketException
Copy link
Contributor

Choose a reason for hiding this comment

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

Can remove the throws InvalidWebSocketException now.

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@lachlan-roberts lachlan-roberts merged commit d3567fc into jetty-10.0.x Mar 23, 2020
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