Re: [tools] StrutsLinkTool's setForward method has different behavior
On Fri, Jul 29, 2011 at 9:24 AM, Christopher Schultz wrote: > Nathan, > > On 7/28/2011 1:20 PM, Nathan Bubna wrote: >> Wow. Slow down. I already fixed VELTOOLS-146. You need to svn >> update your code. > > Aah, I hadn't seen that. I've been at a location that has horrendous > Internet access and email is one of the most painful things to do on > that network. > > It didn't take that long to research and "fix". :) > Yeah, I wasn't convinced that my solution was the best, but at least I > understood what was happening. I've looked at your patch and like it > much better than mine -- I believe it treats URIs more uniformly and > appropriately, and the code is simpler than it was before. > > Confirmed that it works for the one case I was testing. I'll look for > others and then confirm the fix in general. sounds great. thanks! > Thanks, > -chris > > - To unsubscribe, e-mail: user-unsubscr...@velocity.apache.org For additional commands, e-mail: user-h...@velocity.apache.org
Re: [tools] StrutsLinkTool's setForward method has different behavior
Nathan, On 7/28/2011 1:20 PM, Nathan Bubna wrote: > Wow. Slow down. I already fixed VELTOOLS-146. You need to svn > update your code. Aah, I hadn't seen that. I've been at a location that has horrendous Internet access and email is one of the most painful things to do on that network. It didn't take that long to research and "fix". Yeah, I wasn't convinced that my solution was the best, but at least I understood what was happening. I've looked at your patch and like it much better than mine -- I believe it treats URIs more uniformly and appropriately, and the code is simpler than it was before. Confirmed that it works for the one case I was testing. I'll look for others and then confirm the fix in general. Thanks, -chris signature.asc Description: OpenPGP digital signature
Re: [tools] StrutsLinkTool's setForward method has different behavior
Also, these discussions REALLY need to happen on dev@. user@ is not
the right place.
On Thu, Jul 28, 2011 at 10:20 AM, Nathan Bubna wrote:
> Wow. Slow down. I already fixed VELTOOLS-146. You need to svn
> update your code.
>
> On Thu, Jul 28, 2011 at 10:00 AM, Christopher Schultz
> wrote:
>> All,
>>
>> On 7/28/2011 12:48 PM, Christopher Schultz wrote:
>>> I think the solution is to modify this so that
>>> generic.LinkTool.absolute() /does/ use the URI class to parse the URI
>>> and separate-out the query string, etc. and then set them on the copied
>>> LinkTool's instance.
>>
>> Okay, this works, but I'm not sure if it's the best solution, or if it's
>> even a complete solution. StrutsLinkTool calls LinkTool.absolute(), but
>> the path is actually a relative path, so maybe calling
>> LinkTool.relative() is a better thing to do. In that case, the fix
>> should probably be different.
>>
>> Quick diff for generic.LinkTool:
>>
>>
>> Index: src/main/java/org/apache/velocity/tools/generic/LinkTool.java
>> ===
>> --- src/main/java/org/apache/velocity/tools/generic/LinkTool.java
>> (revision 1151249)
>> +++ src/main/java/org/apache/velocity/tools/generic/LinkTool.java
>> (working copy)
>> @@ -1357,6 +1357,7 @@
>> {
>> return null;
>> }
>> +
>> copy.setScheme(uri.getScheme());
>> copy.setUserInfo(uri.getUserInfo());
>> copy.setHost(uri.getHost());
>> @@ -1378,8 +1379,30 @@
>> }
>> return copy;
>> }
>> - else if (!pth.startsWith("/"))
>> + else if (pth.startsWith("/"))
>> {
>> + // This is a relative path, not absolute.
>> + // That's okay -- parse the URI anyway but don't process
>> + // the host, scheme, port, user, etc.
>> + URI uri = toURI(pth);
>> +
>> + pth = uri.getPath();
>> + if (pth.equals("/") || pth.length() == 0)
>> + {
>> + pth = null;
>> + }
>> + copy.setPath(pth);
>> + if (uri.getQuery() != null)
>> + {
>> + copy.setQuery(uri.getQuery());
>> + }
>> + if (uri.getFragment() != null)
>> + {
>> + copy.setFragment(uri.getFragment());
>> + }
>> + }
>> + else
>> + {
>> // paths that don't start with '/'
>> // are considered relative to the current directory
>> pth = combinePath(getDirectory(), pth);
>>
>>
>
-
To unsubscribe, e-mail: user-unsubscr...@velocity.apache.org
For additional commands, e-mail: user-h...@velocity.apache.org
Re: [tools] StrutsLinkTool's setForward method has different behavior
Wow. Slow down. I already fixed VELTOOLS-146. You need to svn
update your code.
On Thu, Jul 28, 2011 at 10:00 AM, Christopher Schultz
wrote:
> All,
>
> On 7/28/2011 12:48 PM, Christopher Schultz wrote:
>> I think the solution is to modify this so that
>> generic.LinkTool.absolute() /does/ use the URI class to parse the URI
>> and separate-out the query string, etc. and then set them on the copied
>> LinkTool's instance.
>
> Okay, this works, but I'm not sure if it's the best solution, or if it's
> even a complete solution. StrutsLinkTool calls LinkTool.absolute(), but
> the path is actually a relative path, so maybe calling
> LinkTool.relative() is a better thing to do. In that case, the fix
> should probably be different.
>
> Quick diff for generic.LinkTool:
>
>
> Index: src/main/java/org/apache/velocity/tools/generic/LinkTool.java
> ===
> --- src/main/java/org/apache/velocity/tools/generic/LinkTool.java
> (revision 1151249)
> +++ src/main/java/org/apache/velocity/tools/generic/LinkTool.java
> (working copy)
> @@ -1357,6 +1357,7 @@
> {
> return null;
> }
> +
> copy.setScheme(uri.getScheme());
> copy.setUserInfo(uri.getUserInfo());
> copy.setHost(uri.getHost());
> @@ -1378,8 +1379,30 @@
> }
> return copy;
> }
> - else if (!pth.startsWith("/"))
> + else if (pth.startsWith("/"))
> {
> + // This is a relative path, not absolute.
> + // That's okay -- parse the URI anyway but don't process
> + // the host, scheme, port, user, etc.
> + URI uri = toURI(pth);
> +
> + pth = uri.getPath();
> + if (pth.equals("/") || pth.length() == 0)
> + {
> + pth = null;
> + }
> + copy.setPath(pth);
> + if (uri.getQuery() != null)
> + {
> + copy.setQuery(uri.getQuery());
> + }
> + if (uri.getFragment() != null)
> + {
> + copy.setFragment(uri.getFragment());
> + }
> + }
> + else
> + {
> // paths that don't start with '/'
> // are considered relative to the current directory
> pth = combinePath(getDirectory(), pth);
>
>
-
To unsubscribe, e-mail: user-unsubscr...@velocity.apache.org
For additional commands, e-mail: user-h...@velocity.apache.org
Re: [tools] StrutsLinkTool's setForward method has different behavior
All,
On 7/28/2011 12:48 PM, Christopher Schultz wrote:
> I think the solution is to modify this so that
> generic.LinkTool.absolute() /does/ use the URI class to parse the URI
> and separate-out the query string, etc. and then set them on the copied
> LinkTool's instance.
Okay, this works, but I'm not sure if it's the best solution, or if it's
even a complete solution. StrutsLinkTool calls LinkTool.absolute(), but
the path is actually a relative path, so maybe calling
LinkTool.relative() is a better thing to do. In that case, the fix
should probably be different.
Quick diff for generic.LinkTool:
Index: src/main/java/org/apache/velocity/tools/generic/LinkTool.java
===
--- src/main/java/org/apache/velocity/tools/generic/LinkTool.java
(revision 1151249)
+++ src/main/java/org/apache/velocity/tools/generic/LinkTool.java
(working copy)
@@ -1357,6 +1357,7 @@
{
return null;
}
+
copy.setScheme(uri.getScheme());
copy.setUserInfo(uri.getUserInfo());
copy.setHost(uri.getHost());
@@ -1378,8 +1379,30 @@
}
return copy;
}
-else if (!pth.startsWith("/"))
+else if (pth.startsWith("/"))
{
+// This is a relative path, not absolute.
+// That's okay -- parse the URI anyway but don't process
+// the host, scheme, port, user, etc.
+URI uri = toURI(pth);
+
+pth = uri.getPath();
+if (pth.equals("/") || pth.length() == 0)
+{
+pth = null;
+}
+copy.setPath(pth);
+if (uri.getQuery() != null)
+{
+copy.setQuery(uri.getQuery());
+}
+if (uri.getFragment() != null)
+{
+copy.setFragment(uri.getFragment());
+}
+}
+else
+{
// paths that don't start with '/'
// are considered relative to the current directory
pth = combinePath(getDirectory(), pth);
signature.asc
Description: OpenPGP digital signature
Re: [tools] StrutsLinkTool's setForward method has different behavior
Nathan, On 7/27/2011 10:55 AM, Nathan Bubna wrote: > [I]t means that after you turn the url into a java.net.URI (which > does the parsing for you) and you are copying out the sections of the > URI into the current LinkTool instance, you don't want to override > things in the currents LinkTool that are absent in the URI. If the > URI has no scheme, don't set the LinkTool to have a null scheme, and > so on. It looks like StrutsLinkTool.setForward() invokes generic.LinkTool.absolute() which doesn't modify the path at all, and doesn't use java.net.URI or java.net.URL. Basically, my path (/foo/bar?foo=bar) gets set as the tool's path directly. I think the solution is to modify this so that generic.LinkTool.absolute() /does/ use the URI class to parse the URI and separate-out the query string, etc. and then set them on the copied LinkTool's instance. I'll give it a shot and see if it works out. -chris signature.asc Description: OpenPGP digital signature
Re: [tools] StrutsLinkTool's setForward method has different behavior
Nathan,
On 7/27/2011 10:55 AM, Nathan Bubna wrote:
> On Wed, Jul 27, 2011 at 7:32 AM, wrote:
>> Nathan Bubna wrote:
>>>
>>> This probably will require a setFromURI(url, withoutOverriding)
>>> adapted from what absolute($url) does for actual absolute URIs.
>>
>> So "withoutOverriding" really means "don't escape"?
>
> no, it means that after you turn the url into a java.net.URI (which
> does the parsing for you) and you are copying out the sections of the
> URI into the current LinkTool instance, you don't want to override
> things in the currents LinkTool that are absent in the URI. If the
> URI has no scheme, don't set the LinkTool to have a null scheme, and
> so on.
Gotcha.
>>> In the meantime, try this:
>>>
>>> context.put(java.net.URLDecoder.class, "url");
>>>
>>> $url.decode($link.setForward('some-reference'))
>>
>> That's not going to work because we also add more query string data to the
>> link after the setForward() call. (I simplified my original example).
>
> As long as your query string data doesn't have anything encoded that
> you don't want decoded, then add the params before decoding and don't
> worry about it.
So... it happens that I do have some items that need to be escaped --
like "next step" URLs and the like.
>> LinkTool would be most flexible if it would parse any existing query string
>> into it's own parameter table OR be sensitive about an existing query string
>> and add anything to is as appropriate (this would probably be best, unless
>> there are methods in LinkTool that expect to be able to /replace/
>> previously-set query string parameters).
>
> The method of upgrading relative(url) and absolute(url) that i
> describe above will parse and add params to the internal map, because
> yes, there are methods that expect to be able to replace parameters.
Okay, great. I'll be happy to immediately try any patch you have.
>>> Or extend StrutsLinkTool to do that for you.
>>
>> I think I'll try to hack SLT to work for me temporarily with an eye towards
>> a longer-term solution that actually gets committed to VELTOOLS. I need to
>> get this working so I can continue to use VELTOOLS-2.0... otherwise I'll
>> have to roll-back to 1.4.
>
> Please don't. :)
Definitely not a preferred course of action.
-chris
signature.asc
Description: OpenPGP digital signature
Re: [tools] StrutsLinkTool's setForward method has different behavior
On Wed, Jul 27, 2011 at 7:32 AM, wrote:
...
>> Basically, setForward
>> passes the url to absolute(url) which would correctly handle absolute
>> urls with query strings but treats relative urls as merely paths,
>> without parsing out anchors or query strings.
>
> Okay, I'll take a look at this.
>
>> I think we should adapt both $link.relative($url) and
>> $link.absolute($url) to accept paths with query strings and anchors.
>
> That would be great. I dunno about anchors, but query strings used to work
> -- at least with SLT's setForward().
anchors should be allowed too.
>> This probably will require a setFromURI(url, withoutOverriding)
>> adapted from what absolute($url) does for actual absolute URIs.
>
> So "withoutOverriding" really means "don't escape"?
no, it means that after you turn the url into a java.net.URI (which
does the parsing for you) and you are copying out the sections of the
URI into the current LinkTool instance, you don't want to override
things in the currents LinkTool that are absent in the URI. If the
URI has no scheme, don't set the LinkTool to have a null scheme, and
so on.
>> In the meantime, try this:
>>
>> context.put(java.net.URLDecoder.class, "url");
>>
>> $url.decode($link.setForward('some-reference'))
>
> That's not going to work because we also add more query string data to the
> link after the setForward() call. (I simplified my original example).
As long as your query string data doesn't have anything encoded that
you don't want decoded, then add the params before decoding and don't
worry about it.
> LinkTool would be most flexible if it would parse any existing query string
> into it's own parameter table OR be sensitive about an existing query string
> and add anything to is as appropriate (this would probably be best, unless
> there are methods in LinkTool that expect to be able to /replace/
> previously-set query string parameters).
The method of upgrading relative(url) and absolute(url) that i
describe above will parse and add params to the internal map, because
yes, there are methods that expect to be able to replace parameters.
>> Or extend StrutsLinkTool to do that for you.
>
> I think I'll try to hack SLT to work for me temporarily with an eye towards
> a longer-term solution that actually gets committed to VELTOOLS. I need to
> get this working so I can continue to use VELTOOLS-2.0... otherwise I'll
> have to roll-back to 1.4.
Please don't. :)
-
To unsubscribe, e-mail: user-unsubscr...@velocity.apache.org
For additional commands, e-mail: user-h...@velocity.apache.org
Re: [tools] StrutsLinkTool's setForward method has different behavior
On Tue, 26 Jul 2011 15:08:28 -0700, Nathan Bubna wrote:
On Tue, Jul 26, 2011 at 2:27 PM, Christopher Schultz
wrote:
All,
We use StrutsLinkTool pretty much everywhere and had been relying on
the
previous behavior (tools 1.4) of StrutsLinkTool.setForward(String)
which
takes a reference to a Struts-defined "forward" which is basically
just
a URL.
In the past, we were able to define a URL in Struts like this:
Then, in the markup:
link text
It appears that somewhere in the modifications, the URL ha sbeen
sanitized and now "/path/to/resource?foo=bar" has been helpfully
replaced by "/path/to/resource%3Ffoo=bar".
Was this intentional? Is there a way to get the old behavior back?
It was unintentionally changed, in part because i wasn't aware that
forwards could/would contain query strings.
Good to know that this wasn't a policy decision. That means it's
possible to get around it :)
Basically, setForward
passes the url to absolute(url) which would correctly handle absolute
urls with query strings but treats relative urls as merely paths,
without parsing out anchors or query strings.
Okay, I'll take a look at this.
I think we should adapt both $link.relative($url) and
$link.absolute($url) to accept paths with query strings and anchors.
That would be great. I dunno about anchors, but query strings used to
work -- at least with SLT's setForward().
This probably will require a setFromURI(url, withoutOverriding)
adapted from what absolute($url) does for actual absolute URIs.
So "withoutOverriding" really means "don't escape"?
In the meantime, try this:
context.put(java.net.URLDecoder.class, "url");
$url.decode($link.setForward('some-reference'))
That's not going to work because we also add more query string data to
the link after the setForward() call. (I simplified my original
example).
LinkTool would be most flexible if it would parse any existing query
string into it's own parameter table OR be sensitive about an existing
query string and add anything to is as appropriate (this would probably
be best, unless there are methods in LinkTool that expect to be able to
/replace/ previously-set query string parameters).
Or extend StrutsLinkTool to do that for you.
I think I'll try to hack SLT to work for me temporarily with an eye
towards a longer-term solution that actually gets committed to VELTOOLS.
I need to get this working so I can continue to use VELTOOLS-2.0...
otherwise I'll have to roll-back to 1.4.
Thanks,
-chris
-
To unsubscribe, e-mail: user-unsubscr...@velocity.apache.org
For additional commands, e-mail: user-h...@velocity.apache.org
Re: [tools] StrutsLinkTool's setForward method has different behavior
On Tue, Jul 26, 2011 at 2:27 PM, Christopher Schultz
wrote:
> All,
>
> We use StrutsLinkTool pretty much everywhere and had been relying on the
> previous behavior (tools 1.4) of StrutsLinkTool.setForward(String) which
> takes a reference to a Struts-defined "forward" which is basically just
> a URL.
>
> In the past, we were able to define a URL in Struts like this:
>
>
>
> Then, in the markup:
>
> link text
>
> It appears that somewhere in the modifications, the URL ha sbeen
> sanitized and now "/path/to/resource?foo=bar" has been helpfully
> replaced by "/path/to/resource%3Ffoo=bar".
>
> Was this intentional? Is there a way to get the old behavior back?
It was unintentionally changed, in part because i wasn't aware that
forwards could/would contain query strings. Basically, setForward
passes the url to absolute(url) which would correctly handle absolute
urls with query strings but treats relative urls as merely paths,
without parsing out anchors or query strings.
I think we should adapt both $link.relative($url) and
$link.absolute($url) to accept paths with query strings and anchors.
This probably will require a setFromURI(url, withoutOverriding)
adapted from what absolute($url) does for actual absolute URIs.
In the meantime, try this:
context.put(java.net.URLDecoder.class, "url");
$url.decode($link.setForward('some-reference'))
Or extend StrutsLinkTool to do that for you.
> Thanks,
> -chris
>
>
-
To unsubscribe, e-mail: user-unsubscr...@velocity.apache.org
For additional commands, e-mail: user-h...@velocity.apache.org
[tools] StrutsLinkTool's setForward method has different behavior
All, We use StrutsLinkTool pretty much everywhere and had been relying on the previous behavior (tools 1.4) of StrutsLinkTool.setForward(String) which takes a reference to a Struts-defined "forward" which is basically just a URL. In the past, we were able to define a URL in Struts like this: Then, in the markup: link text It appears that somewhere in the modifications, the URL ha sbeen sanitized and now "/path/to/resource?foo=bar" has been helpfully replaced by "/path/to/resource%3Ffoo=bar". Was this intentional? Is there a way to get the old behavior back? Thanks, -chris signature.asc Description: OpenPGP digital signature
