I think that UserNamePasswordCredentials should not allow null user
name, as well as NTCredentials should not accept null NT domain and
host. Especially NTCredentials in its present form can be too
misleading. One may assume that feeding the constructor with null value
instead of a real domain name will *somehow* make it pick up the actual
Windows domain name, which is of course not the case.

If there are no objections raised by Tuesday next week, I'll commit the
patch as is.

Oleg
Index: java/org/apache/commons/httpclient/NTCredentials.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/NTCredentials.java,v
retrieving revision 1.6.2.1
diff -u -r1.6.2.1 NTCredentials.java
--- java/org/apache/commons/httpclient/NTCredentials.java	16 Aug 2003 00:09:37 -0000	1.6.2.1
+++ java/org/apache/commons/httpclient/NTCredentials.java	22 Nov 2003 17:49:03 -0000
@@ -105,14 +105,14 @@
     public NTCredentials(String userName, String password, String host,
             String domain) {
         super(userName, password);
-        this.domain = domain;
-        this.host = host;
+        setDomain(domain);
+        setHost(host);
     }
     // ------------------------------------------------------- Instance Methods
 
 
     /**
-     * Sets the domain to authenticate with.
+     * Sets the domain to authenticate with. The domain may not be null.
      *
      * @param domain the NT domain to authenticate in.
      * 
@@ -120,6 +120,9 @@
      * 
      */
     public void setDomain(String domain) {
+        if (domain == null) {
+            throw new IllegalArgumentException("Domain may not be null");
+        }
         this.domain = domain;
     }
 
@@ -135,11 +138,16 @@
         return domain;
     }
 
-    /** Sets the host name of the computer originating the request.
+    /** 
+     * Sets the host name of the computer originating the request. The host name may
+     * not be null.
      *
      * @param host the Host the user is logged into.
      */
     public void setHost(String host) {
+        if (host == null) {
+            throw new IllegalArgumentException("Host may not be null");
+        }
         this.host = host;
     }
 
@@ -159,9 +167,10 @@
     public String toString() {
         final StringBuffer sbResult = new StringBuffer(super.toString());
         
-        sbResult.append(":");
-        sbResult.append(this.host == null ? "null" : this.host);
-        sbResult.append(this.domain == null ? "null" : this.domain);
+        sbResult.append("@");
+        sbResult.append(this.host);
+        sbResult.append(".");
+        sbResult.append(this.domain);
 
         return sbResult.toString();
     }
Index: java/org/apache/commons/httpclient/UsernamePasswordCredentials.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/UsernamePasswordCredentials.java,v
retrieving revision 1.11
diff -u -r1.11 UsernamePasswordCredentials.java
--- java/org/apache/commons/httpclient/UsernamePasswordCredentials.java	31 Jan 2003 00:33:36 -0000	1.11
+++ java/org/apache/commons/httpclient/UsernamePasswordCredentials.java	22 Nov 2003 17:49:04 -0000
@@ -108,8 +108,9 @@
      * @param password the password
      */
     public UsernamePasswordCredentials(String userName, String password) {
-        this.userName = userName;
-        this.password = password;
+        super();
+        setUserName(userName);
+        setPassword(password);
     }
 
     // ----------------------------------------------------- Instance Variables
@@ -130,12 +131,15 @@
 
 
     /**
-     * User name property setter.
+     * User name property setter. User name may not be null.
      *
      * @param userName
      * @see #getUserName()
      */
     public void setUserName(String userName) {
+        if (userName == null) {
+            throw new IllegalArgumentException("Username may not be null");            
+        }
         this.userName = userName;
     }
 
@@ -180,7 +184,7 @@
      */
     public String toString() {
         StringBuffer result = new StringBuffer();
-        result.append((this.userName == null) ? "null" : this.userName);
+        result.append(this.userName);
         result.append(":");
         result.append((this.password == null) ? "null" : this.password);
         return result.toString();
Index: test/org/apache/commons/httpclient/TestAuthenticator.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestAuthenticator.java,v
retrieving revision 1.25.2.4
diff -u -r1.25.2.4 TestAuthenticator.java
--- test/org/apache/commons/httpclient/TestAuthenticator.java	14 Nov 2003 02:26:16 -0000	1.25.2.4
+++ test/org/apache/commons/httpclient/TestAuthenticator.java	22 Nov 2003 17:49:11 -0000
@@ -123,6 +123,33 @@
 
     // ---------------------------------- Test Methods for BasicScheme Authentication
 
+    public void testCredentialConstructors() {
+        try {
+            new UsernamePasswordCredentials(null, null);
+            fail("IllegalArgumentException should have been thrown");
+        } catch (IllegalArgumentException e) {
+            // expected
+        }
+        try {
+            new NTCredentials("user", "password", null, null);
+            fail("IllegalArgumentException should have been thrown");
+        } catch (IllegalArgumentException e) {
+            // expected
+        }
+        try {
+            new NTCredentials("user", "password", "host", null);
+            fail("IllegalArgumentException should have been thrown");
+        } catch (IllegalArgumentException e) {
+            // expected
+        }
+        NTCredentials creds = new NTCredentials("user", null, "host", "domain");
+        assertNotNull(creds.getUserName());
+        assertNull(creds.getPassword());
+        assertNotNull(creds.getDomain());
+        assertNotNull(creds.getHost());
+    }
+
+
     public void testBasicAuthenticationWithNoCreds() {
         String challenge = "Basic realm=\"realm1\"";
         HttpState state = new HttpState();

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to