Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP Proxy Server throws an exception when path contains "|" character #2978

Closed
asfimport opened this issue Nov 13, 2012 · 21 comments
Closed

Comments

@asfimport
Copy link
Collaborator

Marek (Bug 54142):
I've have some application to test which is working fine, but during recording process with JMeter when I select some link, proxy rises an exception (visible only in web browser):

java.net.URISyntaxException: Illegal character in path at index 51: http://10.133.47.78:8080/comarch-cm/cm/showAccounts|C|21737.treeGroupNode|G|21731.treeGroupNode|G|21691.?clickContext=tree
at java.net.URI$Parser.fail(Unknown Source)
at java.net.URI$Parser.checkChars(Unknown Source)
at java.net.URI$Parser.parseHierarchical(Unknown Source)
at java.net.URI$Parser.parse(Unknown Source)
at java.net.URI.<init>(Unknown Source)
at java.net.URL.toURI(Unknown Source)
at org.apache.jmeter.protocol.http.sampler.HTTPHC4Impl.sample(HTTPHC4Impl.java:232)
at org.apache.jmeter.protocol.http.sampler.HTTPSamplerProxy.sample(HTTPSamplerProxy.java:62)
at org.apache.jmeter.protocol.http.sampler.HTTPSamplerBase.sample(HTTPSamplerBase.java:1075)
at org.apache.jmeter.protocol.http.proxy.Proxy.run(Proxy.java:212)

I'm including a short version of recording.

Below is the content of problematic GET request from "View Results Tree" attached to "HTTP Proxy Server" (HTTP view):
Method GET
Protocol http
Host 10.133.47.78
Port 8080
Path /comarch-cm/cm/showAccounts|C|21737.treeGroupNode|G|21731.treeGroupNode|G|21691.

clickContext	tree

Looks like "|" character character is problematic here.

Created attachment bugReportRecord.jmx: Result of recording when exception was raised

bugReportRecord.jmx
<?xml version="1.0" encoding="UTF-8"?>
<jmeterTestPlan version="1.2" properties="2.3" jmeter="2.8 r1393162">
  <hashTree>
    <TestPlan guiclass="TestPlanGui" testclass="TestPlan" testname="Some tests" enabled="true">
      <stringProp name="TestPlan.comments"></stringProp>
      <boolProp name="TestPlan.functional_mode">false</boolProp>
      <boolProp name="TestPlan.serialize_threadgroups">false</boolProp>
      <elementProp name="TestPlan.user_defined_variables" elementType="Arguments" guiclass="ArgumentsPanel" testclass="Arguments" testname="Zmienne zdefiniowane przez użytkownika" enabled="true">
        <collectionProp name="Arguments.arguments"/>
      </elementProp>
      <stringProp name="TestPlan.user_define_classpath"></stringProp>
    </TestPlan>
    <hashTree>
      <ConfigTestElement guiclass="HttpDefaultsGui" testclass="ConfigTestElement" testname="HTTP Request Defaults" enabled="true">
        <elementProp name="HTTPsampler.Arguments" elementType="Arguments" guiclass="HTTPArgumentsPanel" testclass="Arguments" testname="User Defined Variables" enabled="true">
          <collectionProp name="Arguments.arguments"/>
        </elementProp>
        <stringProp name="HTTPSampler.domain">10.133.47.78</stringProp>
        <stringProp name="HTTPSampler.port">8080</stringProp>
        <stringProp name="HTTPSampler.connect_timeout"></stringProp>
        <stringProp name="HTTPSampler.response_timeout"></stringProp>
        <stringProp name="HTTPSampler.protocol"></stringProp>
        <stringProp name="HTTPSampler.contentEncoding"></stringProp>
        <stringProp name="HTTPSampler.path"></stringProp>
        <stringProp name="HTTPSampler.concurrentPool">4</stringProp>
      </ConfigTestElement>
      <hashTree/>
      <ThreadGroup guiclass="ThreadGroupGui" testclass="ThreadGroup" testname="Thread Group" enabled="true">
        <stringProp name="ThreadGroup.on_sample_error">continue</stringProp>
        <elementProp name="ThreadGroup.main_controller" elementType="LoopController" guiclass="LoopControlPanel" testclass="LoopController" testname="Loop Controller" enabled="true">
          <boolProp name="LoopController.continue_forever">false</boolProp>
          <stringProp name="LoopController.loops">1</stringProp>
        </elementProp>
        <stringProp name="ThreadGroup.num_threads">1</stringProp>
        <stringProp name="ThreadGroup.ramp_time">1</stringProp>
        <longProp name="ThreadGroup.start_time">1352735023000</longProp>
        <longProp name="ThreadGroup.end_time">1352735023000</longProp>
        <boolProp name="ThreadGroup.scheduler">false</boolProp>
        <stringProp name="ThreadGroup.duration"></stringProp>
        <stringProp name="ThreadGroup.delay"></stringProp>
      </ThreadGroup>
      <hashTree>
        <TransactionController guiclass="TransactionControllerGui" testclass="TransactionController" testname="/comarch-cm/cm/showCustomers|C|21732.treeGroupNode|G|21731.treeGroupNode|G|21691." enabled="true">
          <boolProp name="TransactionController.parent">false</boolProp>
        </TransactionController>
        <hashTree>
          <HTTPSamplerProxy guiclass="HttpTestSampleGui" testclass="HTTPSamplerProxy" testname="/comarch-cm/cm/showCustomers|C|21732.treeGroupNode|G|21731.treeGroupNode|G|21691." enabled="true">
            <elementProp name="HTTPsampler.Arguments" elementType="Arguments" guiclass="HTTPArgumentsPanel" testclass="Arguments" enabled="true">
              <collectionProp name="Arguments.arguments">
                <elementProp name="clickContext" elementType="HTTPArgument">
                  <boolProp name="HTTPArgument.always_encode">false</boolProp>
                  <stringProp name="Argument.name">clickContext</stringProp>
                  <stringProp name="Argument.value">tree</stringProp>
                  <stringProp name="Argument.metadata">=</stringProp>
                  <boolProp name="HTTPArgument.use_equals">true</boolProp>
                </elementProp>
              </collectionProp>
            </elementProp>
            <stringProp name="HTTPSampler.domain"></stringProp>
            <stringProp name="HTTPSampler.port"></stringProp>
            <stringProp name="HTTPSampler.connect_timeout"></stringProp>
            <stringProp name="HTTPSampler.response_timeout"></stringProp>
            <stringProp name="HTTPSampler.protocol">http</stringProp>
            <stringProp name="HTTPSampler.contentEncoding"></stringProp>
            <stringProp name="HTTPSampler.path">/comarch-cm/cm/showCustomers|C|21732.treeGroupNode|G|21731.treeGroupNode|G|21691.</stringProp>
            <stringProp name="HTTPSampler.method">GET</stringProp>
            <boolProp name="HTTPSampler.follow_redirects">true</boolProp>
            <boolProp name="HTTPSampler.auto_redirects">false</boolProp>
            <boolProp name="HTTPSampler.use_keepalive">true</boolProp>
            <boolProp name="HTTPSampler.DO_MULTIPART_POST">false</boolProp>
            <stringProp name="HTTPSampler.implementation">HttpClient4</stringProp>
            <boolProp name="HTTPSampler.monitor">false</boolProp>
            <stringProp name="HTTPSampler.embedded_url_re"></stringProp>
          </HTTPSamplerProxy>
          <hashTree>
            <HeaderManager guiclass="HeaderPanel" testclass="HeaderManager" testname="HTTP Header Manager" enabled="true">
              <collectionProp name="HeaderManager.headers">
                <elementProp name="Accept-Language" elementType="Header">
                  <stringProp name="Header.name">Accept-Language</stringProp>
                  <stringProp name="Header.value">pl,en-us;q=0.7,en;q=0.3</stringProp>
                </elementProp>
                <elementProp name="Accept" elementType="Header">
                  <stringProp name="Header.name">Accept</stringProp>
                  <stringProp name="Header.value">text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8</stringProp>
                </elementProp>
                <elementProp name="User-Agent" elementType="Header">
                  <stringProp name="Header.name">User-Agent</stringProp>
                  <stringProp name="Header.value">Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/20100101 Firefox/16.0</stringProp>
                </elementProp>
                <elementProp name="Referer" elementType="Header">
                  <stringProp name="Header.name">Referer</stringProp>
                  <stringProp name="Header.value">http://10.133.47.78:8080/comarch-cm/cm/</stringProp>
                </elementProp>
                <elementProp name="Accept-Encoding" elementType="Header">
                  <stringProp name="Header.name">Accept-Encoding</stringProp>
                  <stringProp name="Header.value">gzip, deflate</stringProp>
                </elementProp>
              </collectionProp>
            </HeaderManager>
            <hashTree/>
          </hashTree>
        </hashTree>
      </hashTree>
    </hashTree>
  </hashTree>
</jmeterTestPlan>

Votes in Bugzilla: 1
Severity: major
OS: All

Depends on:

@asfimport
Copy link
Collaborator Author

Marek (migrated from Bugzilla):
Ok. I found something like that: http://tools.ietf.org/html/rfc2396#section-2.4.3
So "|" character is classified as "unwise"! Does it mean that JMeter should treat this character as incorrect? I'm not convinced.

Anyway I will report problem to application owners.

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
"|" seems to be an unsafe character and should have been encoded in your case , read http://www.faqs.org/rfcs/rfc1738.html,:
"Other characters are unsafe because
gateways and other transport agents are known to sometimes modify
such characters. These characters are "{", "}", "|", "", "^", "~",
"[", "]", and "`".

All unsafe characters must always be encoded within a URL."

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
Java uses RFC2396 to check URL.
In this case, | is not valid.

@asfimport
Copy link
Collaborator Author

Marek (migrated from Bugzilla):
Hi,

I understand that this character is classified as "unwise" ("unsafe"), and all new web application must not use it.
On other hand there are some old web application which are using that character in path (like mine).
IMO JMeter should support old web applications, which do not respect this rule. Instead throwing an exception and making recording of test case impossible (or extremely complicated) it should do something else like:

  • escape those problematic characters (modify request)
  • or ignore the problem (add some warnings)

I can understand that you have different opinion and that is why decided to mark report as "WONTFIX", but can you at least give me a general hint how/where can I fix it? This way I could create my own private branch of JMeter.

@asfimport
Copy link
Collaborator Author

Marek (migrated from Bugzilla):
Small update in case if someone else accouter same problem.
This issue is not related to JMeter or web application.
It is web browser problem.

I've encounter this problem when using Fire Fox.
When I've tried Internet Explorer the problematic "|" character was replaced with "%7C" and recording of test was successful.

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
Thanks for feedback.

Regarding your previous comment, we usually do what you describe and tend to help as much as possible users with workarounds.
But in this case I had no idea :-)

Regards
Philippe

@asfimport
Copy link
Collaborator Author

Marek (migrated from Bugzilla):
Thanks.
Under IE problem reappear in later stage, when JavaScript with this character kick in, so I have to find workaround.
If I find it, then I will post it here.

@asfimport
Copy link
Collaborator Author

Sebb (migrated from Bugzilla):
I can confirm that FireFox fails to encode URLs containing | when entered in the location field. IMO that is a bug in Firefox.

However Opera, Chrome and IE do encode the "|" character when entered in the location field.

==

The Proxy intercepts the outgoing request, which should already have been encoded.

In general it's not possible to safely re-encode a URL, because the encoding process introduces % characters which themselves need encoding.

For example /| should be presented as /%7C.

If this is re-encoded, it will encode the % again.

@asfimport
Copy link
Collaborator Author

Marek (migrated from Bugzilla):
Created attachment HTTPHC4Impl.java.patch: path to fixing the unwise characters problem

HTTPHC4Impl.java.patch
Index: src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java
===================================================================
--- src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java	(revision 1418369)
+++ src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java	(working copy)
@@ -231,7 +231,14 @@
         
         HttpRequestBase httpRequest = null;
         try {
-            URI uri = url.toURI();
+            URI uri = new URI(url.getProtocol(), 
+                              null /*userInfo*/,
+                              url.getHost(), 
+                              url.getPort(), 
+                              url.getPath(), 
+                              url.getQuery(), 
+                              null /*fragment*/);
+
             if (method.equals(HTTPConstants.POST)) {
                 httpRequest = new HttpPost(uri);
             } else if (method.equals(HTTPConstants.PUT)) {
@@ -249,7 +256,10 @@
             } else if (method.equals(HTTPConstants.PATCH)) {
                 httpRequest = new HttpPatch(uri);
             } else {
-                throw new IllegalArgumentException("Unexpected method: "+method);
+                throw new IllegalArgumentException(String.format("Unexpected method: \"%s\" for url: \"%s\" frameDepth=%d", 
+                                                                 method,
+                                                                 url.toString(),
+                                                                 frameDepth));
             }
             setupRequest(url, httpRequest, res); // can throw IOException
         } catch (Exception e) {

@asfimport
Copy link
Collaborator Author

Marek (migrated from Bugzilla):
Hi,

I've included patch which fixes this issue.
Please review that and consider to release that to trunk.
Fix is very simple.
Root cause of the problem is in behavior of java.net.URL.toURI() method which throws exception when simple encoding of forbidden/unwise character could do the job.
Patch doesn't contain new test cases to cover corrected functionality.

BR,

Marek Ruszczak

@asfimport
Copy link
Collaborator Author

Marek (migrated from Bugzilla):
Hi,

I've noticed that there is problem with encoding url.
For example FireFox will encode spaces but will not encode unwise characters ("|").
So there is danger of encoding something which was already encoded. To overcome this problem I'm first decoding uri (path and query parts) and then encode everything (including unwise characters).
Choice of encoding (UTF-8/ANSI-ASCII/...) is problematic, so defensively someone should review my patch.

I've notice that this also can be fixed in: org.apache.jmeter.protocol.http.sampler.HTTPSamplerBase.getUrl
it might be better place to fix it but for me it is hard to evaluate.

BR,

Marek

Created attachment HTTPHC4Impl.java.patch: Corrected patch - double encoding problem

HTTPHC4Impl.java.patch
Index: src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java
===================================================================
--- src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java	(revision 1418369)
+++ src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java	(working copy)
@@ -231,7 +231,21 @@
         
         HttpRequestBase httpRequest = null;
         try {
-            URI uri = url.toURI();
+            String urlContentEncoding = getContentEncodingOrNull();
+            if(urlContentEncoding == null || urlContentEncoding.length() == 0) {
+                // Use the default encoding for urls
+                urlContentEncoding = EncoderCache.URL_ARGUMENT_ENCODING;
+            }
+
+            URI uri = new URI(url.getProtocol(), 
+                              null /*userInfo*/,
+                              url.getHost(), 
+                              url.getPort(), 
+                              // path and query might be already encoded so to prevent double encoding first it is decoded
+                              URLDecoder.decode(url.getPath(), urlContentEncoding), 
+                              URLDecoder.decode(url.getQuery(), urlContentEncoding), 
+                              null /*fragment*/);
+
             if (method.equals(HTTPConstants.POST)) {
                 httpRequest = new HttpPost(uri);
             } else if (method.equals(HTTPConstants.PUT)) {
@@ -249,7 +263,10 @@
             } else if (method.equals(HTTPConstants.PATCH)) {
                 httpRequest = new HttpPatch(uri);
             } else {
-                throw new IllegalArgumentException("Unexpected method: "+method);
+                throw new IllegalArgumentException(String.format("Unexpected method: \"%s\" for url: \"%s\" frameDepth=%d", 
+                                                                 method,
+                                                                 url.toString(),
+                                                                 frameDepth));
             }
             setupRequest(url, httpRequest, res); // can throw IOException
         } catch (Exception e) {

@asfimport
Copy link
Collaborator Author

Sebb (migrated from Bugzilla):
The problem only arises when using the Proxy Server.

However the patch affects all usage of the HC4 implementation. I think this is wrong:

  • we should not mess with existing test plans using HC4
  • it may not work for other HTTP implementations
  • it's unnecessary work for most samples

Any patch that is applied needs to be for the Proxy Server only.

But I do agree that decoding and then re-encoding the URL should avoid the double-encoding issue as mentioned in comment 8.

It would be sensible to check if the URL is valid before attempting to fix it, rather than unconditionally "fixing" each URL. This would avoid unnecessary conversions possibly changing the URL.

This would be slightly more work for the invalid case - but the work is done by the Proxy Server, not as part of a test - and is the minimal work required to work round what is a browser bug.

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
You should try to apply your fix in DefaultSamplerCreator#computePath() method.

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
Fixed as part of #3048 fix

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
Date: Mon Feb 4 22:05:10 2013
New Revision: 1442395

URL: http://svn.apache.org/viewvc?rev=1442395&view=rev
Log:
#2978 - HTTP Proxy Server throws an exception when path contains "|" character
#2978

Modified:
jmeter/trunk/xdocs/changes.xml

@asfimport
Copy link
Collaborator Author

Marek (migrated from Bugzilla):
I've tested it and it works perfectly.
Thanks a lot.

BR,

Marek

PS. I'm not sure if I should change status to VERIFIED/FIXED so I leave it as it is.

@asfimport
Copy link
Collaborator Author

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
Date: Thu Apr 11 20:24:48 2013
New Revision: 1467074

URL: http://svn.apache.org/r1467074
Log:
Rollback to fix of bugs:
54482- HC fails to follow redirects with non-encoded chars
54293- JMeter rejects html tags '<' in query params as invalid when they are accepted by the browser
54142- HTTP Proxy Server throws an exception when path contains "|" character

#3048

Modified:
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC3Impl.java
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java
jmeter/trunk/xdocs/changes.xml

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
Patch proposition which escapes IllegalURLCharacters if browser does not.
The idea is to convert to URI the URL, if it fails then it means it contains unsafe characters, then they are fixed.

My issue with this patch is that it fixes the issues for HttpClient3 and 4 but it also fixes for Java which could break

Created attachment HttpRequestHdr.java.patch: Patch for issue fixing unsafe URLs during parsing

HttpRequestHdr.java.patch
Index: src/protocol/http/org/apache/jmeter/protocol/http/proxy/HttpRequestHdr.java
===================================================================
--- src/protocol/http/org/apache/jmeter/protocol/http/proxy/HttpRequestHdr.java	(revision 1509948)
+++ src/protocol/http/org/apache/jmeter/protocol/http/proxy/HttpRequestHdr.java	(working copy)
@@ -21,7 +21,9 @@
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.URI;
 import java.net.URL;
+import java.net.URLDecoder;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.StringTokenizer;
@@ -167,9 +169,31 @@
         if (url.startsWith("/")) {
             url = HTTPS + "://" + paramHttps + url; // $NON-NLS-1$
         }
+        try {
+            URI testCleanUri = new URI(url);
+        } catch (Exception e) {
+            log.info("Url contains unsafe characters");
+            try {
+                url = escapeIllegalURLCharacters(url);
+            } catch (Exception e1) {
+                log.error("Error sanitzing URL:"+url);
+            }
+        }
         log.debug("First Line: " + url);
     }
 
+    /**
+     * @param url
+     * @return
+     * @throws Exception
+     */
+    public static String escapeIllegalURLCharacters(String url) throws Exception{
+        String decodeUrl = URLDecoder.decode(url,"UTF-8");
+        URL urlString = new URL(decodeUrl);
+        URI uri = new URI(urlString.getProtocol(), urlString.getUserInfo(), urlString.getHost(), urlString.getPort(), urlString.getPath(), urlString.getQuery(), urlString.getRef());
+        return uri.toString();
+    }
+    
     /*
      * Split line into name/value pairs and store in headers if relevant
      * If name = "content-length", then return value as int, else return 0

@asfimport
Copy link
Collaborator Author

Sebb (migrated from Bugzilla):
(In reply to Philippe Mouawad from comment 19)

Created attachment 30670 [details]
Patch for issue fixing unsafe URLs during parsing

Patch proposition which escapes IllegalURLCharacters if browser does not.
The idea is to convert to URI the URL, if it fails then it means it contains
unsafe characters, then they are fixed.

Seems OK.
The conversion method could be added to ConversionUtils.
It needs some test cases.

My issue with this patch is that it fixes the issues for HttpClient3 and 4
but it also fixes for Java which could break

Not sure I follow.

The purpose of this patch is to fix up URLs generated by browsers that don't encode all unsafe characters. It should be equivalent to using a well-behaved browser currently.

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
Date: Wed Aug 7 21:25:45 2013
New Revision: 1511503

URL: http://svn.apache.org/r1511503
Log:
#2978 - HTTP Proxy Server throws an exception when path contains "|" character
Integrated my patch with a slight change to make current behaviour with Java Impl remain the same as bug only affects HC impls
#2978

Modified:
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/HttpRequestHdr.java
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/util/ConversionUtils.java
jmeter/trunk/xdocs/changes.xml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant