nacos icon indicating copy to clipboard operation
nacos copied to clipboard

关于authFilter中的ControllerMethodsCache,完全没必要做这个事情

Open 985492783 opened this issue 2 years ago • 18 comments

这是nacos中的ControllerMethodsCache,作用是通过HttpServletRequest获取controller中处理该request的方法,然后获取Secured注解 image

既然已经注入到spring容器中了,为什么不用现成的RequestMappingHandlerMapping方法来处理 image

985492783 avatar Oct 09 '23 17:10 985492783

看了一下DistroFilter同理,都是注入容器中的,单独写一个扫包加处理class是不是太繁琐了,如果没有额外功能应该是能通过第二种方式优化掉的

985492783 avatar Oct 09 '23 17:10 985492783

目的是找到url所映射的controller类的方法, 从而找到该方法是否被特殊注解,并且能获取到这个特殊注解。

之前也想过应该要可以直接用Spring 的mapping,但是一直没有时间做, 如果你有空可以做一下优化, 先找一个注解,比如Distro做一个替换测试,没问题之后再全面替换和删除。

这里有几个比较特殊的点:

  1. 配置中心接口中有使用param来区分方法的情况,需要确认可以实现。
  2. 一些特殊的url写法,也要能够命中方法,比如/xx/xx/,/xx/xx,/xx/xx?,//xx/xx 之类的需要等价,主要是防止绕过鉴权。

KomachiSion avatar Oct 11 '23 01:10 KomachiSion

那完全用RequestMappingHandlerMapping取代ControllerMethodsCache之后,是否要将ControllerMethodsCache该类移除掉呢。我有意来做这个修改工作

目的是找到url所映射的controller类的方法, 从而找到该方法是否被特殊注解,并且能获取到这个特殊注解。

之前也想过应该要可以直接用Spring 的mapping,但是一直没有时间做, 如果你有空可以做一下优化, 先找一个注解,比如Distro做一个替换测试,没问题之后再全面替换和删除。

这里有几个比较特殊的点:

  1. 配置中心接口中有使用param来区分方法的情况,需要确认可以实现。
  2. 一些特殊的url写法,也要能够命中方法,比如/xx/xx/,/xx/xx,/xx/xx?,//xx/xx 之类的需要等价,主要是防止绕过鉴权。

huangkemingyyds avatar Oct 14 '23 14:10 huangkemingyyds

我理解如果平替掉的话就可以移除掉了,包括RequestMappingInfo这些类,都是很老版本的东西了,直接复用spring提供的能力更好

985492783 avatar Oct 14 '23 14:10 985492783

image 在尝试修改成RequestMappingHandlerMapping 获取 Method的时候。 出现了问题。

HandlerExecutionChain executionChain = handlerMapping.getHandler(req); --获取失败

出现了 Expected lookupPath in request attribute "org.springframework.web.util.UrlPathHelper.PATH".报错 源码分析定位 image

结论:spirngBoot版本太高出现,修改失败。 该问题就是源自springboot 2.6.0后的新特性。 @KomachiSion @985492783

huangkemingyyds avatar Oct 15 '23 07:10 huangkemingyyds

我项目里的解决方案是添加spring.mvc.pathmatch.matching-strategy=ant_path_matcher,但是没有深究这两者的区别,如果要替换还得仔细研究研究会不会有不兼容的地方

985492783 avatar Oct 15 '23 07:10 985492783

如果是在项目里使用配置来处理,那么就应该会影响到全局的配置。 个人觉得要自定义拦截器或特定的RequestMappingHandlerMapping实例来处理特定的请求路径。这样可以更细粒度地控制路径匹配策略的应用范围。

huangkemingyyds avatar Oct 15 '23 08:10 huangkemingyyds

如果是在项目里使用配置来处理,那么就应该会影响到全局的配置。 个人觉得要自定义拦截器或特定的RequestMappingHandlerMapping实例来处理特定的请求路径。这样可以更细粒度地控制路径匹配策略的应用范围。

这样实现是否可以 @KomachiSion

huangkemingyyds avatar Oct 18 '23 01:10 huangkemingyyds

没看懂,具体想要怎么做

KomachiSion avatar Oct 18 '23 07:10 KomachiSion

建议PR中的修改:

  1. 先不要彻底移除,添加一个代理层和开关,方便新机制使用过程中有问题快速切换回旧机制。
  2. Any-matcher会不会导致安全问题?

KomachiSion avatar Oct 23 '23 08:10 KomachiSion

建议PR中的修改:

  1. 先不要彻底移除,添加一个代理层和开关,方便新机制使用过程中有问题快速切换回旧机制。
  2. Any-matcher会不会导致安全问题?

第一点:可以修改实现。 第二点:无法评估,这个是springBOot方面的问题。

看是否还要继续修改@KomachiSion

huangkemingyyds avatar Oct 23 '23 10:10 huangkemingyyds

  1. 需要修改,在稳定前有能力立刻切换回旧的ControllerMethodCache, 毕竟这个经历过这么多个版本,稳定性足够。
  2. 调研一下Any-matcher不设置会有什么问题,为什么一定要设置成*, 如果设置成*是否存在相关安全隐患

KomachiSion avatar Oct 30 '23 07:10 KomachiSion

  1. 调研一下Any-matcher不设置会有什么问题,为什么一定要设置成*, 如果设置成*是否存在相关安全隐患

1.如果不设置Any-matcher, 在springBoot版本大于2.6.0. (HttpServletRequest) request 使用 RequestMappingHandlerMapping 获取 method 会有官方的bug出现。 获取不到method 。所有才设置Any-matcher。
2. 安全隐患这方面,具体还不可知,我认为安全隐患不存在于nacos,应该存在于springBoot。(具体还要通过测试)

huangkemingyyds avatar Oct 30 '23 07:10 huangkemingyyds

  1. 调研一下Any-matcher不设置会有什么问题,为什么一定要设置成*, 如果设置成*是否存在相关安全隐患

1.如果不设置Any-matcher, 在springBoot版本大于2.6.0. (HttpServletRequest) request 使用 RequestMappingHandlerMapping 获取 method 会有官方的bug出现。 获取不到method 。所有才设置Any-matcher。 2. 安全隐患这方面,具体还不可知,我认为安全隐患不存在于nacos,应该存在于springBoot。(具体还要通过测试)

我觉得在不明确的情况下, 最好还是不要替换了, 因为即使这个安全隐患存在于spring boot, 但spring boot默认不开启Any-matcher,而nacos开启, 最后就会变成是nacos的安全漏洞,而不是spring boot的。 这点类似于actuator配置路径为*导致的内存dump泄漏等安全问题。

KomachiSion avatar Nov 06 '23 07:11 KomachiSion

  1. 调研一下Any-matcher不设置会有什么问题,为什么一定要设置成*, 如果设置成*是否存在相关安全隐患

1.如果不设置Any-matcher, 在springBoot版本大于2.6.0. (HttpServletRequest) request 使用 RequestMappingHandlerMapping 获取 method 会有官方的bug出现。 获取不到method 。所有才设置Any-matcher。 2. 安全隐患这方面,具体还不可知,我认为安全隐患不存在于nacos,应该存在于springBoot。(具体还要通过测试)

或者等spring boot修复了这个问题,我们升级了对应的修复版本,再进行替换。

KomachiSion avatar Nov 15 '23 08:11 KomachiSion

  1. 调研一下Any-matcher不设置会有什么问题,为什么一定要设置成*, 如果设置成*是否存在相关安全隐患

1.如果不设置Any-matcher, 在springBoot版本大于2.6.0. (HttpServletRequest) request 使用 RequestMappingHandlerMapping 获取 method 会有官方的bug出现。 获取不到method 。所有才设置Any-matcher。 2. 安全隐患这方面,具体还不可知,我认为安全隐患不存在于nacos,应该存在于springBoot。(具体还要通过测试)

或者等spring boot修复了这个问题,我们升级了对应的修复版本,再进行替换。

先不要关闭此话题。我后续保持跟进。

huangkemingyyds avatar Nov 29 '23 06:11 huangkemingyyds

springBoot版本2.6.0之后,Spring默认路径匹配器AntPathMatcher转化为PathMatcher。主要功能上的差异:PathPattern去掉了Ant字样,但保持了很好的向下兼容性:除了不支持将写在path中间**之外,其它的匹配规则从行为上均保持和AntPathMatcher一致,并且还新增了强大的{*pathVariable}的支持。 因此在功能上姑且可认为二者是一致的,极特殊情况下的不兼容除外。

huangkemingyyds avatar Dec 24 '23 03:12 huangkemingyyds

springBoot版本2.6.0之后,Spring默认路径匹配器AntPathMatcher转化为PathMatcher。主要功能上的差异:

  1. 功能差异PathPattern去掉了Ant字样,但保持了很好的向下兼容性:除了不支持将**写在path中间之外,其它的匹配规则从行为上均保持和AntPathMatcher一致,并且还新增了强大的{*pathVariable}的支持。 因此在功能上姑且可认为二者是一致的,极特殊情况下的不兼容除外。 2.性能差异:PathMatcher匹配速度胜于AntPathMatcher。

  2. 给nacos带来的影响:其中,PathMatcher不支持将**写在path中间。将影响到鉴权、安全和路径书写规则。

总结: 改动较大,不建议修改。

@KomachiSion

huangkemingyyds avatar Dec 24 '23 03:12 huangkemingyyds