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

Review Jetty XML syntax to allow calling JDK methods #8913

Closed
sbordet opened this issue Nov 18, 2022 · 0 comments · Fixed by #8915
Closed

Review Jetty XML syntax to allow calling JDK methods #8913

sbordet opened this issue Nov 18, 2022 · 0 comments · Fixed by #8915
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@sbordet
Copy link
Contributor

sbordet commented Nov 18, 2022

Jetty version(s)
10+

Description
Jetty XML syntax allow to call methods of any class via reflection.

However, when calling JDK methods, they may return JDK-private implementation classes that are not accessible from Jetty code.
This surfaced in particular while working on #8863, where a new Jetty module for the thread pool that supports virtual threads need to call JDK methods that return JDK-private classes.

For that example, we have:

<!-- returns JDK private class ThreadBuilders.VirtualThreadBuilder -->
<Call class="java.lang.Thread" name="ofVirtual">
  <!-- Throws IllegalAccessException: class org.eclipse.jetty.xml.XmlConfiguration$JettyXmlConfiguration 
cannot access a member of class java.lang.ThreadBuilders$VirtualThreadBuilder (in module java.base) 
with modifiers "public volatile" -->
  <Call name="name">virtual</Call>
</Call>

The JDK-private class ThreadBuilders.VirtualThreadBuilder is not accessible, but its JDK-public ancestor Thread.Builder is.

The proposal is to change slightly the semantic of the Call element to support this:

<!-- static invocation -->
<Call class="java.lang.Thread" name="ofVirtual">
  <!-- virtual invocation -->
  <Call class="java.lang.Thread$Builder" name="name">virtual</Call>
</Call>

The second Call element specifies the class attribute that should be used to reflect the method named name but it performs a virtual call, not a static call (because it is in the context of an enclosing Call element).

Ambiguities are easily resolved because it's always possible to ask a Method if it is static and if so perform the invocation passing null as the target of the invocation.

@sbordet sbordet added the Bug For general bugs on Jetty side label Nov 18, 2022
@sbordet sbordet self-assigned this Nov 18, 2022
sbordet added a commit that referenced this issue Nov 18, 2022
Now <Call>, <Get> and <Set> elements can use the "class" attribute
to specify the exact class to perform method lookup.

Improved support for <Property>, <SystemProperty> and <Env> so that
attribute "name" is now optional (as specified in the DTD), and a
"deprecated" attribute may be present instead.
This is necessary to terminally deprecate properties that have
no replacement.

Signed-off-by: Simone Bordet <[email protected]>
sbordet added a commit that referenced this issue Nov 18, 2022
…8915)

* Fixes #8913 - Review Jetty XML syntax to allow calling JDK methods

Now `<Call>`, `<Get>` and `<Set>` elements can use the `class` attribute
to specify the exact class to perform method lookup.

Improved support for `<Property>`, `<SystemProperty>` and `<Env>` so that
attribute `name` is now optional (as specified in the DTD), and a
`deprecated` attribute may be present instead.
This is necessary to terminally deprecate properties that have
no replacement.

Signed-off-by: Simone Bordet <[email protected]>
sbordet added a commit that referenced this issue Nov 18, 2022
Now <Call>, <Get> and <Set> elements can use the "class" attribute
to specify the exact class to perform method lookup.

Improved support for <Property>, <SystemProperty> and <Env> so that
attribute "name" is now optional (as specified in the DTD), and a
"deprecated" attribute may be present instead.
This is necessary to terminally deprecate properties that have
no replacement.

Signed-off-by: Simone Bordet <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant