Roland Weber wrote:
Folks, this looks like an API error to me. The protocol security
should depend on the actual type of the factory passed to the
constructor, not on the type of the variable it is stored in!

I agree. It is a design error to use polymorphism when the only difference in the method signature is a type within the same class hierarchy. Also the two constructors have duplicate code. The attached patch takes care of the problem in CVS HEAD.


Odi
Index: API_CHANGES_2_1.txt
===================================================================
RCS file: /home/cvspublic/jakarta-commons/httpclient/API_CHANGES_2_1.txt,v
retrieving revision 1.3
diff -u -r1.3 API_CHANGES_2_1.txt
--- API_CHANGES_2_1.txt 18 Sep 2003 13:56:21 -0000      1.3
+++ API_CHANGES_2_1.txt 6 Nov 2003 08:58:09 -0000
@@ -31,3 +31,5 @@
     state of DigestScheme:
     - createDigest(String, String)
     - createDigestHeader(String, String)
+    
+* There is only one Protocol constructor for both secure and insecure socket 
factories.
Index: src/java/org/apache/commons/httpclient/protocol/Protocol.java
===================================================================
RCS file: 
/home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/protocol/Protocol.java,v
retrieving revision 1.5
diff -u -r1.5 Protocol.java
--- src/java/org/apache/commons/httpclient/protocol/Protocol.java       31 Jan 2003 
00:33:36 -0000      1.5
+++ src/java/org/apache/commons/httpclient/protocol/Protocol.java       6 Nov 2003 
08:58:16 -0000
@@ -194,7 +194,8 @@
     private boolean secure;
   
     /**
-     * Constructs a new Protocol.  The created protcol is insecure.
+     * Constructs a new Protocol. If the created protocol is secure depends on
+     * the class of <code>factory</code>.
      * 
      * @param scheme the scheme (e.g. http, https)
      * @param factory the factory for creating sockets for communication using
@@ -216,36 +217,9 @@
         this.scheme = scheme;
         this.socketFactory = factory;
         this.defaultPort = defaultPort;
-        this.secure = false;
+        this.secure = (factory instanceof SecureProtocolSocketFactory);
     }
     
-    /**
-     * Constructs a new Protocol.  The created protcol is secure.
-     *
-     * @param scheme the scheme (e.g. http, https)
-     * @param factory the factory for creating sockets for communication using
-     * this protocol
-     * @param defaultPort the port this protocol defaults to
-     */
-    public Protocol(String scheme, 
-        SecureProtocolSocketFactory factory, int defaultPort) {
-        
-        if (scheme == null) {
-            throw new IllegalArgumentException("scheme is null");
-        }
-        if (factory == null) {
-            throw new IllegalArgumentException("socketFactory is null");
-        }
-        if (defaultPort <= 0) {
-            throw new IllegalArgumentException("port is invalid: " + defaultPort);
-        }
-
-        this.scheme = scheme;
-        this.socketFactory = factory;
-        this.defaultPort = defaultPort;
-        this.secure = true;        
-    }
-
     /**
      * Returns the defaultPort.
      * @return int

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

Reply via email to