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

The validation logic for request parameters is disorganized and fragmented #10734

Closed
4 tasks done
Sunrisea opened this issue Jul 5, 2023 · 1 comment · Fixed by #10775
Closed
4 tasks done

The validation logic for request parameters is disorganized and fragmented #10734

Sunrisea opened this issue Jul 5, 2023 · 1 comment · Fixed by #10775
Assignees
Labels
kind/enhancement Category issues or prs related to enhancement. kind/feature type/feature
Milestone

Comments

@Sunrisea
Copy link
Contributor

Sunrisea commented Jul 5, 2023

Is your feature request related to a problem? Please describe.
The current implementation of parameter validation logic for gRPC and HTTP requests is fragmented, with each request processing method implementing its own logic. This approach lacks unity and is difficult to manage. Additionally, modifying the code to reflect changes in parameter validation standards can prove challenging.

Describe the solution you'd like
To simplify parameter validation logic, a request parameter validation layer can be added. This layer can intercept requests using interceptors or filters, extract relevant parameters based on request type, and then use a unified parameter validation method to validate parameters.Different extract methods can be added by spi.
Describe alternatives you've considered
To simplify parameter validation logic, a request parameter validation layer can be added. This layer can intercept requests using interceptors or filters, extract relevant parameters based on request type, and then use a unified parameter validation method to validate parameters.Different extract methods can be added by spi.Users can choose to enable or disable parameter validation through VM parameters.

TODO List

  • Refactor and abstract config request basic info into AbstractConfigRequest
  • Implement the parameter validation utility class
  • Implement grpc server interceptor and gprc param extractors
  • Implement http filters and http param extractors
@KomachiSion KomachiSion added kind/feature type/feature kind/enhancement Category issues or prs related to enhancement. labels Jul 5, 2023
@KomachiSion KomachiSion modified the milestones: 1.4.6, 2.3.0 Jul 5, 2023
@KomachiSion
Copy link
Collaborator

Great idea, Now nacos has some dispersed validate for parameters. If the issue can solved these parameter in front sound great.

BTW, we should set an switch for this check for some version to support the old version and some illegal usage for old users.

KomachiSion pushed a commit that referenced this issue Jul 7, 2023
…0737)

* For #10734,Refactor the AbstractConfigRequest ,move common properties from its subclasses to the parent class,to reduce the number of corresponding parameter extractors by .

* For #10734,add ParamCheckRules,ParamCheckUtils, add  ParamExtractor interface and the abstract classes HttpParamExtractor and RpcParamExtractor, implement the corresponding Manager.

* For #10734,fix codestyle

* For #10734,fix codestyle , move extractor and extractorManager to core directory

* For #10734,fix codestyle

* For #10734,fix dependency of common module

* For #10734,fix codestyle and copyright

* For #10734,fix pom codestyle and copyright

* For #10734,fix pom codestyle and copyright

* For #10734,fix  copyright

* For #10734,fix  copyright

* For #10734,fix bug caused by refactor of AbstractConfigRequest ,add ut test of ParamExtractorManager

* For #10734,fix bug caused by refactor of AbstractConfigRequest

---------

Co-authored-by: zhuoguang <[email protected]>
KomachiSion pushed a commit that referenced this issue Jul 10, 2023
…ctors (#10745)

* For #10734,Implement grpc server interceptor and grpc param extractors

* For #10734,add unit test for grpc server interceptor and grpc param extractors

* For #10734,alter the test case

* For #10734,delete the ConnectionSetupRequestParamExtractor
KomachiSion pushed a commit that referenced this issue Jul 11, 2023
…am extractors (#10758)

* For #10734,Implement grpc server interceptor and grpc param extractors

* For #10734,add unit test for grpc server interceptor and grpc param extractors

* For #10734,alter the test case

* For #10734,delete the ConnectionSetupRequestParamExtractor

* For #10734,add the naming http request param check filter and implement the naming http request param extractors

* For #10734,add unit test for naming http request param extractors

* For #10734,Implement grpc server interceptor and grpc param extractors

* For #10734,add unit test for grpc server interceptor and grpc param extractors

* For #10734,delete the ConnectionSetupRequestParamExtractor

* For #10734,add the naming http request param check filter and implement the naming http request param extractors

* For #10734,add unit test for naming http request param extractors

* For #10734,add the config http request param check filter and implement the config http request param extractors and unit test

* For #10734,add the console http request param check filter and implement the console http request param extractors and unit test

* For #10734,fix code style

* For #10734,alter the logic of exception handle in filter

* For #10734,fix code style
lowezheng added a commit to lowezheng/nacos that referenced this issue Jul 11, 2023
* develop:
  Refactor grpc tls (alibaba#10759)
  dump change check task submit (alibaba#10755)
  [ISSUE alibaba#10734] Implement http request param check filter and http param extractors (alibaba#10758)
  nacos ui doc update to v2 (alibaba#10730)
  fix for react unique key warning (alibaba#10742)
  [ISSUE alibaba#10734] Implement grpc server interceptor and grpc param extractors (alibaba#10745)
  improvements for leave node api and UI (alibaba#10748)
  Delete client version (alibaba#10754)
KomachiSion pushed a commit that referenced this issue Jul 19, 2023
* For #10734,refactor the paramextractor and ParamChecker

* For #10734,alter the rules of ParamCheck

* For #10734,alter the rules of ParamCheck

* For #10734,fix bug

* For #10734,fix bug and alter the ParamCheckRules.java

* For #10734,fix code style

* For #10734,fix the param check rules

* For #10734,implement the server param check config

* For #10734,optimize the logic

* For #10734,optimize the logic

* For #10734,optimize the logic
wukong121 pushed a commit to wukong121/nacos that referenced this issue Aug 4, 2023
…ass (alibaba#10737)

* For alibaba#10734,Refactor the AbstractConfigRequest ,move common properties from its subclasses to the parent class,to reduce the number of corresponding parameter extractors by .

* For alibaba#10734,add ParamCheckRules,ParamCheckUtils, add  ParamExtractor interface and the abstract classes HttpParamExtractor and RpcParamExtractor, implement the corresponding Manager.

* For alibaba#10734,fix codestyle

* For alibaba#10734,fix codestyle , move extractor and extractorManager to core directory

* For alibaba#10734,fix codestyle

* For alibaba#10734,fix dependency of common module

* For alibaba#10734,fix codestyle and copyright

* For alibaba#10734,fix pom codestyle and copyright

* For alibaba#10734,fix pom codestyle and copyright

* For alibaba#10734,fix  copyright

* For alibaba#10734,fix  copyright

* For alibaba#10734,fix bug caused by refactor of AbstractConfigRequest ,add ut test of ParamExtractorManager

* For alibaba#10734,fix bug caused by refactor of AbstractConfigRequest

---------

Co-authored-by: zhuoguang <[email protected]>
wukong121 pushed a commit to wukong121/nacos that referenced this issue Aug 4, 2023
…m extractors (alibaba#10745)

* For alibaba#10734,Implement grpc server interceptor and grpc param extractors

* For alibaba#10734,add unit test for grpc server interceptor and grpc param extractors

* For alibaba#10734,alter the test case

* For alibaba#10734,delete the ConnectionSetupRequestParamExtractor
realJackSun added a commit that referenced this issue Aug 10, 2023
…lop branch (#10942)

* [ISSUE #10734] Implement http request param check filter and http param extractors (#10758)

* For #10734,Implement grpc server interceptor and grpc param extractors

* For #10734,add unit test for grpc server interceptor and grpc param extractors

* For #10734,alter the test case

* For #10734,delete the ConnectionSetupRequestParamExtractor

* For #10734,add the naming http request param check filter and implement the naming http request param extractors

* For #10734,add unit test for naming http request param extractors

* For #10734,Implement grpc server interceptor and grpc param extractors

* For #10734,add unit test for grpc server interceptor and grpc param extractors

* For #10734,delete the ConnectionSetupRequestParamExtractor

* For #10734,add the naming http request param check filter and implement the naming http request param extractors

* For #10734,add unit test for naming http request param extractors

* For #10734,add the config http request param check filter and implement the config http request param extractors and unit test

* For #10734,add the console http request param check filter and implement the console http request param extractors and unit test

* For #10734,fix code style

* For #10734,alter the logic of exception handle in filter

* For #10734,fix code style

* dump change check task submit (#10755)

* dump change check task submit

* delete config nid convert error fix

* fix test case

* checkstyle

* Refactor grpc tls (#10759)

* Move Tls negotiator to GrpcSdkServer.

* use protocol negotiator builder replace directly create.

* use SPI load negotiator and set tls as default negotiator.

* Remove tlsconfig in BaseRpcServer.

* Add some ut.

* For checkstyle.

* fix word spelling in `AuthenticationManagerDelegator` (#10777)

* fix(#10427): When the execution of handleServerRequest() encounters an exception, record the log and throw an exception, then quickly response to the server errResponse (#10770)

* fix(#10585): selectInstances and selectOneHealthyInstance methods, if the parameter subscribe is true. Subscription is required when clientProxy.isSubscribe() is false. (#10805)

* disable check port input when use registered port (#10799)

* Add the handle to overload connection (#10783)

* add the handle to overload connection

* fast return

* [ISSUE #10662]Prometheus-sd add namespace and service api (#10663)

* add apis

* add tests

* fix checkstyle

* param namespace to namespaceId

* namespace to namespaceId

* fix test case

* fix test case

* Service instance should display related color when healthy or unhealthy (#10811)

* fix a couple of invaild props (#10810)

* feat(#10539): When the operation configuration fails, log. (#10804)

* [ISSUE #10744]feat:Add HealthControllerV2 and HealthControllerV2Test (#10786)

* [ISSUE #10744]feat:Add HealthControllerV2 and HealthControllerV2Test

* fix:V2 api return Result

* [ISSUE #10734] Refactor the ParamChecker and ParamExtractor (#10775)

* For #10734,refactor the paramextractor and ParamChecker

* For #10734,alter the rules of ParamCheck

* For #10734,alter the rules of ParamCheck

* For #10734,fix bug

* For #10734,fix bug and alter the ParamCheckRules.java

* For #10734,fix code style

* For #10734,fix the param check rules

* For #10734,implement the server param check config

* For #10734,optimize the logic

* For #10734,optimize the logic

* For #10734,optimize the logic

* Refactor Prometheus Module (#10827)

* Refactor Prometheus Module

* Complete Test Case

* format

* format

* For #10734,fix the param check rule (#10826)

* [ISSUE #10792]When nacos client use endpoint, the registration center should support configuring context-path and cluster-name like the configuration center (#10793)

* Reactor code in datasource-plugin (#10791)

* Reactor code in datasource-plugin

* Fix Abstract Mapper Test Case

* Add Empty Check

* Fix Checkstyle

* fix checkstyle

* fix check style

* fix check style

* Fix CheckStyle

* Fix SQL Blank

* bugfix for PersistentClientOperationServiceImpl log (#10825)

* For #10734,fix the param check rule (#10858)

* fix(#10831): When using the deregisterInstance method to remove one of multiple instances registered by batchRegisterInstance, all instances registered by batchRegisterInstance will be removed. (#10836)

* UnsupportedFeatureError (#10860)

* fix(distro): fix issue#10880. (#10881)

* feat(#5608 && #10223): When the custom instance id is empty, the id will be automatically generated. (#10812)

* fix: test-code branch (#10904)

* add nacos ci

* delete client version of nacos ci

* fix: test-code branch

* console-ui 新增 toml 语言支持,修复之前 properties 主题未生效的问题 (#10896)

* console-ui 新增 toml 语言支持,修复之前 properties 主题未生效的问题

* console-ui 新增 toml 语言支持,修复之前 properties 主题未生效的问题

* 老规矩,要编译一波

* feat(10891): Provide a configuration item for the maximum number of push retries, instead of directly hardcoding it to 50 times in the code. (#10895)

* [ISSUE #10824] Remove udp port param for v1-client (#10914)

* Remove UDP Param

* Fix gRPC client

* fix test case

* fix test case

* fix test case

* fix test case

* Fix login failed when close auth.

---------

Co-authored-by: Sunrisea <[email protected]>
Co-authored-by: nov.lzf <[email protected]>
Co-authored-by: 杨翊 SionYang <[email protected]>
Co-authored-by: ZhangShenao <[email protected]>
Co-authored-by: blake.qiu <[email protected]>
Co-authored-by: Joey777210 <[email protected]>
Co-authored-by: chenyiqin <[email protected]>
Co-authored-by: maoling <[email protected]>
Co-authored-by: DiligenceLai <[email protected]>
Co-authored-by: huhongjie2014 <[email protected]>
Co-authored-by: lu-xiaoshuang <[email protected]>
Co-authored-by: zt9788 <[email protected]>
Co-authored-by: 阿魁 <[email protected]>
Co-authored-by: wuyfee <[email protected]>
Co-authored-by: Darren Luo <[email protected]>
Co-authored-by: xxc <[email protected]>
KomachiSion pushed a commit that referenced this issue Aug 14, 2023
* For #10734,优化实现逻辑,提高扩展性,为动态变更限流规则预留接口

* For #10734,优化实现逻辑,提高扩展性,为动态变更参数校验规则预留接口
KomachiSion pushed a commit that referenced this issue Aug 21, 2023
…tractRequestFilter (#10972)

* For #10734,将grpc接口的参数请求移动到AbstractRequestFilter中实现

* For #10734,补充测试用例

* For #10734,fix code style

* For #10734,fix code style

* For #10734,fix test bug
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Category issues or prs related to enhancement. kind/feature type/feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants