[GitHub] jmeter pull request #320: HTTPClient 4.5. migration to last APIs / Bugzilla ...

[ubikloadpack]

Github user ubikloadpack closed the pull request at:

https://github.com/apache/jmeter/pull/320


---

[GitHub] jmeter pull request #320: HTTPClient 4.5. migration to last APIs / Bugzilla ...

[ham1]

Github user ham1 commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/320#discussion_r150891448
  
--- Diff: 
src/protocol/http/org/apache/jmeter/protocol/http/api/auth/DigestParameters.java
 ---
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * 
+ */
+
+package org.apache.jmeter.protocol.http.api.auth;
+
+/**
+ * Allows digest customization
+ * @since 4.0
+ */
+public class DigestParameters {
+public static final String VARIABLE_NAME = "__jmeter_DP__";
+private String qop;
+private String nonce;
+private String charset;
+private String algorithm;
+private String opaque;
+/**
+ * 
--- End diff --

I think this class would be better without any of the JavaDoc comments, 
they do not add anything because the class is so simple there's nothing which 
needs commenting.


---

[GitHub] jmeter pull request #320: HTTPClient 4.5. migration to last APIs / Bugzilla ...

[ham1]

Github user ham1 commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/320#discussion_r150905177
  
--- Diff: 
src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java ---
@@ -215,22 +269,128 @@ public long getKeepAliveDuration(HttpResponse 
response, HttpContext context) {
 
 };
 
-/**
- * Special interceptor made to keep metrics when connection is 
released for some method like HEAD
- * Otherwise calling directly ((HttpConnection) 
localContext.getAttribute(HttpCoreContext.HTTP_CONNECTION)).getMetrics();
- * would throw org.apache.http.impl.conn.ConnectionShutdownException
- * See https://bz.apache.org/jira/browse/HTTPCLIENT-1081;>HTTPCLIENT-1081
- */
-private static final HttpResponseInterceptor METRICS_SAVER = 
(HttpResponse response, HttpContext context) -> {
-HttpConnectionMetrics metrics = ((HttpConnection) 
context.getAttribute(HttpCoreContext.HTTP_CONNECTION)).getMetrics();
-context.setAttribute(CONTEXT_METRICS, metrics);
-};
-private static final HttpRequestInterceptor METRICS_RESETTER = 
(HttpRequest request, HttpContext context) -> {
-HttpConnectionMetrics metrics = ((HttpConnection) 
context.getAttribute(HttpCoreContext.HTTP_CONNECTION)).getMetrics();
-metrics.reset();
+private static final String DIGEST_PARAMETERS = 
DigestParameters.VARIABLE_NAME;
+
+
+private static final HttpRequestInterceptor 
PREEMPTIVE_AUTH_INTERCEPTOR = new HttpRequestInterceptor() {
--- End diff --

This is almost 100 lines and with lots of nesting becomes very hard to read 
and review, could it be split into smaller methods?


---

[GitHub] jmeter pull request #320: HTTPClient 4.5. migration to last APIs / Bugzilla ...

[ham1]

Github user ham1 commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/320#discussion_r150895140
  
--- Diff: 
src/protocol/http/org/apache/jmeter/protocol/http/control/DynamicKerberosSchemeFactory.java
 ---
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.jmeter.protocol.http.control;
+
+import org.apache.http.auth.AuthScheme;
+import org.apache.http.impl.auth.KerberosScheme;
+import org.apache.http.impl.auth.KerberosSchemeFactory;
+import org.apache.http.protocol.HttpContext;
+
+/**
+ * Extends {@link KerberosSchemeFactory} to provide ability to customize 
stripPort
+ * setting in {@link KerberosScheme} based on {@link HttpContext}
+ */
+public class DynamicKerberosSchemeFactory extends KerberosSchemeFactory {
+static final String CONTEXT_ATTRIBUTE_STRIP_PORT = "__jmeter.K_SP__";
+
+/**
+ * @since 4.0
+ */
+public DynamicKerberosSchemeFactory(final boolean stripPort, final 
boolean useCanonicalHostname) {
+super(stripPort, useCanonicalHostname);
+}
+
+@Override
+public AuthScheme create(final HttpContext context) {
+Boolean localStripPort = (Boolean) 
context.getAttribute(CONTEXT_ATTRIBUTE_STRIP_PORT);
+return new KerberosScheme(localStripPort != null ? 
--- End diff --

might be better as a variable rather than a multi-line ternary operator as 
a parameter?
e.g.
```java
Boolean stripPort = localStripPort != null ? localStripPort : isStripPort();
return new KerberosScheme(stripPort, isUseCanonicalHostname());
```


---

[GitHub] jmeter pull request #320: HTTPClient 4.5. migration to last APIs / Bugzilla ...

[ham1]

Github user ham1 commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/320#discussion_r150891784
  
--- Diff: 
src/protocol/http/org/apache/jmeter/protocol/http/control/AuthManager.java ---
@@ -97,10 +94,33 @@
 private static final boolean DEFAULT_CLEAR_VALUE = false;
 
 /** Decides whether port should be omitted from SPN for kerberos 
spnego authentication */
-private static final boolean STRIP_PORT = 
JMeterUtils.getPropDefault("kerberos.spnego.strip_port", true);
+public static final boolean STRIP_PORT = 
JMeterUtils.getPropDefault("kerberos.spnego.strip_port", true);
+
+/** Decides whether port should be omitted from SPN for kerberos 
spnego authentication */
+public static final boolean USE_CANONICAL_HOST_NAME = 
JMeterUtils.getPropDefault("kerberos.spnego.use_canonical_host_name", true);
 
 public enum Mechanism {
-BASIC_DIGEST, KERBEROS
+/**
+ * @deprecated (use {@link Mechanism#BASIC})
+ */
+@Deprecated 
+BASIC_DIGEST,
+/**
+ * Basic Auth
+ */
+BASIC, 
+/**
+ * Digest Auth
+ */
+DIGEST, 
+/**
+ * Kerberos Auth
+ */
+KERBEROS
+}
+
+public static void main(String[] args) {
+System.out.println(Mechanism.values());
--- End diff --

Should this be here?


---

[GitHub] jmeter pull request #320: HTTPClient 4.5. migration to last APIs / Bugzilla ...

[ubikloadpack]

GitHub user ubikloadpack opened a pull request:

https://github.com/apache/jmeter/pull/320

HTTPClient 4.5. migration to last APIs / Bugzilla 58757 & 61664

Hello,
Find in this PR the migration to last HC4 APIs and the fix of Digest Auth 
that was only partially implemented.

This PR fixes:
- https://bz.apache.org/bugzilla/show_bug.cgi?id=61664 
- https://bz.apache.org/bugzilla/show_bug.cgi?id=58757

Regards
Philippe M.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/ubikloadpack/jmeter HC4_FULL_MIGRATION

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/320.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #320


commit 702c8308c2b2db3152e4ef7c1c35747bc686fdf0
Author: pmouawad 
Date:   2017-10-09T21:20:26Z

Migration

commit 1e120df142c2380fdf52298dba9301261907186d
Author: pmouawad 
Date:   2017-10-12T08:32:36Z

Remove deprecated use of VIRTUAL_HOST

commit 92bdafa0e7ba197670f3f278113ff559e29e16ee
Author: pmouawad 
Date:   2017-10-13T19:29:59Z

Migrate Slow Socket feature to new APIs

commit e179a7c17e3c094f3b17436cbf74b360df64a288
Author: pmouawad 
Date:   2017-10-13T19:53:37Z

Use non deprecated constructor in ViewableFileBody
Drop call to
post.getParams().setParameter(CoreProtocolPNames.HTTP_CONTENT_CHARSET,
contentEncoding);

commit f30b574dfe53b6cfb6157877e56669db41eafdb7
Author: pmouawad 
Date:   2017-10-13T19:55:00Z

Merge remote-tracking branch 'upstream/trunk' into HC4_FULL_MIGRATION

commit b1773d9ee3d7444071cd88b02a6170365cbd1b47
Author: pmouawad 
Date:   2017-10-13T22:22:06Z

Implement LazyLayeredConnectionSocketFactory
Migrate Bandwidth measurement

commit 3910c2a8630d0b928d9c61f53a6934eee532ca45
Author: pmouawad 
Date:   2017-10-14T20:28:15Z

Adapt test cases after migration to HC4.5.x new APIs

commit f7164b7cfcdb6158493b8c789f55147c3977
Author: pmouawad 
Date:   2017-10-27T19:56:22Z

Temp code

commit b27bb3caf8f189938c5f8d322df7570d81f0ae79
Author: pmouawad 
Date:   2017-10-27T20:02:36Z

Merge current trunk

commit c6cc9d86df139c4d635547d272ec45457528ff74
Author: pmouawad 
Date:   2017-10-28T22:30:24Z

Merge remote-tracking branch 'upstream/trunk' into HC4_FULL_MIGRATION

commit e6d18781bf04d3bc02fe1291e2376356626a8fd3
Author: pmouawad 
Date:   2017-10-28T22:31:23Z

Add Basic and Digest preemptive auth

commit 4b25ac3c49e064b3508ed1ff443ef1ae23bde89b
Author: pmouawad 
Date:   2017-10-29T09:14:49Z

Sonar cleanups, upgrade ViewableFileBody

commit e2f4c72ed411059564fac94561e6a32a493a382e
Author: pmouawad 
Date:   2017-10-30T17:32:35Z

Fix logger

commit 799d65f6fdf2d074ea46fa10523fc8ac2605cd77
Author: pmouawad 
Date:   2017-10-30T17:32:55Z

Deprecate classes

commit 713474625d8f81b1dfa5a3a76c03844e05935e54
Author: pmouawad 
Date:   2017-10-30T17:33:10Z

Deprecate classes

commit e6293b3267cca306fda060bbdf7e852e387d3f8f
Author: pmouawad 
Date:   2017-10-30T17:35:06Z

Some refactoring
Plugin Proxy Auth
Start Kerberos

commit 96e60fcaff96ac145a828b79190ac2ced1b56048
Author: pmouawad 
Date:   2017-11-04T14:19:29Z

Merge remote-tracking branch 'upstream/trunk' into HC4_FULL_MIGRATION

commit 5e9df36d9ce869eacd99784e79a9e6131892d045
Author: pmouawad 
Date:   2017-11-05T08:42:37Z

Merge remote-tracking branch 'upstream/trunk' into HC4_FULL_MIGRATION

commit d929454a9917565582bc4a361d5095c90a4911ef
Author: pmouawad 
Date:   2017-11-05T08:47:42Z

Complete Kerberos
Use constants

commit e02e8fa64d364e64a1c081e207567d02e975fbf5
Author: pmouawad 
Date:   2017-11-05T08:48:53Z

Complete Kerberos
Use constants

commit 42699f997cfbd1287fff64be77991e3c6e790fbc
Author: pmouawad 
Date:   2017-11-05T11:58:10Z

Fix wrong default

commit 9f710d9f9840b547dc9a1fd93c2ea9420c18c2a8
Author: pmouawad 
Date:   2017-11-05T14:24:22Z

Finalize

commit 234b21fd480daf07cef1e6fc8729bede60e493cb
Author: pmouawad 
Date:   2017-11-05T20:17:53Z

Merge remote-tracking branch 'upstream/trunk' into HC4_FULL_MIGRATION

commit