Implement #1813 for C++ (#1814)

zeroc-ice · Feb 20, 2024 · 046377b · 046377b
1 parent 67d81c5
commit 046377b
Show file tree

Hide file tree

Showing 30 changed files with 93 additions and 156 deletions.
diff --git a/cpp/include/Ice/LocalException.h b/cpp/include/Ice/LocalException.h
@@ -1196,25 +1196,13 @@ class ICE_CLASS(ICE_API) IllegalIdentityException : public LocalExceptionHelper<
     {
     }
 
-    /**
-     * One-shot constructor to initialize all data members.
-     * The file and line number are required for all local exceptions.
-     * @param file The file name in which the exception was raised, typically __FILE__.
-     * @param line The line number at which the exception was raised, typically __LINE__.
-     * @param id The illegal identity.
-     */
-    IllegalIdentityException(const char* file, int line, const Identity& id) : LocalExceptionHelper<IllegalIdentityException, LocalException>(file, line),
-        id(id)
-    {
-    }
-
     /**
      * Obtains a tuple containing all of the exception's data members.
      * @return The data members in a tuple.
      */
-    std::tuple<const ::Ice::Identity&> ice_tuple() const
+    std::tuple<> ice_tuple() const
     {
-        return std::tie(id);
+        return std::tie();
     }
 
     /**
@@ -1227,11 +1215,6 @@ class ICE_CLASS(ICE_API) IllegalIdentityException : public LocalExceptionHelper<
      * @param stream The target stream.
      */
     ICE_MEMBER(ICE_API) virtual void ice_print(::std::ostream& stream) const override;
-
-    /**
-     * The illegal identity.
-     */
-    ::Ice::Identity id;
 };
 
 /**

diff --git a/cpp/src/Ice/CheckIdentity.h b/cpp/src/Ice/CheckIdentity.h
@@ -0,0 +1,21 @@
+//
+// Copyright (c) ZeroC, Inc. All rights reserved.
+//
+
+#include "Ice/Identity.h"
+#include "Ice/LocalException.h"
+
+namespace Ice
+{
+
+// An identity with an empty name is illegal for all Ice APIs: no Ice API returns such an identity, and no Ice API
+// accepts such an identity argument.
+inline void checkIdentity(const Identity& identity, const char* file, int line)
+{
+    if (identity.name.empty())
+    {
+        throw IllegalIdentityException(file, line);
+    }
+}
+
+}
diff --git a/cpp/src/Ice/ConnectionI.cpp b/cpp/src/Ice/ConnectionI.cpp
@@ -22,6 +22,7 @@
 #include <Ice/ReferenceFactory.h> // For createProxy().
 #include <Ice/ProxyFactory.h> // For createProxy().
 #include <Ice/BatchRequestQueue.h>
+#include "CheckIdentity.h"
 
 #ifdef ICE_HAS_BZIP2
 #  include <bzlib.h>
@@ -1282,18 +1283,9 @@ Ice::ConnectionI::getEndpoint() const noexcept
 optional<ObjectPrx>
 Ice::ConnectionI::createProxy(const Identity& ident) const
 {
-    // Create a reference and return a reverse proxy for this reference.
-    ReferencePtr ref =
-        _instance->referenceFactory()->create(ident, const_cast<ConnectionI*>(this)->shared_from_this());
-
-    if (ref)
-    {
-        return ObjectPrx::_fromReference(std::move(ref));
-    }
-    else
-    {
-        return std::nullopt;
-    }
+    checkIdentity(ident, __FILE__, __LINE__);
+    return ObjectPrx::_fromReference(
+        _instance->referenceFactory()->create(ident, const_cast<ConnectionI*>(this)->shared_from_this()));
 }
 
 void

diff --git a/cpp/src/Ice/Exception.cpp b/cpp/src/Ice/Exception.cpp
@@ -306,7 +306,7 @@ void
 Ice::IllegalIdentityException::ice_print(ostream& out) const
 {
     Exception::ice_print(out);
-    out << ":\nillegal identity: `" << identityToString(id, ToStringMode::Unicode) << "'";
+    out << ":\nan identity with an empty name is not allowed";
 }
 
 void

diff --git a/cpp/src/Ice/Initialize.cpp b/cpp/src/Ice/Initialize.cpp
@@ -12,6 +12,7 @@
 #include <Ice/PluginManagerI.h>
 #include <Ice/StringUtil.h>
 #include <Ice/StringConverter.h>
+#include "CheckIdentity.h"
 
 #include <mutex>
 
@@ -515,12 +516,14 @@ Ice::stringToIdentity(const string& s)
         }
     }
 
+    checkIdentity(ident, __FILE__, __LINE__);
     return ident;
 }
 
 string
 Ice::identityToString(const Identity& ident, ToStringMode toStringMode)
 {
+    checkIdentity(ident, __FILE__, __LINE__);
     if(ident.category.empty())
     {
         return escapeString(ident.name, "/", toStringMode);

diff --git a/cpp/src/Ice/InputStream.cpp b/cpp/src/Ice/InputStream.cpp
@@ -1176,7 +1176,14 @@ Ice::InputStream::readReference()
 
     Identity ident;
     read(ident);
-    return _instance->referenceFactory()->create(ident, this);
+    if (ident.name.empty())
+    {
+        return nullptr;
+    }
+    else
+    {
+        return _instance->referenceFactory()->create(ident, this);
+    }
 }
 
 Int

diff --git a/cpp/src/Ice/Instance.cpp b/cpp/src/Ice/Instance.cpp
@@ -44,6 +44,7 @@
 #include <Ice/UUID.h>
 
 #include "Ice/ProxyFunctions.h"
+#include "CheckIdentity.h"
 
 #include <stdio.h>
 #include <list>
@@ -558,10 +559,7 @@ IceInternal::Instance::createAdmin(const ObjectAdapterPtr& adminAdapter, const I
         throw CommunicatorDestroyedException(__FILE__, __LINE__);
     }
 
-    if(adminIdentity.name.empty())
-    {
-        throw Ice::IllegalIdentityException(__FILE__, __LINE__, adminIdentity);
-    }
+    checkIdentity(adminIdentity, __FILE__, __LINE__);
 
     if(_adminAdapter)
     {

diff --git a/cpp/src/Ice/ObjectAdapterI.cpp b/cpp/src/Ice/ObjectAdapterI.cpp
@@ -28,6 +28,7 @@
 #include <Ice/PropertyNames.h>
 #include <Ice/ConsoleUtil.h>
 #include "Ice/ProxyFunctions.h"
+#include "CheckIdentity.h"
 
 #ifdef _WIN32
 #   include <sys/timeb.h>
@@ -43,13 +44,6 @@ using namespace IceInternal;
 
 namespace
 {
-inline void checkIdentity(const Identity& ident)
-{
-    if(ident.name.empty())
-    {
-        throw IllegalIdentityException(__FILE__, __LINE__, ident);
-    }
-}
 
 inline void checkServant(const shared_ptr<Object>& servant)
 {
@@ -381,7 +375,7 @@ Ice::ObjectAdapterI::addFacet(const shared_ptr<Object>& object, const Identity&
 
     checkForDeactivation();
     checkServant(object);
-    checkIdentity(ident);
+    checkIdentity(ident, __FILE__, __LINE__);
 
     _servantManager->addServant(object, ident, facet);
 
@@ -425,7 +419,7 @@ Ice::ObjectAdapterI::removeFacet(const Identity& ident, const string& facet)
     lock_guard lock(_mutex);
 
     checkForDeactivation();
-    checkIdentity(ident);
+    checkIdentity(ident, __FILE__, __LINE__);
 
     return _servantManager->removeServant(ident, facet);
 }
@@ -436,7 +430,7 @@ Ice::ObjectAdapterI::removeAllFacets(const Identity& ident)
     lock_guard lock(_mutex);
 
     checkForDeactivation();
-    checkIdentity(ident);
+    checkIdentity(ident, __FILE__, __LINE__);
 
     return _servantManager->removeAllFacets(ident);
 }
@@ -463,7 +457,7 @@ Ice::ObjectAdapterI::findFacet(const Identity& ident, const string& facet) const
     lock_guard lock(_mutex);
 
     checkForDeactivation();
-    checkIdentity(ident);
+    checkIdentity(ident, __FILE__, __LINE__);
 
     return _servantManager->findServant(ident, facet);
 }
@@ -474,7 +468,7 @@ Ice::ObjectAdapterI::findAllFacets(const Identity& ident) const
     lock_guard lock(_mutex);
 
     checkForDeactivation();
-    checkIdentity(ident);
+    checkIdentity(ident, __FILE__, __LINE__);
 
     return _servantManager->findAllFacets(ident);
 }
@@ -536,7 +530,7 @@ Ice::ObjectAdapterI::createProxy(const Identity& ident) const
     lock_guard lock(_mutex);
 
     checkForDeactivation();
-    checkIdentity(ident);
+    checkIdentity(ident, __FILE__, __LINE__);
 
     return newProxy(ident, "");
 }
@@ -547,7 +541,7 @@ Ice::ObjectAdapterI::createDirectProxy(const Identity& ident) const
     lock_guard lock(_mutex);
 
     checkForDeactivation();
-    checkIdentity(ident);
+    checkIdentity(ident, __FILE__, __LINE__);
 
     return newDirectProxy(ident, "");
 }
@@ -558,7 +552,7 @@ Ice::ObjectAdapterI::createIndirectProxy(const Identity& ident) const
     lock_guard lock(_mutex);
 
     checkForDeactivation();
-    checkIdentity(ident);
+    checkIdentity(ident, __FILE__, __LINE__);
 
     return newIndirectProxy(ident, "", _id);
 }

diff --git a/cpp/src/Ice/Proxy.cpp b/cpp/src/Ice/Proxy.cpp
@@ -13,6 +13,7 @@
 #include "ReferenceFactory.h"
 #include "RequestHandlerCache.h"
 #include "ConnectionI.h"
+#include "CheckIdentity.h"
 
 using namespace std;
 using namespace Ice;
@@ -80,10 +81,7 @@ Ice::ObjectPrx::ice_getIdentity() const
 ObjectPrx
 Ice::ObjectPrx::ice_identity(const Identity& newIdentity) const
 {
-    if (newIdentity.name.empty())
-    {
-        throw IllegalIdentityException(__FILE__, __LINE__);
-    }
+    checkIdentity(newIdentity, __FILE__, __LINE__);
     if (newIdentity == _reference->getIdentity())
     {
         return *this;

diff --git a/cpp/src/Ice/ReferenceFactory.cpp b/cpp/src/Ice/ReferenceFactory.cpp
@@ -87,7 +87,7 @@ IceInternal::ReferenceFactory::create(const string& str, const string& propertyP
 {
     if(str.empty())
     {
-        return 0;
+        return nullptr;
     }
 
     const string delim = " \t\r\n";
@@ -133,37 +133,29 @@ IceInternal::ReferenceFactory::create(const string& str, const string& propertyP
         throw ProxyParseException(__FILE__, __LINE__, "no identity in `" + s + "'");
     }
 
-    //
-    // Parsing the identity may raise IdentityParseException.
-    //
-    Identity ident = Ice::stringToIdentity(idstr);
-
-    if(ident.name.empty())
+    if (idstr.empty())
     {
-        //
-        // An identity with an empty name and a non-empty
-        // category is illegal.
-        //
-        if(!ident.category.empty())
-        {
-            throw IllegalIdentityException(__FILE__, __LINE__, ident);
-        }
         //
         // Treat a stringified proxy containing two double
         // quotes ("") the same as an empty string, i.e.,
         // a null proxy, but only if nothing follows the
         // quotes.
         //
-        else if(s.find_first_not_of(delim, end) != string::npos)
+        if(s.find_first_not_of(delim, end) != string::npos)
         {
             throw ProxyParseException(__FILE__, __LINE__, "invalid characters after identity in `" + s + "'");
         }
         else
         {
-            return 0;
+            return nullptr;
         }
     }
 
+    //
+    // Parsing the identity may raise IdentityParseException.
+    //
+    Identity ident = Ice::stringToIdentity(idstr);
+
     string facet;
     Reference::Mode mode = Reference::ModeTwoway;
     bool secure = false;
@@ -533,11 +525,7 @@ IceInternal::ReferenceFactory::create(const Identity& ident, InputStream* s)
     // Don't read the identity here. Operations calling this
     // constructor read the identity, and pass it as a parameter.
     //
-
-    if(ident.name.empty() && ident.category.empty())
-    {
-        return 0;
-    }
+    assert(!ident.name.empty());
 
     //
     // For compatibility with the old FacetPath.

diff --git a/cpp/test/Ice/exceptions/AllTests.cpp b/cpp/test/Ice/exceptions/AllTests.cpp
@@ -210,7 +210,6 @@ allTests(Test::TestHelper* helper)
             }
             catch(const Ice::IllegalIdentityException& ex)
             {
-                test(ex.id.name == "");
                 if(printException)
                 {
                     Ice::Print printer(communicator->getLogger());

diff --git a/cpp/test/Ice/proxy/AllTests.cpp b/cpp/test/Ice/proxy/AllTests.cpp
@@ -318,6 +318,16 @@ allTests(Test::TestHelper* helper)
     id2 = Ice::stringToIdentity(idStr);
     test(id == id2);
 
+    try
+    {
+        // Ice APIs can't return all illegal identity.
+        id = Ice::stringToIdentity("");
+        assert(false);
+    }
+    catch (const Ice::IllegalIdentityException&)
+    {
+    }
+
     // Input string with various pitfalls
     id = Ice::stringToIdentity("\\342\\x82\\254\\60\\x9\\60\\");
     test(id.name == "\xE2\x82\xAC\60\t0\\" && id.category.empty());

diff --git a/js/src/Ice/LocalException.d.ts b/js/src/Ice/LocalException.d.ts
@@ -297,17 +297,10 @@ export namespace Ice
     }
 
     /**
-     * This exception is raised if an illegal identity is encountered.
+     * This exception is raised if an identity with an empty name is encountered.
      */
     class IllegalIdentityException extends LocalException
     {
-        /**
-         * One-shot constructor to initialize all data members.
-         * @param id The illegal identity.
-         * @param ice_cause The error that cause this exception.
-         */
-        constructor(id?:Identity, ice_cause?:string|Error);
-        id:Identity;
     }
 
     /**

diff --git a/js/src/Ice/LocalException.js b/js/src/Ice/LocalException.js
@@ -499,10 +499,9 @@ Ice.ProxyParseException = class extends Ice.LocalException
  **/
 Ice.IllegalIdentityException = class extends Ice.LocalException
 {
-    constructor(id = new Ice.Identity(), _cause = "")
+    constructor(_cause = "")
     {
         super(_cause);
-        this.id = id;
     }
 
     static get _parent()