Fixes for endpoint to string conversion - Close zeroc-ice#517 (zeroc-…

…ice#519)
pengjason · Sep 10, 2019 · c7b9f79 · c7b9f79
1 parent 1389c57
commit c7b9f79
Show file tree

Hide file tree

Showing 21 changed files with 175 additions and 11 deletions.
diff --git a/CHANGELOG-3.7.md b/CHANGELOG-3.7.md
@@ -73,6 +73,10 @@ These are the changes since Ice 3.7.2.
   not send the extension before. The C# and SChannel engines always send the
   extension. The Java-Compat mapping does not support it.
 
+- Fixed a bug in the conversion of proxy endpoints to string, the colon character
+  was not properly escaped in `--sourceAddress` or `-interface` endpoint
+  options.
+
 ## C++ Changes
 
 - Fixed IceGrid issue which could cause hangs if an IceGrid node became
@@ -159,6 +163,12 @@ These are the changes since Ice 3.7.2.
 - Fix a bug where using an empty sequence with a type that use the Python buffer
   protocol can result in an assert if running with a python debug build.
 
+## JavaScript Changes
+
+- Fix a bug in the IP endpoint initialization,  the default value for the port should
+  was `null` instead of `0`, this cause problems with endpoint string conversion, 
+  where the output included `-p null` instead of the expected `-p 0`.
+
 # Changes in Ice 3.7.2
 
 These are the changes since Ice 3.7.1.

diff --git a/cpp/src/Ice/IPEndpointI.cpp b/cpp/src/Ice/IPEndpointI.cpp
@@ -255,7 +255,18 @@ IceInternal::IPEndpointI::options() const
 
     if(isAddressValid(_sourceAddr))
     {
-        s << " --sourceAddress " << inetAddrToString(_sourceAddr);
+        const string sourceAddr = inetAddrToString(_sourceAddr);
+        bool addQuote = sourceAddr.find(':') != string::npos;
+        s << " --sourceAddress ";
+        if(addQuote)
+        {
+            s << "\"";
+        }
+        s << sourceAddr;
+        if(addQuote)
+        {
+            s << "\"";
+        }
     }
 
     return s.str();

diff --git a/cpp/src/Ice/UdpEndpointI.cpp b/cpp/src/Ice/UdpEndpointI.cpp
@@ -214,7 +214,17 @@ IceInternal::UdpEndpointI::options() const
 
     if(_mcastInterface.length() > 0)
     {
-        s << " --interface " << _mcastInterface;
+        s << " --interface ";
+        bool addQuote = _mcastInterface.find(':') != string::npos;
+        if(addQuote)
+        {
+            s << "\"";
+        }
+        s << _mcastInterface;
+        if(addQuote)
+        {
+            s << "\"";
+        }
     }
 
     if(_mcastTtl != -1)

diff --git a/cpp/test/Ice/proxy/AllTests.cpp b/cpp/test/Ice/proxy/AllTests.cpp
@@ -236,6 +236,12 @@ allTests(Test::TestHelper* helper)
     b1 = communicator->stringToProxy("test -p 6.5 -e 1.0");
     test(b1->ice_toString() == "test -t -p 6.5 -e 1.0");
 
+    b1 = communicator->stringToProxy("test:tcp --sourceAddress \"::1\"");
+    test(Ice::targetEqualTo(b1, communicator->stringToProxy(b1->ice_toString())));
+
+    b1 = communicator->stringToProxy("test:udp --sourceAddress \"::1\" --interface \"0:0:0:0:0:0:0:1%lo\"");
+    test(Ice::targetEqualTo(b1, communicator->stringToProxy(b1->ice_toString())));
+
     try
     {
         communicator->stringToProxy("test:tcp@adapterId");

diff --git a/csharp/src/Ice/IPEndpointI.cs b/csharp/src/Ice/IPEndpointI.cs
@@ -224,7 +224,18 @@ public override string options()
 
             if(sourceAddr_ != null)
             {
-                s += " --sourceAddress " + Network.endpointAddressToString(sourceAddr_);
+                string sourceAddr = Network.endpointAddressToString(sourceAddr_);
+                bool addQuote = sourceAddr.IndexOf(':') != -1;
+                s += " --sourceAddress ";
+                if(addQuote)
+                {
+                    s += "\"";
+                }
+                s += sourceAddr;
+                if(addQuote)
+                {
+                    s += "\"";
+                }
             }
 
             return s;

diff --git a/csharp/src/Ice/UdpEndpointI.cs b/csharp/src/Ice/UdpEndpointI.cs
@@ -198,7 +198,17 @@ public override string options()
 
             if(_mcastInterface.Length != 0)
             {
-                s += " --interface " + _mcastInterface;
+                bool addQuote = _mcastInterface.IndexOf(':') != -1;
+                s += " --interface ";
+                if(addQuote)
+                {
+                    s += "\"";
+                }
+                s += _mcastInterface;
+                if(addQuote)
+                {
+                    s += "\"";
+                }
             }
 
             if(_mcastTtl != -1)

diff --git a/csharp/test/Ice/proxy/AllTests.cs b/csharp/test/Ice/proxy/AllTests.cs
@@ -91,6 +91,12 @@ public static Test.MyClassPrx allTests(global::Test.TestHelper helper)
                 test(b1.ice_getIdentity().name.Equals("test") && b1.ice_getIdentity().category.Equals("category") &&
                      b1.ice_getAdapterId().Length == 0);
 
+                b1 = communicator.stringToProxy("test:tcp --sourceAddress \"::1\"");
+                test(b1.Equals(communicator.stringToProxy(b1.ToString())));
+
+                b1 = communicator.stringToProxy("test:udp --sourceAddress \"::1\" --interface \"0:0:0:0:0:0:0:1%lo\"");
+                test(b1.Equals(communicator.stringToProxy(b1.ToString())));
+
                 b1 = communicator.stringToProxy("");
                 test(b1 == null);
                 b1 = communicator.stringToProxy("\"\"");

diff --git a/java-compat/src/Ice/src/main/java/IceInternal/IPEndpointI.java b/java-compat/src/Ice/src/main/java/IceInternal/IPEndpointI.java
@@ -236,7 +236,18 @@ public String options()
 
         if(_sourceAddr != null)
         {
-            s += " --sourceAddress " + _sourceAddr.getAddress().getHostAddress();
+            String sourceAddr = _sourceAddr.getAddress().getHostAddress();
+            boolean addQuote = sourceAddr.indexOf(':') != -1;
+            s += " --sourceAddress ";
+            if(addQuote)
+            {
+                s += "\"";
+            }
+            s += sourceAddr;
+            if(addQuote)
+            {
+                s += "\"";
+            }
         }
 
         return s;

diff --git a/java-compat/src/Ice/src/main/java/IceInternal/UdpEndpointI.java b/java-compat/src/Ice/src/main/java/IceInternal/UdpEndpointI.java
@@ -221,7 +221,17 @@ public String options()
 
         if(_mcastInterface.length() != 0)
         {
-            s += " --interface " + _mcastInterface;
+            boolean addQuote = _mcastInterface.indexOf(':') != -1;
+            s += " --interface ";
+            if(addQuote)
+            {
+                s += "\"";
+            }
+            s += _mcastInterface;
+            if(addQuote)
+            {
+                s += "\"";
+            }
         }
 
         if(_mcastTtl != -1)

diff --git a/java-compat/test/src/main/java/test/Ice/proxy/AllTests.java b/java-compat/test/src/main/java/test/Ice/proxy/AllTests.java
@@ -105,6 +105,12 @@ public class AllTests
         test(b1.ice_getIdentity().name.equals("test") && b1.ice_getIdentity().category.equals("category") &&
              b1.ice_getAdapterId().length() == 0);
 
+        b1 = communicator.stringToProxy("test:tcp --sourceAddress \"::1\"");
+        test(b1.equals(communicator.stringToProxy(b1.toString())));
+
+        b1 = communicator.stringToProxy("test:udp --sourceAddress \"::1\" --interface \"0:0:0:0:0:0:0:1%lo\"");
+        test(b1.equals(communicator.stringToProxy(b1.toString())));
+
         b1 = communicator.stringToProxy("");
         test(b1 == null);
         b1 = communicator.stringToProxy("\"\"");

diff --git a/java/src/Ice/src/main/java/com/zeroc/IceInternal/IPEndpointI.java b/java/src/Ice/src/main/java/com/zeroc/IceInternal/IPEndpointI.java
@@ -240,7 +240,18 @@ public String options()
 
         if(_sourceAddr != null)
         {
-            s += " --sourceAddress " + _sourceAddr.getAddress().getHostAddress();
+            String sourceAddr = _sourceAddr.getAddress().getHostAddress();
+            s += " --sourceAddress ";
+            boolean addQuote = sourceAddr.indexOf(':') != -1;
+            if(addQuote)
+            {
+                s += "\"";
+            }
+            s += sourceAddr;
+            if(addQuote)
+            {
+                s += "\"";
+            }
         }
 
         return s;

diff --git a/java/src/Ice/src/main/java/com/zeroc/IceInternal/UdpEndpointI.java b/java/src/Ice/src/main/java/com/zeroc/IceInternal/UdpEndpointI.java
@@ -223,7 +223,17 @@ public String options()
 
         if(_mcastInterface.length() != 0)
         {
-            s += " --interface " + _mcastInterface;
+            s += " --interface ";
+            boolean addQuote = _mcastInterface.indexOf(':') != -1;
+            if(addQuote)
+            {
+                s += "\"";
+            }
+            s += _mcastInterface;
+            if(addQuote)
+            {
+                s += "\"";
+            }
         }
 
         if(_mcastTtl != -1)

diff --git a/java/test/src/main/java/test/Ice/proxy/AllTests.java b/java/test/src/main/java/test/Ice/proxy/AllTests.java
@@ -108,6 +108,12 @@ public static MyClassPrx allTests(test.TestHelper helper)
         test(b1.ice_getIdentity().name.equals("test") && b1.ice_getIdentity().category.equals("category") &&
              b1.ice_getAdapterId().length() == 0);
 
+        b1 = communicator.stringToProxy("test:tcp --sourceAddress \"::1\"");
+        test(b1.equals(communicator.stringToProxy(b1.toString())));
+
+        b1 = communicator.stringToProxy("test:udp --sourceAddress \"::1\" --interface \"0:0:0:0:0:0:0:1%lo\"");
+        test(b1.equals(communicator.stringToProxy(b1.toString())));
+
         b1 = communicator.stringToProxy("");
         test(b1 == null);
         b1 = communicator.stringToProxy("\"\"");

diff --git a/js/src/Ice/IPEndpointI.js b/js/src/Ice/IPEndpointI.js
@@ -24,7 +24,7 @@ class IPEndpointI extends Ice.EndpointI
         super();
         this._instance = instance;
         this._host = ho === undefined ? null : ho;
-        this._port = po === undefined ? null : po;
+        this._port = po === undefined ? 0 : po;
         this._sourceAddr = sa === undefined ? null : sa;
         this._connectionId = conId === undefined ? "" : conId;
     }
@@ -132,7 +132,17 @@ class IPEndpointI extends Ice.EndpointI
 
         if(this._sourceAddr !== null && this._sourceAddr.length > 0)
         {
-            s += " --sourceAddr " + this._sourceAddr;
+            s += " --sourceAddress ";
+            const addQuote = this._sourceAddr.indexOf(':') != -1;
+            if(addQuote)
+            {
+                s += "\"";
+            }
+            s += this._sourceAddr;
+            if(addQuote)
+            {
+                s += "\"";
+            }
         }
         return s;
     }

diff --git a/js/test/Ice/proxy/Client.js b/js/test/Ice/proxy/Client.js
@@ -108,6 +108,9 @@
             test(b1.ice_getIdentity().name === "test" && b1.ice_getIdentity().category === "category" &&
                  b1.ice_getAdapterId().length === 0);
 
+            b1 = communicator.stringToProxy("test:tcp --sourceAddress \"::1\"");
+            test(b1.equals(communicator.stringToProxy(b1.toString())));
+
             b1 = communicator.stringToProxy("");
             test(b1 === null);
             b1 = communicator.stringToProxy("\"\"");

diff --git a/matlab/test/Ice/proxy/AllTests.m b/matlab/test/Ice/proxy/AllTests.m
@@ -210,6 +210,10 @@
             catch ex
                 assert(isa(ex, 'Ice.EndpointParseException'));
             end
+
+            b1 = communicator.stringToProxy('test:tcp --sourceAddress "::1"');
+            assert(b1 == communicator.stringToProxy(b1.ice_toString()));
+
             fprintf('ok\n');
 
             fprintf('testing propertyToProxy... ');

diff --git a/objective-c/test/Ice/proxy/AllTests.m b/objective-c/test/Ice/proxy/AllTests.m
@@ -95,6 +95,12 @@
          [[[b1 ice_getIdentity] category] isEqualToString:@"category"] &&
          [[b1 ice_getAdapterId] length] == 0);
 
+    b1 = [communicator stringToProxy:@"test:tcp --sourceAddress \"::1\""];
+    test([b1 isEqual:[communicator stringToProxy:[b1 ice_toString]]]);
+
+    b1 = [communicator stringToProxy:@"test:udp --sourceAddress \"::1\" --interface \"0:0:0:0:0:0:0:1%lo\""];
+    test([b1 isEqual:[communicator stringToProxy:[b1 ice_toString]]]);
+
     b1 = [communicator stringToProxy:@"test@adapter"];
     test([[[b1 ice_getIdentity] name] isEqualToString:@"test"] &&
          [[[b1 ice_getIdentity] category] length] == 0 &&

diff --git a/php/test/Ice/proxy/Client.php b/php/test/Ice/proxy/Client.php
@@ -121,6 +121,12 @@ function allTests($helper)
     test($b1->ice_getIdentity()->name == "test" && $b1->ice_getIdentity()->category == "category" &&
          $b1->ice_getAdapterId() == "");
 
+    $b1 = $communicator->stringToProxy("test:tcp --sourceAddress \"::1\"");
+    test($b1 == $communicator->stringToProxy($b1->ice_toString()));
+
+    $b1 = $communicator->stringToProxy("test:udp --sourceAddress \"::1\" --interface \"0:0:0:0:0:0:0:1%lo\"");
+    test($b1 == $communicator->stringToProxy($b1->ice_toString()));
+
     $b1 = $communicator->stringToProxy("test@adapter");
     test($b1->ice_getIdentity()->name == "test" && $b1->ice_getIdentity()->category == "" &&
          $b1->ice_getAdapterId() == "adapter");
@@ -519,7 +525,6 @@ function allTests($helper)
         $ctx = array();
         $ctx["one"] = "hello";
         $ctx["two"] = "world";
-        echo count($cl->ice_fixed($connection)->ice_getContext());
         test($cl->ice_fixed($connection)->ice_getContext() == null);
         test(count($cl->ice_context($ctx)->ice_fixed($connection)->ice_getContext()) == 2);
         test($cl->ice_fixed($connection)->ice_getInvocationTimeout() == -1);

diff --git a/python/test/Ice/proxy/AllTests.py b/python/test/Ice/proxy/AllTests.py
@@ -85,6 +85,12 @@ def allTests(helper, communicator, collocated):
     test(b1.ice_getIdentity().name == "test" and b1.ice_getIdentity().category == "category" and \
          len(b1.ice_getAdapterId()) == 0)
 
+    b1 = communicator.stringToProxy("test:tcp --sourceAddress \"::1\"")
+    test(b1 == communicator.stringToProxy(b1.ice_toString()))
+
+    b1 = communicator.stringToProxy("test:udp --sourceAddress \"::1\" --interface \"0:0:0:0:0:0:0:1%lo\"")
+    test(b1 == communicator.stringToProxy(b1.ice_toString()))
+
     b1 = communicator.stringToProxy("test@adapter")
     test(b1.ice_getIdentity().name == "test" and len(b1.ice_getIdentity().category) == 0 and \
          b1.ice_getAdapterId() == "adapter")

diff --git a/ruby/test/Ice/proxy/AllTests.rb b/ruby/test/Ice/proxy/AllTests.rb
@@ -70,6 +70,12 @@ def allTests(helper, communicator)
     test(b1.ice_getIdentity().name == "test" && b1.ice_getIdentity().category == "category" && \
          b1.ice_getAdapterId().empty?)
 
+    b1 = communicator.stringToProxy("test:tcp --sourceAddress \"::1\"")
+    test(b1 == communicator.stringToProxy(b1.ice_toString()))
+
+    b1 = communicator.stringToProxy("test:udp --sourceAddress \"::1\" --interface \":0:0:0:0:0:0:0:1\"")
+    test(b1 == communicator.stringToProxy(b1.ice_toString()))
+
     b1 = communicator.stringToProxy("test@adapter")
     test(b1.ice_getIdentity().name == "test" && b1.ice_getIdentity().category.empty? && \
          b1.ice_getAdapterId() == "adapter")

diff --git a/swift/test/Ice/proxy/AllTests.swift b/swift/test/Ice/proxy/AllTests.swift
@@ -96,6 +96,12 @@ public func allTests(_ helper: TestHelper) throws -> MyClassPrx {
         b1.ice_getIdentity().category == "category" &&
         b1.ice_getAdapterId().isEmpty)
 
+    b1 = try communicator.stringToProxy("test:tcp --sourceAddress \"::1\"")!
+    try test(b1 == communicator.stringToProxy(b1.ice_toString())!)
+
+    b1 = try communicator.stringToProxy("test:udp --sourceAddress \"::1\" --interface \"0:0:0:0:0:0:0:0:1%lo\"")!
+    try test(b1 == communicator.stringToProxy(b1.ice_toString())!)
+
     try test(communicator.stringToProxy("") == nil)
     try test(communicator.stringToProxy("\"\"") == nil)