Copilot commented on code in PR #7075: URL: https://github.com/apache/shenyu/pull/7075#discussion_r4032851851
########## shenyu-plugin/shenyu-plugin-logging/shenyu-plugin-logging-console/src/main/java/org/apache/shenyu/plugin/logging/console/entity/LoggingConsoleRuleHandle.java: ########## @@ -0,0 +1,92 @@ +/* + * 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.logging.console.entity; + +import org.apache.commons.lang3.StringUtils; +import org.apache.shenyu.plugin.logging.common.entity.CommonLoggingRuleHandle; +import org.apache.shenyu.plugin.logging.desensitize.api.enums.DataDesensitizeEnum; +import org.apache.shenyu.plugin.logging.desensitize.api.matcher.KeyWordMatch; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Objects; + +/** + * Immutable logging console rule configuration. + */ +public final class LoggingConsoleRuleHandle { + + private final boolean desensitized; + + private final String keyword; + + private final String dataDesensitizeAlg; + + private final KeyWordMatch keyWordMatch; + + /** + * Create a logging console rule configuration. + * + * @param ruleHandle common logging rule configuration + */ + public LoggingConsoleRuleHandle(final CommonLoggingRuleHandle ruleHandle) { + this.keyword = ruleHandle.getKeyword(); + this.desensitized = StringUtils.isNotBlank(keyword) && Boolean.TRUE.equals(ruleHandle.getMaskStatus()); + this.dataDesensitizeAlg = Objects.nonNull(ruleHandle.getMaskType()) + ? ruleHandle.getMaskType() : DataDesensitizeEnum.MD5_ENCRYPT.getDataDesensitizeAlg(); + this.keyWordMatch = new KeyWordMatch(StringUtils.isBlank(keyword) + ? Collections.emptySet() : new HashSet<>(Arrays.asList(keyword.split(";")))); Review Comment: This now compiles configured keywords even when masking is disabled. Previously matcher construction only happened when `maskStatus` was true; because `KeyWordMatch` inserts each keyword directly into `Pattern.compile`, a disabled rule containing a malformed pattern such as `[` will now throw from `handlerRule` and can interrupt configuration synchronization. Use the empty matcher whenever this snapshot is not desensitized. ########## shenyu-plugin/shenyu-plugin-logging/shenyu-plugin-logging-console/src/main/java/org/apache/shenyu/plugin/logging/console/LoggingConsolePlugin.java: ########## @@ -76,36 +74,35 @@ public class LoggingConsolePlugin extends AbstractShenyuPlugin { private static final Logger LOG = LoggerFactory.getLogger(LoggingConsolePlugin.class); - - private static String dataDesensitizeAlg = DataDesensitizeEnum.CHARACTER_REPLACE.getDataDesensitizeAlg(); + + private static final KeyWordMatch EMPTY_KEY_WORD_MATCH = new KeyWordMatch(Collections.emptySet()); @Override protected Mono<Void> doExecute(final ServerWebExchange exchange, final ShenyuPluginChain chain, final SelectorData selector, final RuleData rule) { - CommonLoggingRuleHandle commonLoggingRuleHandle = LoggingConsolePluginDataHandler.CACHED_HANDLE.get().obtainHandle(CacheKeyUtils.INST.getKey(rule)); - Set<String> keywordSets = Sets.newHashSet(); + String cacheKey = CacheKeyUtils.INST.getKey(rule); + LoggingConsoleRuleHandle ruleHandle = LoggingConsolePluginDataHandler.CACHED_HANDLE.get().obtainHandle(cacheKey); boolean desensitized = Boolean.FALSE; - KeyWordMatch keyWordMatch = new KeyWordMatch(Collections.emptySet()); - if (Objects.nonNull(commonLoggingRuleHandle)) { - String keywords = commonLoggingRuleHandle.getKeyword(); - desensitized = StringUtils.isNotBlank(keywords) && commonLoggingRuleHandle.getMaskStatus(); - if (desensitized) { - Collections.addAll(keywordSets, keywords.split(";")); - dataDesensitizeAlg = Optional.ofNullable(commonLoggingRuleHandle.getMaskType()).orElse(DataDesensitizeEnum.MD5_ENCRYPT.getDataDesensitizeAlg()); - keyWordMatch = new KeyWordMatch(keywordSets); - LOG.info("current plugin:{}, keyword:{}, dataDesensitizedAlg:{}", this.named(), keywords, dataDesensitizeAlg); - } + KeyWordMatch keyWordMatch = EMPTY_KEY_WORD_MATCH; + String dataDesensitizeAlg = DataDesensitizeEnum.CHARACTER_REPLACE.getDataDesensitizeAlg(); + if (Objects.nonNull(ruleHandle) && ruleHandle.isDesensitized()) { + desensitized = true; + dataDesensitizeAlg = ruleHandle.getDataDesensitizeAlg(); Review Comment: The added tests cover cache reuse/removal but not the actual algorithm-isolation regression: no test executes two snapshots with different `maskType` values through the request/response decorators. Add an interleaved test using MD5 and character replacement rules and assert that each logged body retains its selected algorithm; otherwise the cross-request leak can recur undetected. -- 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]
