jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Philippe Mouawad <philippe.moua...@gmail.com>
Subject Re: svn commit: r1518861 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/proxy/Proxy.java xdocs/changes.xml
Date Fri, 30 Aug 2013 20:32:25 GMT
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, <sebb@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.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message