On 20/02/16 02:07, Niels Charlier wrote: > On 18-02-16 21:41, Ben Caradoc-Davies wrote: >> As I hinted above, it might be a very good idea to move your path >> percent encoding into ResponseUtils.buildURL so all code can benefit. >> Path elements in URLs muse be encoded when necessary, and until now, >> GeoServer has gotten away with not doing so because all path elements >> are unreserved. > > I disagree. I think the percent encoding must be kept separate from this > method, because it can be specific from situation to situation. > Considering that a path string is provided, slashes would need to be > interpreted as path separators and thus not percent encoded. But if > someone had slashes in their string that needed to be escaped, they > would need to encode the string manually. But then this in turn would > mean that buildURL would do a duplicated percent encoding... which give > a wrong result. Sure there are always ways to work around it. But for > maximum flexibility I would keep the things in two separated methods > that can each be called easily.
Niels, I think you are right. Your encoder could be moved to ResponseUtils so more could benefit, but perhaps the decision to use it depends on the situation. Is there an alternative implementation in HttpClient / HttpComponents already on the classpath? >> Also, ResponseUtils.urlEncode and ResponseUtils.urlDecode use: >> URLEncoder.encode(value, "ISO-8859-1") >> URLDecoder.decode(value, "ISO-8859-1") >> which I think should be changed to "UTF-8". Leaving this as >> "ISO-8859-1" is going to break a lot more things. > Agreed, this is plain wrong. UTF-8 is the standard for percent encoding. Should check the HTML spec to see that this not mandated. I think it was updated to UTF-8 but I am not sure. Kind regards, -- Ben Caradoc-Davies <b...@transient.nz> Director Transient Software Limited <http://transient.nz/> New Zealand ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 _______________________________________________ Geoserver-devel mailing list Geoserver-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geoserver-devel
