On 18 July 2015 at 12:42, Philippe Mouawad <philippe.moua...@gmail.com> wrote: > Hi sebb, > My answers below. > Regards > > On Saturday, July 18, 2015, sebb <seb...@gmail.com> wrote: > >> On 16 July 2015 at 23:26, Philippe Mouawad <philippe.moua...@gmail.com >> <javascript:;>> wrote: >> > On Thursday, July 16, 2015, sebb <seb...@gmail.com <javascript:;>> >> wrote: >> > >> >> -1 >> >> >> >> This may break existing test plans, >> > >> > >> > Are you sure, I only updated the part that concerns embedded download. >> >> Sorry, I thought this related to fixing URLs provided in the "Path" field. >> >> However, I am still wary of changing the processing. >> If the embedded URL has incorrect syntax, then why should JMeter fix it? > > The "broken" url work correctly in browsers, you can test with my test > content. > And note currently JMeter "partially fixes" spaces > >> >> At the very least, I think JMeter should warn the user that the URL was >> invalid. >> That way there is at least a chance that it can be fixed.
This is still true. If the URL is silently fixed, how is it ever going to be reported and fixed at source? >> >> > It's a real issue for some users, and should be fixed. >> >> Or the app developers should fix the URLs so that they are valid. >> >> Not always possible and I am not sure url is illegal If it's not illegal, why is it being rejected? >> > >> >> and is contrary to the >> >> component_reference documentation. >> > >> > This can be updated >> >> If the patch relates to embedded downloads then the component ref docs >> don't need to be changed. >> >> > >> >> >> >> On 14 July 2015 at 22:31, <pmoua...@apache.org <javascript:;> >> <javascript:;>> wrote: >> >> > Author: pmouawad >> >> > Date: Tue Jul 14 21:31:34 2015 >> >> > New Revision: 1691090 >> >> > >> >> > URL: http://svn.apache.org/r1691090 >> >> > Log: >> >> > Bug 58137 - JMeter fails to download embedded URLS that contain >> illegal >> >> characters in URL (it does not escape them) >> >> > Bugzilla Id: 58137 >> >> > >> >> > Modified: >> >> > >> >> >> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java >> >> > >> >> > Modified: >> >> >> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java >> >> > URL: >> >> >> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java?rev=1691090&r1=1691089&r2=1691090&view=diff >> >> > >> >> >> ============================================================================== >> >> > --- >> >> >> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java >> >> (original) >> >> > +++ >> >> >> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java >> >> Tue Jul 14 21:31:34 2015 >> >> > @@ -1242,7 +1242,7 @@ public abstract class HTTPSamplerBase ex >> >> > log.warn("Null URL detected (should not >> >> happen)"); >> >> > } else { >> >> > String urlstr = url.toString(); >> >> > - String urlStrEnc=encodeSpaces(urlstr); >> >> > + String >> >> urlStrEnc=escapeIllegalURLCharacters(encodeSpaces(urlstr)); >> >> > if (!urlstr.equals(urlStrEnc)){// There were >> >> some spaces in the URL >> >> > try { >> >> > url = new URL(urlStrEnc); >> >> > @@ -1352,6 +1352,23 @@ public abstract class HTTPSamplerBase ex >> >> > } >> >> > >> >> > /** >> >> > + * @param url URL to escape >> >> > + * @return escaped url >> >> > + */ >> >> > + private String escapeIllegalURLCharacters(String url) { >> >> > + try { >> >> > + String escapedUrl = >> >> ConversionUtils.escapeIllegalURLCharacters(url); >> >> > + if(log.isDebugEnabled()) { >> >> > + log.debug("Successfully escaped url:'"+url +"' >> >> to:'"+escapedUrl+"'"); >> >> > + } >> >> > + return escapedUrl; >> >> > + } catch (Exception e1) { >> >> > + log.error("Error escaping URL:'"+url+"', >> >> message:"+e1.getMessage()); >> >> > + return url; >> >> > + } >> >> > + } >> >> > + >> >> > + /** >> >> > * Extract User-Agent header value >> >> > * @param sampleResult HTTPSampleResult >> >> > * @return User Agent part >> >> > >> >> > >> >> >> > >> > >> > -- >> > Cordialement. >> > Philippe Mouawad. >> > > > -- > Cordialement. > Philippe Mouawad.
