smolnar82 commented on code in PR #1497:
URL: https://github.com/apache/knox/pull/1497#discussion_r4538422816
##########
gateway-provider-rewrite-common/src/main/java/org/apache/knox/gateway/filter/rewrite/impl/UrlRewriteFilterApplyDescriptorImpl.java:
##########
@@ -44,4 +45,23 @@ public String getRule() {
return rule;
}
+ @Override
+ public boolean find() {
+ return find;
+ }
+
+ @Override
+ public UrlRewriteFilterApplyDescriptor find( boolean find ) {
+ this.find = find;
+ return this;
+ }
+
+ public void setFind( boolean find ) {
Review Comment:
`find` is populated via digester's `SetPropertiesRule` / BeanUtils, which
silently converts an invalid value to `false` (verified:
`ConvertUtils.convert("ture", boolean.class)` returns `false` with beanutils
1.11.0). So a typo like `find="ture"` quietly disables the rewrite with no
error or log. Not a blocker, but it may be worth validating the attribute (only
`true`/`false` accepted) or at least logging a warning when loading the rules.
##########
gateway-provider-rewrite/src/main/java/org/apache/knox/gateway/filter/rewrite/impl/UrlRewriteUtil.java:
##########
@@ -60,13 +60,46 @@ public static UrlRewriteFilterContentDescriptor
getRewriteFilterConfig(
public static String filterJavaScript( String inputValue,
UrlRewriteFilterContentDescriptor config,
UrlRewriteFilterReader filterReader,
UrlRewriteFilterPathDescriptor.Compiler<Pattern> regexCompiler ) {
+ return filterMatches( inputValue, config, filterReader, regexCompiler,
false );
+ }
+
+ /**
+ * Rewrites every match of the applies that are configured with find, the
rest of the input is left untouched.
+ * Applies without find have to match a whole value, they are not used here.
+ */
+ public static String filterFind( String inputValue,
UrlRewriteFilterContentDescriptor config,
+ UrlRewriteFilterReader filterReader,
UrlRewriteFilterPathDescriptor.Compiler<Pattern> regexCompiler ) {
+ return filterMatches( inputValue, config, filterReader, regexCompiler,
true );
+ }
+
+ public static boolean hasFindApply( UrlRewriteFilterContentDescriptor config
) {
+ if( config != null ) {
+ for( UrlRewriteFilterPathDescriptor selector : config.getSelectors() ) {
+ if( selector instanceof UrlRewriteFilterApplyDescriptor &&
((UrlRewriteFilterApplyDescriptor)selector).find() ) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+ /*
+ * The applies are processed one after the other and every apply searches
the output of the previous one, so an
Review Comment:
The applies are chained: each find apply searches the output of the previous
one, so with multiple find applies a value can get rewritten twice (e.g. `/app`
-> `/gateway/topo/app`, then a second find apply matching `/gateway/...`
rewrites it again). That was already the case for `<script>` content, but with
this change it applies to every HTML attribute and text node. Could the dev
guide state that find paths must be disjoint/anchored (or should a value be
rewritten by at most one find apply)?
--
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]
