Copilot commented on code in PR #2017:
URL: https://github.com/apache/shiro/pull/2017#discussion_r2258388925
##########
web/src/main/java/org/apache/shiro/web/filter/authc/BasicHttpAuthenticationFilter.java:
##########
@@ -74,10 +74,11 @@ public BasicHttpAuthenticationFilter() {
* <p/>
* This implementation:
* <ol><li>acquires the username and password based on the request's
- * {@link #getAuthzHeader(javax.servlet.ServletRequest) authorization
header} via the
- * {@link #getPrincipalsAndCredentials(String,
javax.servlet.ServletRequest) getPrincipalsAndCredentials} method</li>
+ * {@link #getAuthzHeader(jakarta.servlet.ServletRequest) authorization
header} via the
+ * {@link #getPrincipalsAndCredentials(String,
jakarta.servlet.ServletRequest) getPrincipalsAndCredentials} method</li>
* <li>The return value of that method is converted to an
<code>AuthenticationToken</code> via the
- * {@link #createToken(String, String, javax.servlet.ServletRequest,
javax.servlet.ServletResponse) createToken} method</li>
+ * {@link #createToken(String, String, jakarta.servlet.ServletRequest,
jakarta.servlet.ServletResponse)
+ * reateToken} method</li>
Review Comment:
There's a typo in the word 'reateToken' which should be 'createToken'. The
'c' at the beginning of 'createToken' appears to have been accidentally removed.
```suggestion
* createToken} method</li>
```
##########
support/spring/src/main/java/org/apache/shiro/spring/config/AbstractShiroConfiguration.java:
##########
@@ -49,12 +49,14 @@
import org.apache.shiro.session.mgt.eis.SessionDAO;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
+import org.springframework.stereotype.Component;
import java.util.List;
/**
* @since 1.4.0
*/
+@Component
Review Comment:
Adding @Component annotation to AbstractShiroConfiguration may cause
unintended side effects. Abstract configuration classes should typically not be
annotated as components since they are meant to be extended, not instantiated
directly by the container.
```suggestion
import java.util.List;
/**
* @since 1.4.0
*/
```
##########
support/spring/src/main/java/org/apache/shiro/spring/web/ShiroFilterFactoryBean.java:
##########
@@ -18,8 +18,11 @@
*/
package org.apache.shiro.spring.web;
+import jakarta.servlet.ServletRequest;
+import jakarta.servlet.http.HttpServletRequest;
import org.apache.shiro.config.Ini;
import org.apache.shiro.mgt.SecurityManager;
+import org.apache.shiro.spring.web.ee10.ShiroHttpServletRequestEE10;
Review Comment:
The import for ShiroHttpServletRequestEE10 is added but the existing
ShiroHttpServletRequest import on line 43 suggests both classes might be
present. This could cause confusion about which implementation should be used.
```suggestion
```
##########
support/spring/src/main/java/org/apache/shiro/spring/web/ShiroFilterFactoryBean.java:
##########
@@ -595,5 +599,18 @@ protected SpringShiroFilter(WebSecurityManager
webSecurityManager,
setFilterChainResolver(resolver);
}
}
+
+ /**
+ * Wraps the original HttpServletRequest in a {@link
ShiroHttpServletRequest}, which is required for supporting
+ * Servlet Specification behavior backed by a {@link
org.apache.shiro.subject.Subject Subject} instance.
+ *
+ * @param orig the original Servlet Container-provided incoming {@code
HttpServletRequest} instance.
+ * @return {@link ShiroHttpServletRequest ShiroHttpServletRequest}
instance wrapping the original.
+ * @since 1.0
+ */
Review Comment:
The new wrapServletRequest method overrides a protected method but there's
no @Override annotation. This method should be annotated to ensure it properly
overrides the parent implementation.
```suggestion
*/
@Override
```
--
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]
