Didn't see test for nullity, but comment is disturbing
On Fri, Aug 30, 2013 at 10:29 PM, Philippe Mouawad < [email protected]> wrote: > Hello sebb, > See my comments below. > Regards > > On Fri, Aug 30, 2013 at 2:00 AM, <[email protected]> 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.
