Fix Gihub issue 496

(#496)

The solutions are detailed in the issue and pull request
comments.

For threading problem, I centralized all of the connection management
code into a new class called 'HTTPConnections'.  I then carefully
synchronized it.  I then modified other httpservices module code to
utilize this class.

For the UrlAuthenticationDialog problem, I reverted
'UrlAuthenticatorDialog' to its original form that has no internal
credentials database.

cdm-test/src/test/java/ucar/nc2/util/net/Login.java

@@ -0,0 +1,165 @@
+/*
+ * Copyright 1998-2009 University Corporation for Atmospheric Research/Unidata
+ *
+ * Portions of this software were developed by the Unidata Program at the
+ * University Corporation for Atmospheric Research.
+ *
+ * Access and use of this software shall impose the following obligations
+ * and understandings on the user. The user is granted the right, without
+ * any fee or cost, to use, copy, modify, alter, enhance and distribute
+ * this software, and any derivative works thereof, and its supporting
+ * documentation for any purpose whatsoever, provided that this entire
+ * notice appears in all copies of the software, derivative works and
+ * supporting documentation.  Further, UCAR requests that the user credit
+ * UCAR/Unidata in any publications that result from the use of this
+ * software or in any product that includes this software. The names UCAR
+ * and/or Unidata, however, may not be used in any advertising or publicity
+ * to endorse or promote any products or commercial entity unless specific
+ * written permission is obtained from UCAR/Unidata. The user also
+ * understands that UCAR/Unidata is not obligated to provide the user with
+ * any support, consulting, training or assistance of any kind with regard
+ * to the use, operation and performance of this software nor to provide
+ * the user with any updates, revisions, new versions or "bug fixes."
+ *
+ * THIS SOFTWARE IS PROVIDED BY UCAR/UNIDATA "AS IS" AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL UCAR/UNIDATA BE LIABLE FOR ANY SPECIAL,
+ * INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+ * FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+ * WITH THE ACCESS, USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+package ucar.nc2.util.net;
+
+import org.apache.http.auth.AuthScope;
+import org.apache.http.auth.Credentials;
+import org.apache.http.auth.UsernamePasswordCredentials;
+import org.apache.http.client.CredentialsProvider;
+import ucar.httpservices.HTTPAuthUtil;
+
+import javax.swing.*;
+import java.awt.*;
+import java.util.HashMap;
+import java.util.Map;
+
+public class Login extends JFrame implements CredentialsProvider
+{
+    static final boolean DEBUG = true;
+
+    protected String dfaltuser = null;
+    protected String dfaltpwd = null;
+
+  // In order to act as a proper CredentialsProvider, this class
+  // needs to support the setCredentials operation properly.
+  // This means that it must provide an internal database of
+  // AuthScope->Credentials.
+  // Because this class is interactive, entries in the internal database
+  // must only be set by explicit calls to setCredentials().
+  // Why do we need this? Because this provider will be used everywhere
+  // in the path to the server to provide credentials at intermediate point.
+  // So, it must be possible to set proxy credentials as well as 
+  // possible redirections (e.g. OAUTH2).
+
+  private Map<AuthScope, Credentials> creds = new HashMap<>();
+
+
+    /**
+     * constructor
+     */
+
+    public Login(String user, String pwd)
+    {
+        this.setAlwaysOnTop(true);
+        this.dfaltuser = user;
+        this.dfaltpwd = pwd;
+    }
+
+    public void clear()
+    {
+      this.creds.clear();
+    }
+
+    public void setCredentials(AuthScope scope, Credentials cred)
+    {
+      this.creds.put(scope, cred);
+    }
+
+    public Credentials getCredentials(AuthScope scope)
+    {
+     Credentials credentials = this.creds.get(scope);
+     if(credentials == null) {
+       AuthScope bestMatch = HTTPAuthUtil.bestmatch(scope,this.creds.keySet());
+       if(bestMatch != null)
+         credentials = this.creds.get(bestMatch);
+    }
+    if(credentials != null)
+      return credentials;
+
+        String up = login(this.dfaltuser, this.dfaltpwd, scope);
+        if(up == null) throw new IllegalStateException();
+        String[] pieces = up.split("[:]");
+        UsernamePasswordCredentials upc = null;
+        if(pieces.length == 2) {
+            if(DEBUG) {
+                System.out.println("user= (" + pieces[0] + ")");
+                System.out.println("password= (" + pieces[1] + ")");
+            }
+            // Is this really necessary?
+            upc = new UsernamePasswordCredentials(pieces[0], pieces[1]);
+        } else
+            upc = null;
+        return upc;
+    }
+
+    protected String login(String defaltuser, String dfaltpwd, AuthScope scope)
+    {
+        String user;
+        String pwd;
+        for(; ; ) {
+            user = null;
+            pwd = null;
+            JPanel userPanel = new JPanel();
+            userPanel.setLayout(new GridLayout(2, 3));
+            userPanel.setPreferredSize(new Dimension(500,0));
+            JLabel usernameLbl = new JLabel("Username:");
+            JLabel passwordLbl = new JLabel("Password:");
+            JTextField userFld = new JTextField();
+            if(dfaltuser != null)
+                userFld.setText(dfaltuser);
+            JTextField passwordFld = new JTextField();
+            if(dfaltpwd != null)
+                passwordFld.setText(dfaltpwd);
+            userPanel.add(usernameLbl);
+            userPanel.add(userFld);
+            userPanel.add(new JLabel(""));
+            userPanel.add(passwordLbl);
+            userPanel.add(passwordFld);
+            userPanel.add(new JLabel(""));
+            userPanel.setVisible(true);
+            StringBuilder title = new StringBuilder();
+            title.append("Enter password for: ");
+            title.append(scope.getHost());
+            if(scope.getPort() > 0) {
+                title.append(":");
+                title.append(scope.getPort());
+            }
+            int optionindex = JOptionPane.showConfirmDialog(null, userPanel, title.toString(),
+                    JOptionPane.OK_CANCEL_OPTION, JOptionPane.PLAIN_MESSAGE);
+            switch (optionindex) {
+            case 0: // ok
+                user = userFld.getText();
+                pwd = passwordFld.getText();
+                if(user != null && pwd != null)
+                    return user + ":" + pwd;
+                break;
+            default: // treat rest as cancel
+                break;
+            }
+            break;
+        }
+        return null;
+    }
+}
+

cdm-test/src/test/java/ucar/nc2/util/net/TestFormBuilder.java

@@ -100,6 +100,7 @@ public TestFormBuilder()
         setTitle("HTTPFormBuilder test(s)");
         setSystemProperties();
         // Turn on Session debugging
+        HTTPSession.TESTING = true;
         HTTPSession.debugHeaders(false);
     }
 

cdm-test/src/test/java/ucar/nc2/util/net/TestHTTPMethod.java

@@ -37,7 +37,6 @@
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
-import ucar.httpservices.HTTPException;
 import ucar.httpservices.HTTPFactory;
 import ucar.httpservices.HTTPMethod;
 import ucar.httpservices.HTTPSession;
@@ -78,6 +77,7 @@ public TestHTTPMethod()
     {
         super();
         setTitle("HTTP Method tests");
+        HTTPSession.TESTING = true;
     }
 
     @Test
@@ -98,14 +98,8 @@ public TestHTTPMethod()
             byte[] buffer = new byte[EXPECTED];
             int count = stream.read(buffer);
             stream.close(); /* should close the method also */
-            try {
-                method.execute();
-                pass = false;
-            } catch (HTTPException he) {
-                pass = true;
-            }
+            Assert.assertTrue("TestHTTPMethod: stream close did not close method",method.isClosed());
         }
-        Assert.assertTrue("TestHTTPMethod.testGetStream", pass);
     }
 
     @Test
@@ -124,14 +118,14 @@ public TestHTTPMethod()
             InputStream stream = method.getResponseBodyAsStream();
             byte[] buffer = new byte[EXPECTED];
             int count = stream.read(buffer, 0, 10); // partial read
+            Assert.assertTrue("TestHTTPMethod: partial stream read closed ,ethod",!method.isClosed());
             method.close();
+            Assert.assertTrue("TestHTTPMetthod: method.close() did not close",method.isClosed());
             try {
                 count = stream.read(buffer);
-                pass = false;
             } catch (Throwable t) {
-                pass = true;
+                Assert.assertTrue("TestHTTPMethod: stream read after method.close()",method.isClosed());
             }
         }
-        Assert.assertTrue("TestHTTPMethod.testGetStreamPartial", pass);
     }
 }

cdm-test/src/test/java/ucar/nc2/util/net/TestHTTPSession.java

@@ -33,7 +33,6 @@
 package ucar.nc2.util.net;
 
 import org.apache.http.Header;
-import org.apache.http.HttpMessage;
 import org.apache.http.auth.Credentials;
 import org.apache.http.auth.UsernamePasswordCredentials;
 import org.apache.http.client.config.RequestConfig;
@@ -49,8 +48,6 @@
 
 import java.util.List;
 
-import static ucar.httpservices.HTTPSession.*;
-
 @Category(NeedsExternalResource.class)
 public class TestHTTPSession extends CommonTestUtils
 {
@@ -89,27 +86,27 @@ public TestHTTPSession()
         System.out.println("Test: HTTPSession.setGlobalUserAgent(" + GLOBALAGENT + ")");
 
         HTTPSession.debugHeaders(false);
-
         HTTPSession.setGlobalUserAgent(GLOBALAGENT);
         try (HTTPSession session = HTTPFactory.newSession(TESTURL1)) {
+            List<Header> agents = null;
             HTTPMethod method = HTTPFactory.Get(session, TESTURL1);
             method.execute();
             // Use special interface to access the request
             // Look for the user agent header
-            List<Header> agents = HTTPSession.debugRequestInterceptor().getHeaders(HTTPSession.HEADER_USERAGENT);
+            agents = HTTPSession.debugRequestInterceptor().getHeaders(HTTPSession.HEADER_USERAGENT);
             Assert.assertFalse("User-Agent Header not found", agents.size() == 0);
             Assert.assertFalse("Multiple User-Agent Headers", agents.size() > 1);
             Assert.assertTrue(String.format("User-Agent mismatch: expected %s found:%s",
                     GLOBALAGENT, agents.get(0).getValue()),
                     GLOBALAGENT.equals(agents.get(0).getValue()));
             System.out.println("*** Pass: set global agent");
+            //method.close();
 
             System.out.println("Test: HTTPSession.setUserAgent(" + SESSIONAGENT + ")");
             HTTPSession.debugReset();
             session.setUserAgent(SESSIONAGENT);
             method = HTTPFactory.Get(session, TESTURL1);
             method.execute();
-
             // Use special interface to access the request
             agents = HTTPSession.debugRequestInterceptor().getHeaders(HTTPSession.HEADER_USERAGENT);
             Assert.assertFalse("User-Agent Header not found", agents.size() == 0);
@@ -118,11 +115,11 @@ public TestHTTPSession()
                     SESSIONAGENT, agents.get(0).getValue()),
                     SESSIONAGENT.equals(agents.get(0).getValue()));
             System.out.println("*** Pass: set session agent");
+            method.close();
         }
     }
 
-    // Verify that other configuration parameters
-    // Can at least be set.
+    // Verify that other configuration parameters Can at least be set.
     @Test
     public void
     testConfigure() throws Exception
@@ -162,7 +159,7 @@ public TestHTTPSession()
             n = dbgcfg.getConnectTimeout();
             Assert.assertTrue("*** Fail: Connection Timeout", n == 37777);
             System.out.println("*** Pass: SO Timeout");
-
+            method.close();
         }
     }
 }

cdm-test/src/test/java/ucar/nc2/util/net/TestHttpRda.java

@@ -77,6 +77,26 @@ public static List<Object[]> getTestParameters() {
     result.add(new Object[]{"ds628.1_anl_land"});
     result.add(new Object[]{"ds628.1_anl_isentrop"});
 
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
+    result.add(new Object[]{"ds628.1_anl_isentrop"});
 
     return result;
   }
@@ -107,7 +127,7 @@ public TestHttpRda(String ds) {
     this.fullUrl = url + ds;
   }
 
-  @Ignore("Failing - HTTP hangs")
+  //@Ignore("Failing - HTTP hangs")
   @Test
   public void testSession() throws Exception {
 

cdm-test/src/test/java/ucar/nc2/util/net/TestSSH.java

@@ -66,13 +66,13 @@ public class TestSSH extends CommonTestUtils
     static protected final boolean IGNORE = true;
 
     //static protected String SERVER = "localhost:8443";
-    static protected String SERVER = TestDir.threddsTestServer;
+    static protected String SERVER = TestDir.remoteTestServer;
 
     static protected String Dkeystore = null;
     static protected String Dkeystorepassword = null;
 
     static final String[] sshurls = {
-            "https://" + SERVER + "/thredds/dodsC/testdata/testData.nc.dds"
+            "https://" + SERVER + "/thredds/dodsC/localContent/testData.nc.dds"
     };
 
     static {
@@ -194,7 +194,7 @@ public TestSSH()
         System.out.println("*** Testing: Simple Https");
 
         // Reset the ssl stores
-        HTTPSession.setGlobalSSLAuth(null, null, null, null);
+        HTTPSession.clearkeystore();
 
         for(String url : sshurls) {
             System.out.println("*** URL: " + url);
@@ -226,7 +226,7 @@ public TestSSH()
 
         // (Re-)Establish the client key store and password
         // Reset the ssl stores
-        HTTPSession.setGlobalSSLAuth(Dkeystore, Dkeystorepassword, null, null);
+        HTTPSession.rebuildkeystore(Dkeystore, Dkeystorepassword);
 
         for(String url : sshurls) {
             System.out.println("*** URL: " + url);