afs commented on code in PR #4038:
URL: https://github.com/apache/jena/pull/4038#discussion_r3529044449


##########
jena-fuseki2/jena-fuseki-main/src/main/java/org/apache/jena/fuseki/mod/shiro/FMod_Shiro.java:
##########
@@ -144,83 +157,73 @@ public void prepare(FusekiServer.Builder serverBuilder, 
Set<String> datasetNames
         if ( shiroConfig == null )
             return;
 
-        // Shiro prints stack traces and exceptions. Rather than silence
-        //checkShiroConfiguration(shiroConfig);
-
         // Shiro-style.
         String shiroResourceName = FusekiShiro.withResourcePrefix(shiroConfig);
         if ( ! ResourceUtils.resourceExists(shiroResourceName) )
             throw new FusekiConfigException("Shiro resource does not exist");
-        Filter filter = new FusekiShiroFilter(shiroResourceName);
+
+        Filter filter = new FusekiShiroFilter(shiroResourceName, 
DISABLE_SESSIONS);
         // This is a "before" filter.
         serverBuilder.addFilter("/*", filter);
         serverBuilder.setServletAttribute(Fuseki.attrShiroResource, 
shiroResourceName);
-
         // Clear.
         this.argShiroFile = null;
     }
 
-    /**
-     * FusekiShiroFilter, includes Shiro initialization. Fuseki is a
-     * not a webapp so it needs to trigger off servlet initialization.
-     */
-    private static class FusekiShiroFilter extends ShiroFilter {
-
-        private final String shiroInitializationResource;
-
-        FusekiShiroFilter(String shiroResourceName) {
-            // Shiro file -- URLs are "file:<no encoding>"
-            shiroInitializationResource = shiroResourceName;
-        }
-
-        @Override
-        public void init() throws Exception {
-            try {
-                // Intercept Shiro initialization.
-                List<String> locations = List.of();
-                if ( shiroInitializationResource != null ) {
-                    locations = List.of(shiroInitializationResource);
-                }
-                FusekiShiro.shiroEnvironment(getServletContext(), locations);
-            } catch (FusekiException ex) {
-                // Wrap so ShiroFilter does not log it.
-                throw new ServletException(ex);
-            }
-            super.init();
-        }
-    }
-
     @Override
     public void serverBeforeStarting(FusekiServer server) {
         try {
             String x 
=(String)server.getServletContext().getAttribute(Fuseki.attrShiroResource);
             if ( x != null )
                 FmtLog.info(shiroConfigLog, "Shiro configuration: %s", x);
-//            else
-//                FmtLog.info(shiroConfigLog, "No Shiro configuration");
+            //            else
+            //                FmtLog.info(shiroConfigLog, "No Shiro 
configuration");
         } catch (ClassCastException ex) {
             FmtLog.warn(shiroConfigLog, "Unexpected Shiro configuration: %s", 
server.getServletContext().getAttribute(Fuseki.attrShiroResource));
         }
 
-        // Shiro requires a session handler.
-        // This needs the Jetty server to have been created.
-        org.eclipse.jetty.server.Server jettyServer = server.getJettyServer();
-        try {
-            ServletContextHandler servletContextHandler = 
(ServletContextHandler)(jettyServer.getHandler());
-            if ( servletContextHandler.getSessionHandler() == null ) {
-                SessionHandler sessionsHandler = new SessionHandler();
-                servletContextHandler.setHandler(sessionsHandler);
+        if ( ! DISABLE_SESSIONS ) {
+            // If shiro requires a session handler.
+            // This needs the Jetty server to have been created.
+            // Allocate a Jakarta SessionHandler
+            try {
+                org.eclipse.jetty.server.Server jettyServer = 
server.getJettyServer();
+
+                ServletContextHandler servletContextHandler = 
(ServletContextHandler)(jettyServer.getHandler());
+                if ( servletContextHandler.getSessionHandler() == null ) {
+                    SessionHandler sessionHandler = new SessionHandler();

Review Comment:
   Fuseki is using only one part of Shiro's web support - the HTTP 
authentication and URL pattern matching. Sessions seem to be motivated by 
webapp sessions in JakartaEE.  It is the linkage of Shiro Sessions to HTTP 
Sessions that seems to be the problem.
   
   The Shiro filter code is in `shiro-web-jakarata` which brings in JarakaEE 
related code which Fuseki does not need.
   
   I tried with timeouts on HTTP Sessions and adding a session listener that 
log sessions being created every request, but wasn't seeing sessions being 
released. I didn't investigate too deeply because I think the long term is no 
Shiro sessions (Fuseki does not need them) and so strip out this code bock. 
DISABLE_SESSIONS is , I hope, temporary. The cost is adding a servlet attribute 
to each request which seems crude but that approach does not navigating Shiro 
initialization code and so it is hopefully more stable.
   
   The Jetty server can be configured and may be some other future feature 
wants HTTP sessions. It is free to add them without interference.
   
   BTW Good thing you commented - I'd left some development code in the PR.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to