[
https://issues.apache.org/jira/browse/KNOX-3556?focusedWorklogId=1054055&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1054055
]
ASF GitHub Bot logged work on KNOX-3556:
----------------------------------------
Author: ASF GitHub Bot
Created on: 11/Nov/26 13:25
Start Date: 11/Nov/26 13:25
Worklog Time Spent: 10m
Work Description: 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)?
Issue Time Tracking
-------------------
Worklog Id: (was: 1054055)
Time Spent: 0.5h (was: 20m)
> Rewrite: add find option to apply so URLs embedded in HTML attribute values
> and style text can be rewritten
> -----------------------------------------------------------------------------------------------------------
>
> Key: KNOX-3556
> URL: https://issues.apache.org/jira/browse/KNOX-3556
> Project: Apache Knox
> Issue Type: Task
> Components: Server
> Affects Versions: 3.0.0
> Reporter: Tamás Marcinkovics
> Assignee: Tamás Marcinkovics
> Priority: Major
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Add find mode to rewrite {{<apply>}} for HTML attributes and text.
>
> OpenSearch Dashboards puts its base path into the page as
> {{{}<osd-injected-metadata
> data="\{"basePath":"/ctx","serverBasePath":"/ctx",…}">{}}}.
> The browser JS reads this once at startup and builds every request URL and
> the router base from it, so the value must include the Knox prefix
> ({{{}/gateway/<topology>/ctx{}}}).
>
> Knox rewrites an attribute only when its whole value is a URL, so a URL
> embedded in a JSON attribute value is left unchanged.
> Because Knox cannot rewrite inside the attribute, the backend has to be
> configured with the gateway path and topology name. In that page only four
> values need rewriting: basePath, serverBasePath, i18n.translationsUrl and
> branding.assetFolderUrl.
> About 100 font URLs in inline {{<style>}} are affected the same way.
> h4. Proposal
> Add an optional find attribute to <apply> (default false, so existing service
> definitions are unaffected):
> {{<content type="*/html">}}
> {{<apply path="/ctx[^&"'\s\\)]*" find="true" rule="..."/>}}
> {{</content>}}
> When it is set, the HTML reader applies the regex with {{Matcher.find()}} to
> attribute values and to {{<style>}} text. It rewrites each match through the
> named rule and leaves the surrounding text untouched. This is the behaviour
> the reader already has for {{<script>}} text.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
