That's sort of what I thought. If you have any suggestions as to how you would change this then I would be more than happy to implement then and post the patch file up to this group.
Well, the first thing that needs to go is the checking for href, page, forward, and action. These should have their own methods (like computeForwardURL) so we don't have nasty if statements in computeURL. Then the anchor and parameter handling need their own methods
The TestRequestUtils class needs to be updated to test all the new functionality so nothing breaks. We shouldn't do this for 1.1 but if you post a patch I'll look at applying it for 1.2.
In fact, we should investigate separating all URL related methods from RequestUtils into a new UrlUtils (or similarly named) class.
David
Regards
David Wilkinson
-----Original Message----- From: David Graham [mailto:[EMAIL PROTECTED] Sent: 18 June 2003 16:59 To: [EMAIL PROTECTED] Subject: RE: Problem with the html:rewrite tag
>But looking into what is happening in the code still does not make sense >to me, why is the redirect parameter being used for this encoding. >According to the following code taken from the computeURL method in >RequestUtils > > > // Perform URL rewriting to include our session ID (if any) > if (pageContext.getSession() != null) { > HttpServletResponse response = (HttpServletResponse) >pageContext.getResponse(); > if (redirect) { > return (response.encodeRedirectURL(url.toString())); > } else { > return (response.encodeURL(url.toString())); > } > } else { > return (url.toString()); > } > >the only time the url string will be encoded is if there is an active >session object and if there is then the url will be encoded regardless >just using different methods to do so. Which to me says there is an >error with the logic of using the redirect flag.
Well, you're right. It was a poor decision I made to fix a different bug. The computeURL method is already fairly gross and adding another parameter makes it even worse so rather than refactor I chose to use the redirect parameter. I'll look at changing this as soon as I can.
David
> >Regards > >David Wilkinson > > >-----Original Message----- >From: David Graham [mailto:[EMAIL PROTECTED] >Sent: 18 June 2003 14:37 >To: [EMAIL PROTECTED] >Subject: Re: Problem with the html:rewrite tag > >The ampersand is only encoded in xhtml mode because people wanted to use > ><html:rewrite> in javascript which breaks with encoded ampersands. > >David > > >Hi > > > >Since getting hold of struts 1.1 RC2 I have been getting a problem with > >the rewrite tag generating a url with the context root in twice. > >I have seen that this error has been raised as an error under > > > >http://issues.apache.org/bugzilla/show_bug.cgi?id=20835 > > > >upon looking at the code I have noticed that the class RewriteTag.java > >has been changed between RC1 and RC2 to in clued the following > > > > // Note that we're encoding the & character to & in > >XHTML mode only, > > // otherwise the & is written as is to work in >javascripts. > > url = > > >RequestUtils.computeURL( > > >pageContext, > > forward, > > href, > > page, > > null, > > params, > > anchor, > > > >!this.isXhtml()); > > > >My question is why is the isXhtml() value being passed as the redirect > >parameter to computeURL, as this does not make sense, what has > >redirecting the URL got to do with the encoding any & characters. And >if > >it is so important to do this then why was the same code not included >in > >the LinkTag which is still passing the redirect parameter as false. > > > >Regards > > > >David Wilkinson > > > >_________________________________________________________________ >Help STOP SPAM with the new MSN 8 and get 2 months FREE* >http://join.msn.com/?page=features/junkmail > > >--------------------------------------------------------------------- >To unsubscribe, e-mail: [EMAIL PROTECTED] >For additional commands, e-mail: [EMAIL PROTECTED] > > > > > >--------------------------------------------------------------------- >To unsubscribe, e-mail: [EMAIL PROTECTED] >For additional commands, e-mail: [EMAIL PROTECTED] >
_________________________________________________________________ The new MSN 8: advanced junk mail protection and 2 months FREE* http://join.msn.com/?page=features/junkmail
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
_________________________________________________________________
The new MSN 8: smart spam protection and 2 months FREE* http://join.msn.com/?page=features/junkmail
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
