You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jmeter.apache.org by se...@apache.org on 2013/08/30 02:00:16 UTC

svn commit: r1518861 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java xdocs/changes.xml

Author: sebb
Date: Fri Aug 30 00:00:15 2013
New Revision: 1518861

URL: http://svn.apache.org/r1518861
Log:
Proxy should deliver failed requests to any configured Listeners
Bugzilla Id: 55506

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

Modified: jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java?rev=1518861&r1=1518860&r2=1518861&view=diff
==============================================================================
--- jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java (original)
+++ jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java Fri Aug 30 00:00:15 2013
@@ -20,6 +20,7 @@ package org.apache.jmeter.protocol.http.
 
 import java.io.BufferedInputStream;
 import java.io.BufferedOutputStream;
+import java.io.ByteArrayOutputStream;
 import java.io.DataOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
@@ -27,6 +28,7 @@ import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.io.PrintStream;
 import java.net.Socket;
 import java.net.URL;
 import java.net.UnknownHostException;
@@ -206,7 +208,15 @@ public class Proxy extends Thread {
                     throw new JMeterException(); // hack to skip processing
                 }
                 // Re-parse (now it's the http request over SSL)
-                ba = request.parse(new BufferedInputStream(clientSocket.getInputStream()));
+                try {
+                    ba = request.parse(new BufferedInputStream(clientSocket.getInputStream()));
+                } catch (IOException ioe) { // most likely this is because of a certificate error
+                    final String url = param.length>0 ?  " for '"+ param[0] +"'" : "";
+                    log.warn(port + "Problem with SSL certificate"+url+"? Ensure browser is set to accept the JMeter proxy cert: " + ioe.getMessage());
+                    // won't work: writeErrorToClient(HttpReplyHdr.formInternalError());
+                    result = generateErrorResult(result, request, ioe, "\n**ensure browser is set to accept the JMeter proxy certificate**"); // Generate result (if nec.) and populate it
+                    throw new JMeterException(); // hack to skip processing
+                }
                 if (log.isDebugEnabled()) {
                     log.debug(port + "Reparse: " + new String(ba));
                 }
@@ -244,26 +254,17 @@ public class Proxy extends Thread {
         } catch (UnknownHostException uhe) {
             log.warn(port + "Server Not Found.", uhe);
             writeErrorToClient(HttpReplyHdr.formServerNotFound());
-            result = generateErrorResult(result, uhe); // Generate result (if nec.) and populate it
+            result = generateErrorResult(result, request, uhe); // Generate result (if nec.) and populate it
         } catch (IllegalArgumentException e) {
             log.error(port + "Not implemented (probably used https)", e);
             writeErrorToClient(HttpReplyHdr.formNotImplemented("Probably used https instead of http. " +
                     "To record https requests, see " +
                     "<a href=\"http://jmeter.apache.org/usermanual/component_reference.html#HTTP_Proxy_Server\">HTTP Proxy Server documentation</a>"));
-            result = generateErrorResult(result, e); // Generate result (if nec.) and populate it
-        } catch (IOException ioe) {
-            final String url = param.length>0 ?  " for '"+ param[0] +"'" : "";
-            log.error(port + "Problem with SSL certificate"+url+"? Ensure browser is set to accept the JMeter proxy cert:", ioe);
-            // won't work: writeErrorToClient(HttpReplyHdr.formInternalError());
-            if (result == null) {
-                result = new SampleResult();
-                result.setSampleLabel("Sample failed");
-            }
-            result.setResponseMessage(ioe.getMessage()+ "\n**ensure browser is set to accept the JMeter proxy certificate**");
+            result = generateErrorResult(result, request, e); // Generate result (if nec.) and populate it
         } catch (Exception e) {
             log.error(port + "Exception when processing sample", e);
             writeErrorToClient(HttpReplyHdr.formInternalError());
-            result = generateErrorResult(result, e); // Generate result (if nec.) and populate it
+            result = generateErrorResult(result, request, e); // Generate result (if nec.) and populate it
         } finally {
             if (log.isDebugEnabled()) {
                 if(sampler != null) {
@@ -281,7 +282,8 @@ public class Proxy extends Thread {
                     headers.removeHeaderNamed(hdr);
                 }
             }
-            if(sampler != null) {
+            if(result != null) // sampler is allowed to be null
+            {
                 target.deliverSampler(sampler, new TestElement[] { captureHttpHeaders ? headers : null }, result);
             }
             try {
@@ -397,12 +399,21 @@ public class Proxy extends Thread {
         return in;
     }
 
-    private static SampleResult generateErrorResult(SampleResult result, Exception e) {
+    private SampleResult generateErrorResult(SampleResult result, HttpRequestHdr request, Exception e) {
+        return generateErrorResult(result, request, e, "");
+    }
+
+    private static SampleResult generateErrorResult(SampleResult result, HttpRequestHdr request, Exception e, String msg) {
         if (result == null) {
             result = new SampleResult();
-            result.setSampleLabel("Sample failed");
+            ByteArrayOutputStream text = new ByteArrayOutputStream(200);
+            e.printStackTrace(new PrintStream(text));
+            result.setResponseData(text.toByteArray());
+            result.setSamplerData(request.getFirstLine());
+            result.setSampleLabel(request.getUrl());
         }
-        result.setResponseMessage(e.getMessage());
+        result.setSuccessful(false);
+        result.setResponseMessage(e.getMessage()+msg);
         return result;
     }
 

Modified: jmeter/trunk/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1518861&r1=1518860&r2=1518861&view=diff
==============================================================================
--- jmeter/trunk/xdocs/changes.xml (original)
+++ jmeter/trunk/xdocs/changes.xml Fri Aug 30 00:00:15 2013
@@ -260,6 +260,7 @@ Previously the default was 1, which coul
 <li><bugzilla>55455</bugzilla> - HTTPS with HTTPClient4 ignores cps setting</li>
 <li><bugzilla>55502</bugzilla> - Proxy generates empty http:/ entries when recording</li>
 <li><bugzilla>55504</bugzilla> - Proxy incorrectly issues CONNECT requests when browser prompts for certificate override</li>
+<li><bugzilla>55506</bugzilla> - Proxy should deliver failed requests to any configured Listeners</li>
 </ul>
 
 <h3>Other Samplers</h3>



Re: svn commit: r1518861 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java xdocs/changes.xml

Posted by Philippe Mouawad <ph...@gmail.com>.
Didn't see test for nullity, but comment is disturbing


On Fri, Aug 30, 2013 at 10:29 PM, Philippe Mouawad <
philippe.mouawad@gmail.com> wrote:

> Hello sebb,
> See my comments below.
> Regards
>
> On Fri, Aug 30, 2013 at 2:00 AM, <se...@apache.org> wrote:
>
>> Author: sebb
>> Date: Fri Aug 30 00:00:15 2013
>> New Revision: 1518861
>>
>> URL: http://svn.apache.org/r1518861
>> Log:
>> Proxy should deliver failed requests to any configured Listeners
>> Bugzilla Id: 55506
>>
>> Modified:
>>
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java
>>     jmeter/trunk/xdocs/changes.xml
>>
>> Modified:
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java
>> URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java?rev=1518861&r1=1518860&r2=1518861&view=diff
>>
>> ==============================================================================
>> ---
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java
>> (original)
>> +++
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java
>> Fri Aug 30 00:00:15 2013
>> @@ -20,6 +20,7 @@ package org.apache.jmeter.protocol.http.
>>
>>  import java.io.BufferedInputStream;
>>  import java.io.BufferedOutputStream;
>> +import java.io.ByteArrayOutputStream;
>>  import java.io.DataOutputStream;
>>  import java.io.File;
>>  import java.io.FileInputStream;
>> @@ -27,6 +28,7 @@ import java.io.FileNotFoundException;
>>  import java.io.IOException;
>>  import java.io.InputStream;
>>  import java.io.OutputStream;
>> +import java.io.PrintStream;
>>  import java.net.Socket;
>>  import java.net.URL;
>>  import java.net.UnknownHostException;
>> @@ -206,7 +208,15 @@ public class Proxy extends Thread {
>>                      throw new JMeterException(); // hack to skip
>> processing
>>                  }
>>                  // Re-parse (now it's the http request over SSL)
>> -                ba = request.parse(new
>> BufferedInputStream(clientSocket.getInputStream()));
>> +                try {
>> +                    ba = request.parse(new
>> BufferedInputStream(clientSocket.getInputStream()));
>> +                } catch (IOException ioe) { // most likely this is
>> because of a certificate error
>> +                    final String url = param.length>0 ?  " for '"+
>> param[0] +"'" : "";
>> +                    log.warn(port + "Problem with SSL
>> certificate"+url+"? Ensure browser is set to accept the JMeter proxy cert:
>> " + ioe.getMessage());
>> +                    // won't work:
>> writeErrorToClient(HttpReplyHdr.formInternalError());
>> +                    result = generateErrorResult(result, request, ioe,
>> "\n**ensure browser is set to accept the JMeter proxy certificate**"); //
>> Generate result (if nec.) and populate it
>> +                    throw new JMeterException(); // hack to skip
>> processing
>> +                }
>>                  if (log.isDebugEnabled()) {
>>                      log.debug(port + "Reparse: " + new String(ba));
>>                  }
>> @@ -244,26 +254,17 @@ public class Proxy extends Thread {
>>          } catch (UnknownHostException uhe) {
>>              log.warn(port + "Server Not Found.", uhe);
>>              writeErrorToClient(HttpReplyHdr.formServerNotFound());
>> -            result = generateErrorResult(result, uhe); // Generate
>> result (if nec.) and populate it
>> +            result = generateErrorResult(result, request, uhe); //
>> Generate result (if nec.) and populate it
>>          } catch (IllegalArgumentException e) {
>>              log.error(port + "Not implemented (probably used https)", e);
>>              writeErrorToClient(HttpReplyHdr.formNotImplemented("Probably
>> used https instead of http. " +
>>                      "To record https requests, see " +
>>                      "<a href=\"
>> http://jmeter.apache.org/usermanual/component_reference.html#HTTP_Proxy_Server\">HTTP
>> Proxy Server documentation</a>"));
>> -            result = generateErrorResult(result, e); // Generate result
>> (if nec.) and populate it
>> -        } catch (IOException ioe) {
>> -            final String url = param.length>0 ?  " for '"+ param[0] +"'"
>> : "";
>> -            log.error(port + "Problem with SSL certificate"+url+"?
>> Ensure browser is set to accept the JMeter proxy cert:", ioe);
>> -            // won't work:
>> writeErrorToClient(HttpReplyHdr.formInternalError());
>> -            if (result == null) {
>> -                result = new SampleResult();
>> -                result.setSampleLabel("Sample failed");
>> -            }
>> -            result.setResponseMessage(ioe.getMessage()+ "\n**ensure
>> browser is set to accept the JMeter proxy certificate**");
>> +            result = generateErrorResult(result, request, e); //
>> Generate result (if nec.) and populate it
>>          } catch (Exception e) {
>>              log.error(port + "Exception when processing sample", e);
>>              writeErrorToClient(HttpReplyHdr.formInternalError());
>> -            result = generateErrorResult(result, e); // Generate result
>> (if nec.) and populate it
>> +            result = generateErrorResult(result, request, e); //
>> Generate result (if nec.) and populate it
>>          } finally {
>>              if (log.isDebugEnabled()) {
>>                  if(sampler != null) {
>> @@ -281,7 +282,8 @@ public class Proxy extends Thread {
>>                      headers.removeHeaderNamed(hdr);
>>                  }
>>              }
>> -            if(sampler != null) {
>> +            if(result != null) // sampler is allowed to be null
>>
>
> This seems wrong to me, as if sampler is null (in case of JMeterException
> above) you will get a NullPointerException.
>
>> +            {
>>                  target.deliverSampler(sampler, new TestElement[] {
>> captureHttpHeaders ? headers : null }, result);
>
>              }
>>              try {
>> @@ -397,12 +399,21 @@ public class Proxy extends Thread {
>>          return in;
>>      }
>>
>> -    private static SampleResult generateErrorResult(SampleResult result,
>> Exception e) {
>> +    private SampleResult generateErrorResult(SampleResult result,
>> HttpRequestHdr request, Exception e) {
>> +        return generateErrorResult(result, request, e, "");
>> +    }
>> +
>> +    private static SampleResult generateErrorResult(SampleResult result,
>> HttpRequestHdr request, Exception e, String msg) {
>>          if (result == null) {
>>              result = new SampleResult();
>> -            result.setSampleLabel("Sample failed");
>> +            ByteArrayOutputStream text = new ByteArrayOutputStream(200);
>> +            e.printStackTrace(new PrintStream(text));
>> +            result.setResponseData(text.toByteArray());
>> +            result.setSamplerData(request.getFirstLine());
>> +            result.setSampleLabel(request.getUrl());
>>          }
>> -        result.setResponseMessage(e.getMessage());
>> +        result.setSuccessful(false);
>> +        result.setResponseMessage(e.getMessage()+msg);
>>          return result;
>>      }
>>
>>
>> Modified: jmeter/trunk/xdocs/changes.xml
>> URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1518861&r1=1518860&r2=1518861&view=diff
>>
>> ==============================================================================
>> --- jmeter/trunk/xdocs/changes.xml (original)
>> +++ jmeter/trunk/xdocs/changes.xml Fri Aug 30 00:00:15 2013
>> @@ -260,6 +260,7 @@ Previously the default was 1, which coul
>>  <li><bugzilla>55455</bugzilla> - HTTPS with HTTPClient4 ignores cps
>> setting</li>
>>  <li><bugzilla>55502</bugzilla> - Proxy generates empty http:/ entries
>> when recording</li>
>>  <li><bugzilla>55504</bugzilla> - Proxy incorrectly issues CONNECT
>> requests when browser prompts for certificate override</li>
>> +<li><bugzilla>55506</bugzilla> - Proxy should deliver failed requests to
>> any configured Listeners</li>
>>  </ul>
>>
>>  <h3>Other Samplers</h3>
>>
>>
>>
>
>
> --
> Cordialement.
> Philippe Mouawad.
>
>
>


-- 
Cordialement.
Philippe Mouawad.

Re: svn commit: r1518861 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java xdocs/changes.xml

Posted by Philippe Mouawad <ph...@gmail.com>.
Hello sebb,
See my comments below.
Regards

On Fri, Aug 30, 2013 at 2:00 AM, <se...@apache.org> wrote:

> Author: sebb
> Date: Fri Aug 30 00:00:15 2013
> New Revision: 1518861
>
> URL: http://svn.apache.org/r1518861
> Log:
> Proxy should deliver failed requests to any configured Listeners
> Bugzilla Id: 55506
>
> Modified:
>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java
>     jmeter/trunk/xdocs/changes.xml
>
> Modified:
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java
> URL:
> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java?rev=1518861&r1=1518860&r2=1518861&view=diff
>
> ==============================================================================
> ---
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java
> (original)
> +++
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java
> Fri Aug 30 00:00:15 2013
> @@ -20,6 +20,7 @@ package org.apache.jmeter.protocol.http.
>
>  import java.io.BufferedInputStream;
>  import java.io.BufferedOutputStream;
> +import java.io.ByteArrayOutputStream;
>  import java.io.DataOutputStream;
>  import java.io.File;
>  import java.io.FileInputStream;
> @@ -27,6 +28,7 @@ import java.io.FileNotFoundException;
>  import java.io.IOException;
>  import java.io.InputStream;
>  import java.io.OutputStream;
> +import java.io.PrintStream;
>  import java.net.Socket;
>  import java.net.URL;
>  import java.net.UnknownHostException;
> @@ -206,7 +208,15 @@ public class Proxy extends Thread {
>                      throw new JMeterException(); // hack to skip
> processing
>                  }
>                  // Re-parse (now it's the http request over SSL)
> -                ba = request.parse(new
> BufferedInputStream(clientSocket.getInputStream()));
> +                try {
> +                    ba = request.parse(new
> BufferedInputStream(clientSocket.getInputStream()));
> +                } catch (IOException ioe) { // most likely this is
> because of a certificate error
> +                    final String url = param.length>0 ?  " for '"+
> param[0] +"'" : "";
> +                    log.warn(port + "Problem with SSL certificate"+url+"?
> Ensure browser is set to accept the JMeter proxy cert: " +
> ioe.getMessage());
> +                    // won't work:
> writeErrorToClient(HttpReplyHdr.formInternalError());
> +                    result = generateErrorResult(result, request, ioe,
> "\n**ensure browser is set to accept the JMeter proxy certificate**"); //
> Generate result (if nec.) and populate it
> +                    throw new JMeterException(); // hack to skip
> processing
> +                }
>                  if (log.isDebugEnabled()) {
>                      log.debug(port + "Reparse: " + new String(ba));
>                  }
> @@ -244,26 +254,17 @@ public class Proxy extends Thread {
>          } catch (UnknownHostException uhe) {
>              log.warn(port + "Server Not Found.", uhe);
>              writeErrorToClient(HttpReplyHdr.formServerNotFound());
> -            result = generateErrorResult(result, uhe); // Generate result
> (if nec.) and populate it
> +            result = generateErrorResult(result, request, uhe); //
> Generate result (if nec.) and populate it
>          } catch (IllegalArgumentException e) {
>              log.error(port + "Not implemented (probably used https)", e);
>              writeErrorToClient(HttpReplyHdr.formNotImplemented("Probably
> used https instead of http. " +
>                      "To record https requests, see " +
>                      "<a href=\"
> http://jmeter.apache.org/usermanual/component_reference.html#HTTP_Proxy_Server\">HTTP
> Proxy Server documentation</a>"));
> -            result = generateErrorResult(result, e); // Generate result
> (if nec.) and populate it
> -        } catch (IOException ioe) {
> -            final String url = param.length>0 ?  " for '"+ param[0] +"'"
> : "";
> -            log.error(port + "Problem with SSL certificate"+url+"? Ensure
> browser is set to accept the JMeter proxy cert:", ioe);
> -            // won't work:
> writeErrorToClient(HttpReplyHdr.formInternalError());
> -            if (result == null) {
> -                result = new SampleResult();
> -                result.setSampleLabel("Sample failed");
> -            }
> -            result.setResponseMessage(ioe.getMessage()+ "\n**ensure
> browser is set to accept the JMeter proxy certificate**");
> +            result = generateErrorResult(result, request, e); // Generate
> result (if nec.) and populate it
>          } catch (Exception e) {
>              log.error(port + "Exception when processing sample", e);
>              writeErrorToClient(HttpReplyHdr.formInternalError());
> -            result = generateErrorResult(result, e); // Generate result
> (if nec.) and populate it
> +            result = generateErrorResult(result, request, e); // Generate
> result (if nec.) and populate it
>          } finally {
>              if (log.isDebugEnabled()) {
>                  if(sampler != null) {
> @@ -281,7 +282,8 @@ public class Proxy extends Thread {
>                      headers.removeHeaderNamed(hdr);
>                  }
>              }
> -            if(sampler != null) {
> +            if(result != null) // sampler is allowed to be null
>

This seems wrong to me, as if sampler is null (in case of JMeterException
above) you will get a NullPointerException.

> +            {
>                  target.deliverSampler(sampler, new TestElement[] {
> captureHttpHeaders ? headers : null }, result);

             }
>              try {
> @@ -397,12 +399,21 @@ public class Proxy extends Thread {
>          return in;
>      }
>
> -    private static SampleResult generateErrorResult(SampleResult result,
> Exception e) {
> +    private SampleResult generateErrorResult(SampleResult result,
> HttpRequestHdr request, Exception e) {
> +        return generateErrorResult(result, request, e, "");
> +    }
> +
> +    private static SampleResult generateErrorResult(SampleResult result,
> HttpRequestHdr request, Exception e, String msg) {
>          if (result == null) {
>              result = new SampleResult();
> -            result.setSampleLabel("Sample failed");
> +            ByteArrayOutputStream text = new ByteArrayOutputStream(200);
> +            e.printStackTrace(new PrintStream(text));
> +            result.setResponseData(text.toByteArray());
> +            result.setSamplerData(request.getFirstLine());
> +            result.setSampleLabel(request.getUrl());
>          }
> -        result.setResponseMessage(e.getMessage());
> +        result.setSuccessful(false);
> +        result.setResponseMessage(e.getMessage()+msg);
>          return result;
>      }
>
>
> Modified: jmeter/trunk/xdocs/changes.xml
> URL:
> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1518861&r1=1518860&r2=1518861&view=diff
>
> ==============================================================================
> --- jmeter/trunk/xdocs/changes.xml (original)
> +++ jmeter/trunk/xdocs/changes.xml Fri Aug 30 00:00:15 2013
> @@ -260,6 +260,7 @@ Previously the default was 1, which coul
>  <li><bugzilla>55455</bugzilla> - HTTPS with HTTPClient4 ignores cps
> setting</li>
>  <li><bugzilla>55502</bugzilla> - Proxy generates empty http:/ entries
> when recording</li>
>  <li><bugzilla>55504</bugzilla> - Proxy incorrectly issues CONNECT
> requests when browser prompts for certificate override</li>
> +<li><bugzilla>55506</bugzilla> - Proxy should deliver failed requests to
> any configured Listeners</li>
>  </ul>
>
>  <h3>Other Samplers</h3>
>
>
>


-- 
Cordialement.
Philippe Mouawad.