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.

Sign up for GitHub

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

Jump to bottom

Servlet 4 metadata-complete=true should skip introspection of annotations #4506

Closed
janbartel opened this issue Jan 21, 2020 · 2 comments · Fixed by #4587
Closed

Servlet 4 metadata-complete=true should skip introspection of annotations #4506

janbartel opened this issue Jan 21, 2020 · 2 comments · Fixed by #4587
Assignees
@janbartel
Labels
Specification For all industry Specifications (IETF / Servlet / etc)

Comments

@janbartel
Copy link
Contributor

jetty-10

jakartaee/servlet#144

Servlet Spec 4, Changes Since 3.1, pg A-203:

Clarify metadata-complete in Section 8.1, “Annotations and pluggability”.

This clarification states that if metadata-complete==true, any annotation that has an equivalent in web.xml must be ignored. This includes all annotations for which jetty introspects on a servlet, filter or listener such as PostConstruct, PreDestroy, Resource, Resources, DeclaresRoles, ServletSecurity, MultipartConfig.
@janbartel janbartel added the Specification For all industry Specifications (IETF / Servlet / etc) label Jan 21, 2020
@janbartel janbartel self-assigned this Jan 21, 2020
janbartel added a commit that referenced this issue Jan 31, 2020
@janbartel
Issue #4506 Do not introspect annotations on metadata-complete fragments
ac4cfb0 
Signed-off-by: Jan Bartel <janb@webtide.com>
@olamy
Copy link
Member

olamy commented Feb 5, 2020

related to #4234?

@janbartel
Copy link
Contributor Author

@olamy yes I believe so. Your issue states that if metadata-complete==true, then ServletSecurity should never be processed. According to the new wording of the servlet spec, that isn't true - when you programmatically add a servlet using the ServletContext.createServlet methods, then you always have to introspect it for ServletSecurity/Multipart/PostConstruct/PreDestroy etc annotations regardless of what metadata-complete says. My reading of the spec is:

  1. web.xml metadata-complete==true: no discovered annotations, no introspection on descriptor defined servlets/filters/listeners but yes on programmatically created servlets/filters/listeners

  2. web.xml metadata-complete==false, web-fragment.xml metadata-complete==true: no scan for discovered annotations on that fragment jar, no introspection on servlets/filters/listeners defined in that descriptor but yes on programmatically created servlets/filters/listeners.

  3. web.xml metadata-complete==false, web-fragment.xml metadata-complete==false or no fragment descriptor: scan for discovered annotations on that fragment jar, introspect servlets/filters/listeners defined via annotation or descriptor or programatically.

This issue is for cases 1 && 2: we have been introspecting servlets/filters/listeners defined in web.xml or web-fragment.xml with metadata-complete==true.

janbartel added a commit that referenced this issue Feb 5, 2020
@janbartel
Issue #4506 WIP
5f538d1 
Signed-off-by: Jan Bartel <janb@webtide.com>
janbartel added a commit that referenced this issue Feb 6, 2020
@janbartel
Issue #4506 WIP
5165591 
Signed-off-by: Jan Bartel <janb@webtide.com>
janbartel added a commit that referenced this issue Feb 6, 2020
@janbartel
Issue #4506 WIP
f2a2c27
janbartel added a commit that referenced this issue Feb 17, 2020
@janbartel
Issue #4506 fix test
2012ac7 
Signed-off-by: Jan Bartel <janb@webtide.com>
janbartel added a commit that referenced this issue Feb 17, 2020
@janbartel
Issue #4506 fix checkstyle
e2f14c3 
Signed-off-by: Jan Bartel <janb@webtide.com>
janbartel added a commit that referenced this issue Feb 18, 2020
@janbartel
Issue #4506 Fix javadoc
daa77e9 
Signed-off-by: Jan Bartel <janb@webtide.com>
@janbartel janbartel mentioned this issue Feb 18, 2020
Merged
janbartel added a commit that referenced this issue Feb 25, 2020
@janbartel
Issue #4506 Changes post review.
8420b24 
Signed-off-by: Jan Bartel <janb@webtide.com>
janbartel added a commit that referenced this issue Mar 9, 2020
@janbartel
Jetty 10.0.x 4506 metadata complete (#4587)
dfd3875 
* Issue #4506 Do not introspect annotations on metadata-complete fragments

Signed-off-by: Jan Bartel <janb@webtide.com>
viv added a commit to viv/Openfire that referenced this issue May 16, 2023
@viv
WIP: Upgrades Jetty 9.4.43.v20210629 to 10.0.15
2c0f741 
Remaining Issues:
- JSP compilation failure caused by:

apache/tomcat@224fb6c#diff-ef02fd9c3f90ca4838d3cdaa051489556eca75d537fc396022e0ed456c24d895R329

- `WebSocketClientConnectionHandler.onConnect` assumes that `session.getRemoteAddress()` supplies an `InetSocketAddress` which might not always be true. What's the best approach in our context?

Fixed in this commit:
- Websocket server Maven artifact renamed from `websocket-server` to `websocket-jetty-server`
- `WebSocketServlet` renamed to `JettyWebSocketServlet`
- `WebSocketServletFactory` renamed to `JettyWebSocketServletFactory`

See: https://www.eclipse.org/jetty/documentation/jetty-10/programming-guide/index.html#pg-migration-94-to-10

- `GzipHandler` is no longer part of `ServletContextHandler`
jetty/jetty.project#4765

- Renamed `setWebInfClassesDirs` to `setWebInfClassesResources`
jetty/jetty.project#4506

- Split `SslContextFactory` into Client and Server
jetty/jetty.project#3464
@viv viv mentioned this issue May 16, 2023
Merged
viv added a commit to viv/Openfire that referenced this issue Sep 4, 2023
@viv
WIP: Upgrades Jetty 9.4.43.v20210629 to 10.0.15
5e9befe 
Remaining Issues:
- JSP compilation failure caused by:

apache/tomcat@224fb6c#diff-ef02fd9c3f90ca4838d3cdaa051489556eca75d537fc396022e0ed456c24d895R329

- `WebSocketClientConnectionHandler.onConnect` assumes that `session.getRemoteAddress()` supplies an `InetSocketAddress` which might not always be true. What's the best approach in our context?

Fixed in this commit:
- Websocket server Maven artifact renamed from `websocket-server` to `websocket-jetty-server`
- `WebSocketServlet` renamed to `JettyWebSocketServlet`
- `WebSocketServletFactory` renamed to `JettyWebSocketServletFactory`

See: https://www.eclipse.org/jetty/documentation/jetty-10/programming-guide/index.html#pg-migration-94-to-10

- `GzipHandler` is no longer part of `ServletContextHandler`
jetty/jetty.project#4765

- Renamed `setWebInfClassesDirs` to `setWebInfClassesResources`
jetty/jetty.project#4506

- Split `SslContextFactory` into Client and Server
jetty/jetty.project#3464
guusdk pushed a commit to igniterealtime/Openfire that referenced this issue Sep 7, 2023
@viv @guusdk
WIP: Upgrades Jetty 9.4.43.v20210629 to 10.0.15
09c12d2 
Remaining Issues:
- JSP compilation failure caused by:

apache/tomcat@224fb6c#diff-ef02fd9c3f90ca4838d3cdaa051489556eca75d537fc396022e0ed456c24d895R329

- `WebSocketClientConnectionHandler.onConnect` assumes that `session.getRemoteAddress()` supplies an `InetSocketAddress` which might not always be true. What's the best approach in our context?

Fixed in this commit:
- Websocket server Maven artifact renamed from `websocket-server` to `websocket-jetty-server`
- `WebSocketServlet` renamed to `JettyWebSocketServlet`
- `WebSocketServletFactory` renamed to `JettyWebSocketServletFactory`

See: https://www.eclipse.org/jetty/documentation/jetty-10/programming-guide/index.html#pg-migration-94-to-10

- `GzipHandler` is no longer part of `ServletContextHandler`
jetty/jetty.project#4765

- Renamed `setWebInfClassesDirs` to `setWebInfClassesResources`
jetty/jetty.project#4506

- Split `SslContextFactory` into Client and Server
jetty/jetty.project#3464
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@janbartel janbartel

Labels
Specification For all industry Specifications (IETF / Servlet / etc)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Servlet 4 metadata-complete=true should skip introspection of annotations jetty/jetty.project
2 participants
@olamy @janbartel