Aias00 commented on code in PR #7153: URL: https://github.com/apache/shenyu/pull/7153#discussion_r4059856486
########## shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-sensitive-word/src/main/java/org/apache/shenyu/plugin/ai/sensitive/word/SensitiveWordPlugin.java: ########## @@ -0,0 +1,148 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.plugin.ai.sensitive.word; + +import org.apache.shenyu.common.dto.RuleData; +import org.apache.shenyu.common.dto.SelectorData; +import org.apache.shenyu.common.dto.convert.rule.SensitiveWordHandle; +import org.apache.shenyu.common.enums.PluginEnum; +import org.apache.shenyu.plugin.api.ShenyuPluginChain; +import org.apache.shenyu.plugin.api.exception.ResponsiveException; +import org.apache.shenyu.plugin.api.utils.WebFluxResultUtils; +import org.apache.shenyu.plugin.base.AbstractShenyuPlugin; +import org.apache.shenyu.plugin.base.utils.CacheKeyUtils; +import org.apache.shenyu.plugin.base.utils.ServerWebExchangeUtils; +import org.apache.shenyu.plugin.ai.sensitive.word.ac.AhoCorasick; +import org.apache.shenyu.plugin.ai.sensitive.word.handler.SensitiveWordPluginDataHandler; +import org.apache.shenyu.plugin.ai.sensitive.word.handler.SensitiveWordPluginDataHandler.CachedDictionary; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.data.redis.core.ReactiveRedisTemplate; +import org.springframework.http.codec.HttpMessageReader; +import org.springframework.web.server.ServerWebExchange; +import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; + +import java.util.List; +import java.util.Objects; + +/** + * The sensitive word plugin, it rejects the request when its body contains a word of the dictionary + * configured for the matched rule. + * + * <p>The dictionary is read from a redis set, so it can be maintained outside of shenyu. A loaded + * dictionary is reused for the refresh interval of the rule handle, see {@link SensitiveWordHandle}. + */ +public class SensitiveWordPlugin extends AbstractShenyuPlugin { + + private static final Logger LOG = LoggerFactory.getLogger(SensitiveWordPlugin.class); + + /** + * The error code returned to the client when the request contains sensitive words. + */ + private static final int SENSITIVE_WORD_CODE = 1500; + + private final List<HttpMessageReader<?>> readers; + + public SensitiveWordPlugin(final List<HttpMessageReader<?>> readers) { + this.readers = readers; + } + + @Override + protected Mono<Void> doExecute(final ServerWebExchange exchange, + final ShenyuPluginChain chain, + final SelectorData selector, + final RuleData rule) { + SensitiveWordHandle handle = SensitiveWordPluginDataHandler.CACHED_HANDLE.get() + .obtainHandle(CacheKeyUtils.INST.getKey(rule)); + if (Objects.isNull(handle)) { + return chain.execute(exchange); + } + ReactiveRedisTemplate<String, String> redisTemplate = SensitiveWordPluginDataHandler.REDIS_TEMPLATES.get() + .obtainHandle(SensitiveWordPluginDataHandler.PLUGIN_NAME); + if (Objects.isNull(redisTemplate)) { + LOG.warn("sensitive word plugin: the redis template is not initialized, skip the sensitive word check"); + return chain.execute(exchange); + } + return ServerWebExchangeUtils.rewriteRequestBody(exchange, readers, + body -> check(exchange, redisTemplate, handle, body)) + .flatMap(chain::execute) + .onErrorResume(error -> { + if (error instanceof ResponsiveException) { + return WebFluxResultUtils.failedResult((ResponsiveException) error); + } + return Mono.error(error); + }); + } + + private Mono<String> check(final ServerWebExchange exchange, + final ReactiveRedisTemplate<String, String> redisTemplate, + final SensitiveWordHandle handle, + final String body) { + return dictionary(redisTemplate, handle) + .map(automaton -> automaton.search(body)) Review Comment: Blocking: this scan runs on the event loop. Only the redis path hops off it (`subscribeOn(Schedulers.boundedElastic())` further down); the cached path returns `Mono.just(cached.getAutomaton())` with no scheduler, which is the path every request takes after the first one. Nothing here switches threads, so `automaton.search(body)` executes on the subscribing thread - the Netty event loop of the request currently being filtered - over the entire request body. That contradicts point 4 of the PR description ("the automaton is compiled on a bounded elastic thread, never inside the request thread"): compilation is off-loop, but execution is on-loop, per request. `search(...)` is also O(n * failure-chain depth) since every character walks the failure link chain. Suggestion: hop once for both paths, e.g. return dictionary(redisTemplate, handle) .publishOn(Schedulers.boundedElastic()) .map(automaton -> automaton.search(body)); (or put the `publishOn` inside `dictionary()` so both branches are covered). Precomputing an output list per node while building the failure links would additionally turn search into plain O(n). ########## shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-sensitive-word/src/main/java/org/apache/shenyu/plugin/ai/sensitive/word/SensitiveWordPlugin.java: ########## @@ -0,0 +1,148 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.shenyu.plugin.ai.sensitive.word; + +import org.apache.shenyu.common.dto.RuleData; +import org.apache.shenyu.common.dto.SelectorData; +import org.apache.shenyu.common.dto.convert.rule.SensitiveWordHandle; +import org.apache.shenyu.common.enums.PluginEnum; +import org.apache.shenyu.plugin.api.ShenyuPluginChain; +import org.apache.shenyu.plugin.api.exception.ResponsiveException; +import org.apache.shenyu.plugin.api.utils.WebFluxResultUtils; +import org.apache.shenyu.plugin.base.AbstractShenyuPlugin; +import org.apache.shenyu.plugin.base.utils.CacheKeyUtils; +import org.apache.shenyu.plugin.base.utils.ServerWebExchangeUtils; +import org.apache.shenyu.plugin.ai.sensitive.word.ac.AhoCorasick; +import org.apache.shenyu.plugin.ai.sensitive.word.handler.SensitiveWordPluginDataHandler; +import org.apache.shenyu.plugin.ai.sensitive.word.handler.SensitiveWordPluginDataHandler.CachedDictionary; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.data.redis.core.ReactiveRedisTemplate; +import org.springframework.http.codec.HttpMessageReader; +import org.springframework.web.server.ServerWebExchange; +import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; + +import java.util.List; +import java.util.Objects; + +/** + * The sensitive word plugin, it rejects the request when its body contains a word of the dictionary + * configured for the matched rule. + * + * <p>The dictionary is read from a redis set, so it can be maintained outside of shenyu. A loaded + * dictionary is reused for the refresh interval of the rule handle, see {@link SensitiveWordHandle}. + */ +public class SensitiveWordPlugin extends AbstractShenyuPlugin { + + private static final Logger LOG = LoggerFactory.getLogger(SensitiveWordPlugin.class); + + /** + * The error code returned to the client when the request contains sensitive words. + */ + private static final int SENSITIVE_WORD_CODE = 1500; + + private final List<HttpMessageReader<?>> readers; + + public SensitiveWordPlugin(final List<HttpMessageReader<?>> readers) { + this.readers = readers; + } + + @Override + protected Mono<Void> doExecute(final ServerWebExchange exchange, + final ShenyuPluginChain chain, + final SelectorData selector, + final RuleData rule) { + SensitiveWordHandle handle = SensitiveWordPluginDataHandler.CACHED_HANDLE.get() + .obtainHandle(CacheKeyUtils.INST.getKey(rule)); + if (Objects.isNull(handle)) { + return chain.execute(exchange); + } + ReactiveRedisTemplate<String, String> redisTemplate = SensitiveWordPluginDataHandler.REDIS_TEMPLATES.get() + .obtainHandle(SensitiveWordPluginDataHandler.PLUGIN_NAME); + if (Objects.isNull(redisTemplate)) { + LOG.warn("sensitive word plugin: the redis template is not initialized, skip the sensitive word check"); + return chain.execute(exchange); + } + return ServerWebExchangeUtils.rewriteRequestBody(exchange, readers, + body -> check(exchange, redisTemplate, handle, body)) + .flatMap(chain::execute) + .onErrorResume(error -> { + if (error instanceof ResponsiveException) { + return WebFluxResultUtils.failedResult((ResponsiveException) error); + } + return Mono.error(error); + }); + } + + private Mono<String> check(final ServerWebExchange exchange, + final ReactiveRedisTemplate<String, String> redisTemplate, + final SensitiveWordHandle handle, + final String body) { + return dictionary(redisTemplate, handle) + .map(automaton -> automaton.search(body)) + .flatMap(matches -> { + if (matches.isEmpty()) { + return Mono.just(body); + } + return Mono.error(new ResponsiveException(SENSITIVE_WORD_CODE, + String.format("The request contains sensitive words: %s", matches), exchange)); Review Comment: Blocking: this echoes the matched dictionary words back to the caller. `matches` goes into the `ResponsiveException` message and reaches the client through `WebFluxResultUtils#failedResult`; it will also be captured by the logging plugins and any access log. For a compliance filter that is the wrong direction: - it hands the blacklisted words back to the requester, turning the gateway into an oracle that confirms what is on the list; - it re-emits content the plugin has just classified as forbidden, including into logs (where `shenyu-plugin-logging-*` desensitization cannot know these words). Suggestion: return a generic message to the client (for example "request rejected: sensitive content detected") and log the matches server-side only. -- 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]
