Bug 58137 - JMeter fails to download embedded URLS that contain illegal characters in URL (it does not escape them)
Summary: JMeter fails to download embedded URLS that contain illegal characters in URL...
Status: RESOLVED FIXED
Alias: None
Product: JMeter
Classification: Unclassified
Component: HTTP (show other bugs)
Version: 2.13
Hardware: All All
: P2 normal (vote)
Target Milestone: ---
Assignee: JMeter issues mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-14 19:32 UTC by Philippe Mouawad
Modified: 2015-08-21 12:40 UTC (History)
1 user (show)



Attachments
Test plan showing issue (4.93 KB, application/xml)
2015-07-14 19:32 UTC, Philippe Mouawad
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Mouawad 2015-07-14 19:32:01 UTC
Created attachment 32904 [details]
Test plan showing issue

Issue found here:

http://stackoverflow.com/questions/31401399/jmeter-url-encoded-embedded-resources


To reproduce :
Put this in <Tomcat>/examples/index.jsp:

<!--
  Licensed to the Apache Software Foundation (ASF) under one or more
  contributor license agreements.  See the NOTICE file distributed with
  this work for additional information regarding copyright ownership.
  The ASF licenses this file to You under the Apache License, Version 2.0
  (the "License"); you may not use this file except in compliance with
  the License.  You may obtain a copy of the License at

      http://www.apache.org/licenses/LICENSE-2.0

  Unless required by applicable law or agreed to in writing, software
  distributed under the License is distributed on an "AS IS" BASIS,
  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  See the License for the specific language governing permissions and
  limitations under the License.
-->
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<HTML><HEAD><TITLE>Apache Tomcat Examples</TITLE>
<META http-equiv=Content-Type content="text/html">
</HEAD>
<BODY>
<P>
<H3>Apache Tomcat Examples</H3>
<P></P>
<ul>
<img src="http://www.ubik-ingenierie.com/wp-content/uploads/2015/03/logo_ubik_ingenierie.png?getItem={someID}" ></img>
<li><a href="servlets">Servlets examples</a></li>
<li><a href="jsp">JSP Examples</a></li>
</ul>
</BODY></HTML>

and use attached test plan
Comment 1 Philippe Mouawad 2015-07-14 21:37:31 UTC
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

Author: pmouawad
Date: Tue Jul 14 21:32:41 2015
New Revision: 1691091

URL: http://svn.apache.org/r1691091
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/xdocs/changes.xml
Comment 2 Sebb 2015-07-16 11:40:36 UTC
The problem with fixing this is that it will break any existing tests that rely on the path being pre-encoded.

In fact the component reference explicitly says:

"Also please note that the path is not encoded - apart from replacing spaces with %20 - so unsafe characters may need to be encoded to avoid errors such as URISyntaxException."

The fix is wrong, and must be reverted.

However, the docs could be updated with info on how to encode the path.
Comment 3 Sebb 2015-07-18 11:35:17 UTC
(In reply to Sebb from comment #2)
> The problem with fixing this is that it will break any existing tests that
> rely on the path being pre-encoded.
> 
> In fact the component reference explicitly says:
> 
> "Also please note that the path is not encoded - apart from replacing spaces
> with %20 - so unsafe characters may need to be encoded to avoid errors such
> as URISyntaxException."
> 

Ignore the above: I misread the issue.

However, AFAICT the sample JSP uses invalid syntax for the URL, so should not just be unconditionally corrected. Apart from hiding the invalid syntax, it may cause problems for correctly encoded URLs.
Comment 4 Sebb 2015-07-23 12:48:19 UTC
A further issue is that this breaks the behaviour of JMeter for file URLs.

At present, it is possible to use relative file names with the file protocol.

The current fix makes it impossible to use relative file URLs.

A simple fix is to only apply the additional fix-ups for non-file protocols.

However I still think it is wrong to silently correct invalid URLs.
Comment 5 UbikLoadPack support 2015-07-23 12:50:18 UTC
Hi sebb,
Isn't is an issue not to fix it , as those "broken/illegal" URL are ok for browsers.

See:
http://stackoverflow.com/questions/31401399/jmeter-url-encoded-embedded-resources

User is happy with fix, so should it really be reverted this way ?

Thanks
Comment 6 Sebb 2015-07-23 13:11:06 UTC
The fact that the originator of the SO question is happy with the fix only proves that the fix works for their test cases. It does not say anything about other use cases. 

As the SO question itself says, the URL needs to be encoded in the HTML source.
If this were done, then the error would disappear.

I don't think it is right to silently correct embedded URLs.

Apart from the fact that this hides the problem, any 'fix' applied by JMeter might well cause issues in other cases (as has been seen with file URLs).

Thus it is vital that the user is made aware that a fix has been applied to a given URL, and that the user can disable the fix if necessary. And as already noted, the fix must not be applied to file URLs.
Comment 7 Milamber 2015-08-01 17:07:20 UTC
Theses changes breaks the unit tests when the ant distribution task works.

batchtest:
     [echo] Starting HTMLParserTestFile_2 using -X
   [jmeter] Creating summariser <summary>
   [jmeter] Created the tree successfully using testfiles/HTMLParserTestFile_2.jmx
   [jmeter] Starting the test @ Sat Aug 01 18:06:04 WEST 2015 (1438448764996)
   [jmeter] Waiting for possible Shutdown/StopTestNow/Heapdump message on port 4445
   [jmeter] summary =      1 in   0.4s =    2.6/s Avg:   248 Min:   248 Max:   248 Err:     0 (0.00%)
   [jmeter] Tidying up ...    @ Sat Aug 01 18:06:06 WEST 2015 (1438448766076)
   [jmeter] ... end of run
   [concat] 2015/08/01 18:06:06 ERROR - jmeter.protocol.http.sampler.HTTPSamplerBase: Error escaping URL:'file:testfiles/HTMLParserTestFile_2_files/style.css', message:Relative path in absolute URI: file://testfiles/HTMLParserTestFile_2_files/style.css 
   [concat] 2015/08/01 18:06:06 ERROR - jmeter.protocol.http.sampler.HTTPSamplerBase: Error escaping URL:'file:testfiles/HTMLParserTestFile_2_files/halfbanner.htm', message:Relative path in absolute URI: file://testfiles/HTMLParserTestFile_2_files/halfbanner.htm 
   [concat] 2015/08/01 18:06:06 ERROR - jmeter.protocol.http.sampler.HTTPSamplerBase: Error escaping URL:'file:testfiles/HTMLParserTestFile_2_files/halfbanner_data/2011-na-234x60.png', message:Relative path in absolute URI: file://testfiles/HTMLParserTestFile_2_files/halfbanner_data/2011-na-234x60.png 
   [concat] 2015/08/01 18:06:06 ERROR - jmeter.protocol.http.sampler.HTTPSamplerBase: Error escaping URL:'file:testfiles/HTMLParserTestFile_2_files/jakarta-logo.gif', message:Relative path in absolute URI: file://testfiles/HTMLParserTestFile_2_files/jakarta-logo.gif 
   [concat] 2015/08/01 18:06:06 ERROR - jmeter.protocol.http.sampler.HTTPSamplerBase: Error escaping URL:'file:testfiles/HTMLParserTestFile_2_files/logo.jpg', message:Relative path in absolute URI: file://testfiles/HTMLParserTestFile_2_files/logo.jpg 
   [concat] 2015/08/01 18:06:06 ERROR - jmeter.protocol.http.sampler.HTTPSamplerBase: Error escaping URL:'file:testfiles/HTMLParserTestFile_2_files/http-config-example.png', message:Relative path in absolute URI: file://testfiles/HTMLParserTestFile_2_files/http-config-example.png 
   [concat] 2015/08/01 18:06:06 ERROR - jmeter.protocol.http.sampler.HTTPSamplerBase: Error escaping URL:'file:testfiles/HTMLParserTestFile_2_files/scoping1.png', message:Relative path in absolute URI: file://testfiles/HTMLParserTestFile_2_files/scoping1.png 
   [concat] 2015/08/01 18:06:06 ERROR - jmeter.protocol.http.sampler.HTTPSamplerBase: Error escaping URL:'file:testfiles/HTMLParserTestFile_2_files/scoping2.png', message:Relative path in absolute URI: file://testfiles/HTMLParserTestFile_2_files/scoping2.png 
   [concat] 2015/08/01 18:06:06 ERROR - jmeter.protocol.http.sampler.HTTPSamplerBase: Error escaping URL:'file:testfiles/HTMLParserTestFile_2_files/scoping3.png', message:Relative path in absolute URI: file://testfiles/HTMLParserTestFile_2_files/scoping3.png 
     [echo] HTMLParserTestFile_2 output files compared OK



(In reply to Philippe Mouawad from comment #1)
> 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
> 
> Author: pmouawad
> Date: Tue Jul 14 21:32:41 2015
> New Revision: 1691091
> 
> URL: http://svn.apache.org/r1691091
> 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/xdocs/changes.xml
Comment 8 Felix Schumacher 2015-08-02 13:55:04 UTC
With the following changes file urls will not be escaped and all urls that were escaped will be logged as a warning.

Date: Sun Aug  2 13:52:42 2015
New Revision: 1693814

URL: http://svn.apache.org/r1693814
Log:
Bug 58137: Don't escape file protocol urls
Bugzilla Id: 58137

Modified:
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java

Date: Sun Aug  2 13:53:18 2015
New Revision: 1693815

URL: http://svn.apache.org/r1693815
Log:
Bug 58137: Warn about urls that had to be escaped.
Bugzilla Id: 58137

Modified:
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java