You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "Stefan Seifert (JIRA)" <ji...@apache.org> on 2010/12/17 00:13:01 UTC
[jira] Created: (SLING-1899) JcrResolver map method is not reverse
operation of resolve method with special chars in path
JcrResolver map method is not reverse operation of resolve method with special chars in path
--------------------------------------------------------------------------------------------
Key: SLING-1899
URL: https://issues.apache.org/jira/browse/SLING-1899
Project: Sling
Issue Type: Bug
Components: JCR
Affects Versions: JCR Resource 2.0.6
Reporter: Stefan Seifert
Attachments: 101216_mapresolve_specialchars_test.patch
accoring to the javadocs of the ResourceResolver API interface, the map method is inteded as the reverse operation of the resolve method.
this is not the case if the path contains special chars like spaces - they are url-encoded, although the javadocs of the API does not require this.
additionally the resolve method does not url-deocde the path (which is correct in my opinion because the servlet engine already decodes the path).
the map method should be fixed, so that not url-encoding takes place in it. the application is responsible for url-encoding a path if needed, not the sling api.
i've attached a patch which contains a simple unit tests that makes the current implementation fail when resolving a path generated with the map method: [^101216_mapresolve_specialchars_test.patch]
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SLING-1899) JcrResourceResolver map method is not
reverse operation of resolve method with special chars in path
Posted by "Stefan Seifert (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-1899?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Stefan Seifert updated SLING-1899:
----------------------------------
Description:
according to the javadocs of the ResourceResolver API interface, the map method is intended as the reverse operation of the resolve method.
this is not the case if the path contains special chars like spaces - they are url-encoded, although the javadocs of the API does not require this.
additionally the resolve method does not url-decode the path (which is correct in my opinion because the servlet engine already decodes the path).
the map method should be fixed, so that not url-encoding takes place in it. the application is responsible for url-encoding a path if needed, not the sling api.
i've attached a patch which contains a simple unit tests that makes the current implementation fail when resolving a path generated with the map method: [^101216_mapresolve_specialchars_test.patch]
was:
accoring to the javadocs of the ResourceResolver API interface, the map method is inteded as the reverse operation of the resolve method.
this is not the case if the path contains special chars like spaces - they are url-encoded, although the javadocs of the API does not require this.
additionally the resolve method does not url-deocde the path (which is correct in my opinion because the servlet engine already decodes the path).
the map method should be fixed, so that not url-encoding takes place in it. the application is responsible for url-encoding a path if needed, not the sling api.
i've attached a patch which contains a simple unit tests that makes the current implementation fail when resolving a path generated with the map method: [^101216_mapresolve_specialchars_test.patch]
Summary: JcrResourceResolver map method is not reverse operation of resolve method with special chars in path (was: JcrResolver map method is not reverse operation of resolve method with special chars in path)
> JcrResourceResolver map method is not reverse operation of resolve method with special chars in path
> ----------------------------------------------------------------------------------------------------
>
> Key: SLING-1899
> URL: https://issues.apache.org/jira/browse/SLING-1899
> Project: Sling
> Issue Type: Bug
> Components: JCR
> Affects Versions: JCR Resource 2.0.6
> Reporter: Stefan Seifert
> Attachments: 101216_mapresolv_specialchars_fix.patch, 101216_mapresolve_specialchars_test.patch
>
>
> according to the javadocs of the ResourceResolver API interface, the map method is intended as the reverse operation of the resolve method.
> this is not the case if the path contains special chars like spaces - they are url-encoded, although the javadocs of the API does not require this.
> additionally the resolve method does not url-decode the path (which is correct in my opinion because the servlet engine already decodes the path).
> the map method should be fixed, so that not url-encoding takes place in it. the application is responsible for url-encoding a path if needed, not the sling api.
> i've attached a patch which contains a simple unit tests that makes the current implementation fail when resolving a path generated with the map method: [^101216_mapresolve_specialchars_test.patch]
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SLING-1899) JcrResolver map method is not reverse
operation of resolve method with special chars in path
Posted by "Stefan Seifert (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-1899?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Stefan Seifert updated SLING-1899:
----------------------------------
Attachment: 101216_mapresolve_specialchars_test.patch
> JcrResolver map method is not reverse operation of resolve method with special chars in path
> --------------------------------------------------------------------------------------------
>
> Key: SLING-1899
> URL: https://issues.apache.org/jira/browse/SLING-1899
> Project: Sling
> Issue Type: Bug
> Components: JCR
> Affects Versions: JCR Resource 2.0.6
> Reporter: Stefan Seifert
> Attachments: 101216_mapresolve_specialchars_test.patch
>
>
> accoring to the javadocs of the ResourceResolver API interface, the map method is inteded as the reverse operation of the resolve method.
> this is not the case if the path contains special chars like spaces - they are url-encoded, although the javadocs of the API does not require this.
> additionally the resolve method does not url-deocde the path (which is correct in my opinion because the servlet engine already decodes the path).
> the map method should be fixed, so that not url-encoding takes place in it. the application is responsible for url-encoding a path if needed, not the sling api.
> i've attached a patch which contains a simple unit tests that makes the current implementation fail when resolving a path generated with the map method: [^101216_mapresolve_specialchars_test.patch]
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SLING-1899) JcrResourceResolver map method is
not reverse operation of resolve method with special chars in path
Posted by "Carsten Ziegeler (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-1899?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12972774#action_12972774 ]
Carsten Ziegeler commented on SLING-1899:
-----------------------------------------
> But I agree that a map() method that does not do url-encoding would make sense. I would simply add a new one, make the javadocs on the existing ones more
> explicit, but _not_ deprecate them, since they do have a purpose. It should not be moved to yet-another utility class.
Deprecating does not mean removing, for obvious reasons this method has to stay as is for a long term
And no, we're not talking about an utility class here either - I think we should add the methods to the resource resolver;
And I agree with Justin, that "map" might not be the best name for a new method.
> JcrResourceResolver map method is not reverse operation of resolve method with special chars in path
> ----------------------------------------------------------------------------------------------------
>
> Key: SLING-1899
> URL: https://issues.apache.org/jira/browse/SLING-1899
> Project: Sling
> Issue Type: Bug
> Components: JCR
> Affects Versions: JCR Resource 2.0.6
> Reporter: Stefan Seifert
> Attachments: 101216_mapresolv_specialchars_fix.patch, 101216_mapresolve_specialchars_test.patch
>
>
> according to the javadocs of the ResourceResolver API interface, the map method is intended as the reverse operation of the resolve method.
> this is not the case if the path contains special chars like spaces - they are url-encoded, although the javadocs of the API does not require this.
> additionally the resolve method does not url-decode the path (which is correct in my opinion because the servlet engine already decodes the path).
> the map method should be fixed, so that not url-encoding takes place in it. the application is responsible for url-encoding a path if needed, not the sling api.
> i've attached a patch which contains a simple unit tests that makes the current implementation fail when resolving a path generated with the map method: [^101216_mapresolve_specialchars_test.patch]
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SLING-1899) JcrResourceResolver map method is
not reverse operation of resolve method with special chars in path
Posted by "Alexander Klimetschek (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-1899?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12972511#action_12972511 ]
Alexander Klimetschek commented on SLING-1899:
----------------------------------------------
> not a convenience layer for JSP scripting
This is not about "scripting". map()'s purpose was to generate full URLs to send back in HTTP responses and that's what it does, regardless from where you call it.
> within a web framework like wicket
Ok, yes, but that's a special case ;-)
> to pass it over to javascript or JSON you have to manually url-decode the value returned by the map method, otherwise it may get double URL-encoded
Could you give an example of passing it over to javascript or JSON?
The normal form of a URL is in its encoded form, including relative URL paths, such as "/some/path%20foo.html". Any client-side javascript would be expected to properly parse it if it does more then simply placing it in a href for example.
But I agree that a map() method that does not do url-encoding would make sense. I would simply add a new one, make the javadocs on the existing ones more explicit, but _not_ deprecate them, since they do have a purpose. It should not be moved to yet-another utility class.
> JcrResourceResolver map method is not reverse operation of resolve method with special chars in path
> ----------------------------------------------------------------------------------------------------
>
> Key: SLING-1899
> URL: https://issues.apache.org/jira/browse/SLING-1899
> Project: Sling
> Issue Type: Bug
> Components: JCR
> Affects Versions: JCR Resource 2.0.6
> Reporter: Stefan Seifert
> Attachments: 101216_mapresolv_specialchars_fix.patch, 101216_mapresolve_specialchars_test.patch
>
>
> according to the javadocs of the ResourceResolver API interface, the map method is intended as the reverse operation of the resolve method.
> this is not the case if the path contains special chars like spaces - they are url-encoded, although the javadocs of the API does not require this.
> additionally the resolve method does not url-decode the path (which is correct in my opinion because the servlet engine already decodes the path).
> the map method should be fixed, so that not url-encoding takes place in it. the application is responsible for url-encoding a path if needed, not the sling api.
> i've attached a patch which contains a simple unit tests that makes the current implementation fail when resolving a path generated with the map method: [^101216_mapresolve_specialchars_test.patch]
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SLING-1899) JcrResourceResolver map method is
not reverse operation of resolve method with special chars in path
Posted by "Bertrand Delacretaz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-1899?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12972493#action_12972493 ]
Bertrand Delacretaz commented on SLING-1899:
--------------------------------------------
I like Carsten's proposal - it's important to keep backwards compatibility, but the current map method does too much.
I'd like to add (and you see me coming ;-) that we should make sure we have automated tests for the existing behavior before changing it.
> JcrResourceResolver map method is not reverse operation of resolve method with special chars in path
> ----------------------------------------------------------------------------------------------------
>
> Key: SLING-1899
> URL: https://issues.apache.org/jira/browse/SLING-1899
> Project: Sling
> Issue Type: Bug
> Components: JCR
> Affects Versions: JCR Resource 2.0.6
> Reporter: Stefan Seifert
> Attachments: 101216_mapresolv_specialchars_fix.patch, 101216_mapresolve_specialchars_test.patch
>
>
> according to the javadocs of the ResourceResolver API interface, the map method is intended as the reverse operation of the resolve method.
> this is not the case if the path contains special chars like spaces - they are url-encoded, although the javadocs of the API does not require this.
> additionally the resolve method does not url-decode the path (which is correct in my opinion because the servlet engine already decodes the path).
> the map method should be fixed, so that not url-encoding takes place in it. the application is responsible for url-encoding a path if needed, not the sling api.
> i've attached a patch which contains a simple unit tests that makes the current implementation fail when resolving a path generated with the map method: [^101216_mapresolve_specialchars_test.patch]
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SLING-1899) JcrResolver map method is not reverse
operation of resolve method with special chars in path
Posted by "Stefan Seifert (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-1899?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Stefan Seifert updated SLING-1899:
----------------------------------
Attachment: 101216_mapresolv_specialchars_fix.patch
attached is a fix, that resolves the problem: [^101216_mapresolv_specialchars_fix.patch]
if applied, the unit "testMapURLEscaping" fails. but in my opinition, this unit test is not correct either and should be removed/adapted to a non-escaped URI.
> JcrResolver map method is not reverse operation of resolve method with special chars in path
> --------------------------------------------------------------------------------------------
>
> Key: SLING-1899
> URL: https://issues.apache.org/jira/browse/SLING-1899
> Project: Sling
> Issue Type: Bug
> Components: JCR
> Affects Versions: JCR Resource 2.0.6
> Reporter: Stefan Seifert
> Attachments: 101216_mapresolv_specialchars_fix.patch, 101216_mapresolve_specialchars_test.patch
>
>
> accoring to the javadocs of the ResourceResolver API interface, the map method is inteded as the reverse operation of the resolve method.
> this is not the case if the path contains special chars like spaces - they are url-encoded, although the javadocs of the API does not require this.
> additionally the resolve method does not url-deocde the path (which is correct in my opinion because the servlet engine already decodes the path).
> the map method should be fixed, so that not url-encoding takes place in it. the application is responsible for url-encoding a path if needed, not the sling api.
> i've attached a patch which contains a simple unit tests that makes the current implementation fail when resolving a path generated with the map method: [^101216_mapresolve_specialchars_test.patch]
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SLING-1899) JcrResourceResolver map method is
not reverse operation of resolve method with special chars in path
Posted by "Stefan Seifert (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-1899?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12972486#action_12972486 ]
Stefan Seifert commented on SLING-1899:
---------------------------------------
from my point of view the sling API should be a general purpose Java API - not a convenience layer for JSP scripting.
if you use the API in other usecases - e.g. within a web framework like wicket, or to pass it over to javascript or JSON you have to manually url-decode the value returned by the map method, otherwise it may get double URL-encoded.
if the implementation of the API method is not changed at least the javadocs of the map method in the ResourceResolver interface have to be updated to document that the paths returned are URL-encoded, and they cannot be used 1:1 as reverse operation for the resolve method.
> JcrResourceResolver map method is not reverse operation of resolve method with special chars in path
> ----------------------------------------------------------------------------------------------------
>
> Key: SLING-1899
> URL: https://issues.apache.org/jira/browse/SLING-1899
> Project: Sling
> Issue Type: Bug
> Components: JCR
> Affects Versions: JCR Resource 2.0.6
> Reporter: Stefan Seifert
> Attachments: 101216_mapresolv_specialchars_fix.patch, 101216_mapresolve_specialchars_test.patch
>
>
> according to the javadocs of the ResourceResolver API interface, the map method is intended as the reverse operation of the resolve method.
> this is not the case if the path contains special chars like spaces - they are url-encoded, although the javadocs of the API does not require this.
> additionally the resolve method does not url-decode the path (which is correct in my opinion because the servlet engine already decodes the path).
> the map method should be fixed, so that not url-encoding takes place in it. the application is responsible for url-encoding a path if needed, not the sling api.
> i've attached a patch which contains a simple unit tests that makes the current implementation fail when resolving a path generated with the map method: [^101216_mapresolve_specialchars_test.patch]
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SLING-1899) JcrResourceResolver map method is
not reverse operation of resolve method with special chars in path
Posted by "Justin Edelson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-1899?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12972503#action_12972503 ]
Justin Edelson commented on SLING-1899:
---------------------------------------
> - introduce a new "map" method which does pure mapping
Could we name the new method something else? Personally, I've never found the name "map" to be particularly intuitive, although it is now drilled into my head (see also Effective Java #41, although that describes a different problem).
That said, I don't have any great ideas for a new name. "unresolve" - no. "reverseResolve" - no. "theMethodFormerlyKnownAsMap" - too long
> JcrResourceResolver map method is not reverse operation of resolve method with special chars in path
> ----------------------------------------------------------------------------------------------------
>
> Key: SLING-1899
> URL: https://issues.apache.org/jira/browse/SLING-1899
> Project: Sling
> Issue Type: Bug
> Components: JCR
> Affects Versions: JCR Resource 2.0.6
> Reporter: Stefan Seifert
> Attachments: 101216_mapresolv_specialchars_fix.patch, 101216_mapresolve_specialchars_test.patch
>
>
> according to the javadocs of the ResourceResolver API interface, the map method is intended as the reverse operation of the resolve method.
> this is not the case if the path contains special chars like spaces - they are url-encoded, although the javadocs of the API does not require this.
> additionally the resolve method does not url-decode the path (which is correct in my opinion because the servlet engine already decodes the path).
> the map method should be fixed, so that not url-encoding takes place in it. the application is responsible for url-encoding a path if needed, not the sling api.
> i've attached a patch which contains a simple unit tests that makes the current implementation fail when resolving a path generated with the map method: [^101216_mapresolve_specialchars_test.patch]
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SLING-1899) JcrResourceResolver map method is
not reverse operation of resolve method with special chars in path
Posted by "Carsten Ziegeler (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-1899?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12972490#action_12972490 ]
Carsten Ziegeler commented on SLING-1899:
-----------------------------------------
I tend to agree with Stefan here - the resource resolver API is general purpose and it seems strange that a map method also does url encoding.
Of course we need some convenience for jsps or other scripting layers.
And changing the behaviour of the method now - after we have done several releases with url encoding - we shouldn't change it for compatilibty reasons.
So I think we should:
- fix the javadocs of the map method
- deprecate the map methods
- introduce a new "map" method which does pure mapping
- introduce some convenience stuff for scripting (invoking map and encoding)
> JcrResourceResolver map method is not reverse operation of resolve method with special chars in path
> ----------------------------------------------------------------------------------------------------
>
> Key: SLING-1899
> URL: https://issues.apache.org/jira/browse/SLING-1899
> Project: Sling
> Issue Type: Bug
> Components: JCR
> Affects Versions: JCR Resource 2.0.6
> Reporter: Stefan Seifert
> Attachments: 101216_mapresolv_specialchars_fix.patch, 101216_mapresolve_specialchars_test.patch
>
>
> according to the javadocs of the ResourceResolver API interface, the map method is intended as the reverse operation of the resolve method.
> this is not the case if the path contains special chars like spaces - they are url-encoded, although the javadocs of the API does not require this.
> additionally the resolve method does not url-decode the path (which is correct in my opinion because the servlet engine already decodes the path).
> the map method should be fixed, so that not url-encoding takes place in it. the application is responsible for url-encoding a path if needed, not the sling api.
> i've attached a patch which contains a simple unit tests that makes the current implementation fail when resolving a path generated with the map method: [^101216_mapresolve_specialchars_test.patch]
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SLING-1899) JcrResourceResolver map method is
not reverse operation of resolve method with special chars in path
Posted by "Alexander Klimetschek (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SLING-1899?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12972481#action_12972481 ]
Alexander Klimetschek commented on SLING-1899:
----------------------------------------------
> the map method should be fixed, so that not url-encoding takes place in it. the application is responsible for url-encoding a path if needed, not the sling api.
I disagree. Because then you need another utility method in your application to url-encode the resulting string. Such a "basic" thing should be done by the Sling API to be able to do things like this in a jsp, for example:
<a href="<%= resourceResolver.map(repoPath) %>">link</a>
It is true that the resource resolution does not url-decode the URL since it has already been done by the servlet engine, however, on the way back, there is nothing that would do the url-encoding automatically, since a servlet directly writes to the response.
I think the case were you want a not-url-encoded string back from the map() method is rarer.
> JcrResourceResolver map method is not reverse operation of resolve method with special chars in path
> ----------------------------------------------------------------------------------------------------
>
> Key: SLING-1899
> URL: https://issues.apache.org/jira/browse/SLING-1899
> Project: Sling
> Issue Type: Bug
> Components: JCR
> Affects Versions: JCR Resource 2.0.6
> Reporter: Stefan Seifert
> Attachments: 101216_mapresolv_specialchars_fix.patch, 101216_mapresolve_specialchars_test.patch
>
>
> according to the javadocs of the ResourceResolver API interface, the map method is intended as the reverse operation of the resolve method.
> this is not the case if the path contains special chars like spaces - they are url-encoded, although the javadocs of the API does not require this.
> additionally the resolve method does not url-decode the path (which is correct in my opinion because the servlet engine already decodes the path).
> the map method should be fixed, so that not url-encoding takes place in it. the application is responsible for url-encoding a path if needed, not the sling api.
> i've attached a patch which contains a simple unit tests that makes the current implementation fail when resolving a path generated with the map method: [^101216_mapresolve_specialchars_test.patch]
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.