[
https://issues.apache.org/jira/browse/SLING-7000?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16088622#comment-16088622
]
Konrad Windszus edited comment on SLING-7000 at 7/15/17 2:41 PM:
-----------------------------------------------------------------
The attached file fixes this issue and several other flaws (due to some
internal refactoring to rely more on java.net.URI and to make decomposition of
the URL closer to the Sling standard used elsewhere).
The following tests from the HTL TCK fail with this patch, but they are kind of
unexpected and should IMHO be fixed in the TCK:
# add-schema-1
(https://github.com/Adobe-Marketing-Cloud/htl-tck/blob/master/src/main/resources/testfiles/output/exprlang/filters.html#L92)
is taking a URI as input which neither start with a scheme nor with a slash
(https://github.com/Adobe-Marketing-Cloud/htl-tck/blob/master/src/main/resources/testfiles/scripts/exprlang/filters/filters.html#L92)
therefore it must be assumed to be a relative path only. In the TCK there is
the wrong assumption that {{example.com}} is being detected as host part which
would not be according to RFC 2396. I would suggest to just remove this test.
# add-selectors-1 and add-selectors-2
(https://github.com/Adobe-Marketing-Cloud/htl-tck/blob/master/src/main/resources/testfiles/scripts/exprlang/filters/filters.html#L145)
incorrectly assumes that each selector value may only appear once in a request
url. Sling does not have such a limitation (compare with
https://github.com/apache/sling/blob/4df9ab2d6592422889c71fa13afd453a10a5a626/bundles/engine/src/main/java/org/apache/sling/engine/impl/request/SlingRequestPathInfo.java#L90),
therefore I would strongly recommend to also not strip duplicates here. This
also makes sense, because the order of selectors must in most cases be kept!
# no-path-info-1
(https://github.com/Adobe-Marketing-Cloud/htl-tck/blob/master/src/main/resources/testfiles/scripts/exprlang/filters/filters.html#L113)
incorrectly assume that the URI manipulation implicitly always inserts the
path "/". This would be unexpected, please rather expect a URI without a path,
i.e. {{http://example.com?q=sightly&array=1&array=2&array=3#fragment}}
# no-path-info-2
(https://github.com/Adobe-Marketing-Cloud/htl-tck/blob/master/src/main/resources/testfiles/scripts/exprlang/filters/filters.html#L120)
incorrectly assume that the URI does not contain a path, although it contains
the path {{/}}. Therefore path prefix, suffix, selector, extension as well as
query and fragment are modified and you end up with
{{http://example.com/one/three.a.b.c.html?q=sightly&array=1&array=2&array=3#fragment"}}
# no-path-info-3 and no-path-info-4
(https://github.com/Adobe-Marketing-Cloud/htl-tck/blob/master/src/main/resources/testfiles/scripts/exprlang/filters/filters.html#L127)
do not modify the query, although it is requested. This is unexpected as
{{?q=sightly&array=1&array=2&array=3#fragment}} is a perfectly valid URI for
both cases.
[~radu.cotescu] Could you please review this patch and let me know what you
think?
was (Author: kwin):
The attached file fixes this issue and several other flaws (due to some
internal refactoring to rely more on java.net.URI and to make decomposition of
the URL closer to the Sling standard used elsewhere).
The following tests from the HTL TCK fail with this patch, but they are kind of
unexpected and should IMHO be fixed in the TCK:
# add-schema-1
(https://github.com/Adobe-Marketing-Cloud/htl-tck/blob/master/src/main/resources/testfiles/output/exprlang/filters.html#L92)
is taking a URI as input which neither start with a scheme nor with a slash
(https://github.com/Adobe-Marketing-Cloud/htl-tck/blob/master/src/main/resources/testfiles/scripts/exprlang/filters/filters.html#L92)
therefore it must be assumed to be a relative path only. In the TCK there is
the wrong assumption that {{example.com}} is being detected as host part which
would not be according to RFC 2396. I would suggest to just remove this test.
# add-selectors-1 and add-selectors-2
(https://github.com/Adobe-Marketing-Cloud/htl-tck/blob/master/src/main/resources/testfiles/scripts/exprlang/filters/filters.html#L145)
incorrectly assumes that each selector value may only appear once in a request
url. Sling does not have such a limitation (compare with
https://github.com/apache/sling/blob/4df9ab2d6592422889c71fa13afd453a10a5a626/bundles/engine/src/main/java/org/apache/sling/engine/impl/request/SlingRequestPathInfo.java#L90),
therefore I would strongly recommend to also not strip duplicates here.
# no-path-info-1
(https://github.com/Adobe-Marketing-Cloud/htl-tck/blob/master/src/main/resources/testfiles/scripts/exprlang/filters/filters.html#L113)
incorrectly assume that the URI manipulation implicitly always inserts the
path "/". This would be unexpected, please rather expect a URI without a path,
i.e. {{http://example.com?q=sightly&array=1&array=2&array=3#fragment}}
# no-path-info-2
(https://github.com/Adobe-Marketing-Cloud/htl-tck/blob/master/src/main/resources/testfiles/scripts/exprlang/filters/filters.html#L120)
incorrectly assume that the URI does not contain a path, although it contains
the path {{/}}. Therefore path prefix, suffix, selector, extension as well as
query and fragment are modified and you end up with
{{http://example.com/one/three.a.b.c.html?q=sightly&array=1&array=2&array=3#fragment"}}
# no-path-info-3 and no-path-info-4
(https://github.com/Adobe-Marketing-Cloud/htl-tck/blob/master/src/main/resources/testfiles/scripts/exprlang/filters/filters.html#L127)
do not modify the query, although it is requested. This is unexpected as
{{?q=sightly&array=1&array=2&array=3#fragment}} is a perfectly valid URI for
both cases.
[~radu.cotescu] Could you please review this patch and let me know what you
think?
> HTL: URIManipulatorFilterExtension destroys opaque URIs
> -------------------------------------------------------
>
> Key: SLING-7000
> URL: https://issues.apache.org/jira/browse/SLING-7000
> Project: Sling
> Issue Type: Bug
> Components: Scripting
> Affects Versions: Scripting Sightly Engine 1.0.0, Scripting HTL Engine
> 1.0.34
> Reporter: Konrad Windszus
> Assignee: Konrad Windszus
> Attachments: SLING-7000-v01-test.patch, SLING-7000-v1.patch
>
>
> When using the URI manipulator's extension option
> (https://github.com/Adobe-Marketing-Cloud/htl-spec/blob/master/SPECIFICATION.md#125-uri-manipulation)
> on a mailto link (or any other opaque URI) the value is completely messed up.
> {code:java}
> <a href="${'mailto:some.m...@apache.org' @ extension='html'}">
> {code}
> The {{URIManipulationFilterExtension}} returns
> {code}
> mailto://null.html
> {code}
> through its {{call(...)}} method with parameter {{arguments}} being set to
> {code}
> [mailto:kon...@netcentric.biz, {extension=html}]
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
