Mailing-List: contact dev-help@jmeter.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@jmeter.apache.org
MIME-Version: 1.0
In-Reply-To: <CAOGo0VaGoMDdXA47YjmKaF1xCyu3nj_q=XfZ+sopR8X=utJtpw@mail.gmail.com>
References: <20170227075744.350353A059D@svn01-us-west.apache.org> <CAOGo0VaGoMDdXA47YjmKaF1xCyu3nj_q=XfZ+sopR8X=utJtpw@mail.gmail.com>
From: Philippe Mouawad <philippe.mouawad@gmail.com>
Date: Mon, 27 Feb 2017 12:26:09 +0100
Message-ID: <CAH9fUpZUiXGBtOtWZfcPndXCOkjus5UAWYvB-PpU1B9y1jxD5w@mail.gmail.com>
Subject: Re: svn commit: r1784506 - in /jmeter/trunk: build.xml
 src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java xdocs/changes.xml
To: "dev@jmeter.apache.org" <dev@jmeter.apache.org>
Content-Type: multipart/alternative; boundary=001a113ba366a602eb0549815824
archived-at: Mon, 27 Feb 2017 11:26:17 -0000

--001a113ba366a602eb0549815824
Content-Type: text/plain; charset=UTF-8

On Mon, Feb 27, 2017 at 11:17 AM, sebb <sebbaz@gmail.com> wrote:

> On 27 February 2017 at 07:57,  <pmouawad@apache.org> wrote:
> > Author: pmouawad
> > Date: Mon Feb 27 07:57:43 2017
> > New Revision: 1784506
> >
> > URL: http://svn.apache.org/viewvc?rev=1784506&view=rev
> > Log:
> > Bug 60778 - Http Java Impl does not show Authorization header in
> SampleResult even if it is sent
> > Bugzilla Id: 60778
> >
> > Modified:
> >     jmeter/trunk/build.xml
> >     jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/
> http/sampler/HTTPJavaImpl.java
> >     jmeter/trunk/xdocs/changes.xml
> >
> > Modified: jmeter/trunk/build.xml
> > URL: http://svn.apache.org/viewvc/jmeter/trunk/build.xml?rev=1784
> 506&r1=1784505&r2=1784506&view=diff
> > ============================================================
> ==================
> > --- jmeter/trunk/build.xml (original)
> > +++ jmeter/trunk/build.xml Mon Feb 27 07:57:43 2017
> > @@ -2739,6 +2739,13 @@ run JMeter unless all the JMeter jars ar
> >        <param name="batchtest.jmx" value="Http4ImplPreemptiveBasi
> cAuth.jmx"/>
> >      </antcall>
> >
> > +       <antcall target="batchtest">
> > +      <param name="batchtest.name" value="Http4ImplPreemptiveBasi
> cAuth"/>
> > +      <param name="batchtest.jmx" value="Http4ImplPreemptiveBasi
> cAuth.jmx"/>
> > +         <param name="batchtest.variable" value="jmeter.httpsampler"/>
> > +         <param name="batchtest.value" value="Java"/>
> > +    </antcall>
> > +
> >      <antcall target="batchtest">
> >        <param name="batchtest.name" value="TestKeepAlive"/>
> >        <param name="batchtest.jmx" value="TestKeepAlive.jmx"/>
> >
> > Modified: jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/
> http/sampler/HTTPJavaImpl.java
> > URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/
> org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.
> java?rev=1784506&r1=1784505&r2=1784506&view=diff
> > ============================================================
> ==================
> > --- jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java
> (original)
> > +++ jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java
> Mon Feb 27 07:57:43 2017
> > @@ -26,6 +26,7 @@ import java.net.InetSocketAddress;
> >  import java.net.Proxy;
> >  import java.net.URL;
> >  import java.net.URLConnection;
> > +import java.util.HashMap;
> >  import java.util.List;
> >  import java.util.Map;
> >  import java.util.zip.GZIPInputStream;
> > @@ -63,7 +64,6 @@ public class HTTPJavaImpl extends HTTPAb
> >
> >      static {
> >          log.info("Maximum connection retries = {}", MAX_CONN_RETRIES);
> // $NON-NLS-1$
> > -        // Temporary copies, so can set the final ones
> >      }
> >
> >      private static final byte[] NULL_BA = new byte[0];// can share these
> > @@ -183,6 +183,9 @@ public class HTTPJavaImpl extends HTTPAb
> >          // with the last request to an HTTP server. Instead, most
> browsers
> >          // leave it to the server to close the connection after their
> >          // timeout period. Leave it to the JMeter user to decide.
> > +        // Ensure System property "" is set to true to allow headers
>
> Missing property name
>
Good catch , fixed

>
> > +        // such as "Host" and "Connection" to be passed through.
> > +        // See http://bugs.java.com/bugdataba
> se/view_bug.do?bug_id=6996110
> >          if (getUseKeepAlive()) {
> >              conn.setRequestProperty(HTTPConstants.HEADER_CONNECTION,
> HTTPConstants.KEEP_ALIVE);
> >          } else {
> > @@ -193,7 +196,7 @@ public class HTTPJavaImpl extends HTTPAb
> >          setConnectionHeaders(conn, u, getHeaderManager(),
> getCacheManager());
> >          String cookies = setConnectionCookie(conn, u,
> getCookieManager());
> >
> > -        setConnectionAuthorization(conn, u, getAuthManager());
> > +        Map<String, String> securityHeaders =
> setConnectionAuthorization(conn, u, getAuthManager());
> >
> >          if (method.equals(HTTPConstants.POST)) {
> >              setPostHeaders(conn);
> > @@ -202,7 +205,7 @@ public class HTTPJavaImpl extends HTTPAb
> >          }
> >
> >          if (res != null) {
> > -            res.setRequestHeaders(getConnectionHeaders(conn));
> > +            res.setRequestHeaders(getConnectionHeaders(conn,
> securityHeaders));
> >              res.setCookies(cookies);
> >          }
> >
> > @@ -383,9 +386,10 @@ public class HTTPJavaImpl extends HTTPAb
> >       * @param conn
> >       *            <code>HttpUrlConnection</code> which represents the
> URL
> >       *            request
> > +     * @param securityHeaders Map of security Header or null
> >       * @return the headers as a string
> >       */
> > -    private String getConnectionHeaders(HttpURLConnection conn) {
> > +    private String getConnectionHeaders(HttpURLConnection conn,
> Map<String, String> securityHeaders) {
> >          // Get all the request properties, which are the headers set on
> the connection
> >          StringBuilder hdrs = new StringBuilder(100);
> >          Map<String, List<String>> requestHeaders =
> conn.getRequestProperties();
> > @@ -402,6 +406,14 @@ public class HTTPJavaImpl extends HTTPAb
> >                  }
> >              }
> >          }
> > +        if(securityHeaders != null) {
> > +            for(Map.Entry<String, String> entry :
> securityHeaders.entrySet()) {
> > +                hdrs.append(entry.getKey())
> > +                .append(": ") // $NON-NLS-1$
> > +                .append(entry.getValue())
> > +                .append("\n"); // $NON-NLS-1$
> > +            }
> > +        }
> >          return hdrs.toString();
> >      }
> >
> > @@ -417,14 +429,20 @@ public class HTTPJavaImpl extends HTTPAb
> >       * @param authManager
> >       *            the <code>AuthManager</code> containing all the
> cookies for
> >       *            this <code>UrlConfig</code>
> > +     * @return String Authorization header value or null if not set
> >       */
> > -    private void setConnectionAuthorization(HttpURLConnection conn,
> URL u, AuthManager authManager) {
> > +    private Map<String, String> setConnectionAuthorization(HttpURLConnection
> conn, URL u, AuthManager authManager) {
> >          if (authManager != null) {
> >              Authorization auth = authManager.getAuthForURL(u);
> >              if (auth != null) {
> > -                conn.setRequestProperty(HTTPConstants.HEADER_AUTHORIZATION,
> auth.toBasicHeader());
> > +                String headerValue = auth.toBasicHeader();
> > +                conn.setRequestProperty(HTTPConstants.HEADER_AUTHORIZATION,
> headerValue);
> > +                Map<String, String> map = new HashMap<>(1);
> > +                map.put(HTTPConstants.HEADER_AUTHORIZATION,
> headerValue);
> > +                return map;
> >              }
> >          }
> > +        return null;
>
> It'a almost always better to return an empty array or collection
> instead of null.
> Simplifies the code as callers don't need to check for null.
>
I usually agree with such rule but not here as it will result in a lot of
object creation for nothing.

>
> >      }
> >
> >      /**
> >
> > Modified: jmeter/trunk/xdocs/changes.xml
> > URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?
> rev=1784506&r1=1784505&r2=1784506&view=diff
> > ============================================================
> ==================
> > --- jmeter/trunk/xdocs/changes.xml [utf-8] (original)
> > +++ jmeter/trunk/xdocs/changes.xml [utf-8] Mon Feb 27 07:57:43 2017
> > @@ -243,6 +243,7 @@ JMeter now requires Java 8. Ensure you u
> >      <li><bug>60690</bug>Default values for
> "httpclient4.validate_after_inactivity" and "httpclient4.time_to_live"
> which are equal to each other makes validation useless</li>
> >      <li><bug>60758</bug>HTTP(s) Test Script Recorder : Number request
> may generate duplicate numbers. Contributed by Ubik Load Pack (support at
> ubikloadpack.com)</li>
> >      <li><bug>56939</bug>Parameters are not passed with OPTIONS HTTP
> Request</li>
> > +    <li><bug>60778</bug>Http Java Impl does not show Authorization
> header in SampleResult even if it is sent</li>
> >  </ul>
> >
> >  <h3>Other Samplers</h3>
> >
> >
>


-- 
Cordialement.
Philippe Mouawad.

--001a113ba366a602eb0549815824--