diff options
19 files changed, 146 insertions, 31 deletions
diff --git a/cpp/test/Ice/servantLocator/ServantLocatorI.cpp b/cpp/test/Ice/servantLocator/ServantLocatorI.cpp index 2282aaca474..752b55ad044 100644 --- a/cpp/test/Ice/servantLocator/ServantLocatorI.cpp +++ b/cpp/test/Ice/servantLocator/ServantLocatorI.cpp @@ -43,6 +43,11 @@ ServantLocatorI::locate(const Ice::Current& current, Ice::LocalObjectPtr& cookie return 0; } + if(current.id.name == "invalidReturnValue" || current.id.name == "invalidReturnType") + { + return 0; + } + test(current.id.name == "locate" || current.id.name == "finished"); if(current.id.name == "locate") { diff --git a/csharp/test/Ice/servantLocator/ServantLocatorAMDI.cs b/csharp/test/Ice/servantLocator/ServantLocatorAMDI.cs index d67230a694d..e56c84f7525 100644 --- a/csharp/test/Ice/servantLocator/ServantLocatorAMDI.cs +++ b/csharp/test/Ice/servantLocator/ServantLocatorAMDI.cs @@ -50,6 +50,12 @@ namespace Ice return null; } + if(current.id.name.Equals("invalidReturnValue") || current.id.name.Equals("invalidReturnType")) + { + cookie = null; + return null; + } + test(current.id.name.Equals("locate") || current.id.name.Equals("finished")); if(current.id.name.Equals("locate")) { diff --git a/csharp/test/Ice/servantLocator/ServantLocatorI.cs b/csharp/test/Ice/servantLocator/ServantLocatorI.cs index 4cdc5b96b2b..6e0708546ff 100644 --- a/csharp/test/Ice/servantLocator/ServantLocatorI.cs +++ b/csharp/test/Ice/servantLocator/ServantLocatorI.cs @@ -48,6 +48,12 @@ namespace Ice return null; } + if(current.id.name.Equals("invalidReturnValue") || current.id.name.Equals("invalidReturnType")) + { + cookie = null; + return null; + } + test(current.id.name.Equals("locate") || current.id.name.Equals("finished")); if(current.id.name.Equals("locate")) { diff --git a/java-compat/test/src/main/java/test/Ice/servantLocator/AMDServantLocatorI.java b/java-compat/test/src/main/java/test/Ice/servantLocator/AMDServantLocatorI.java index accd91b9fa7..0f131ffffee 100644 --- a/java-compat/test/src/main/java/test/Ice/servantLocator/AMDServantLocatorI.java +++ b/java-compat/test/src/main/java/test/Ice/servantLocator/AMDServantLocatorI.java @@ -57,6 +57,11 @@ public final class AMDServantLocatorI implements Ice.ServantLocator return null; } + if(current.id.name.equals("invalidReturnValue") || current.id.name.equals("invalidReturnType")) + { + return null; + } + test(current.id.name.equals("locate") || current.id.name.equals("finished")); if(current.id.name.equals("locate")) { diff --git a/java-compat/test/src/main/java/test/Ice/servantLocator/ServantLocatorI.java b/java-compat/test/src/main/java/test/Ice/servantLocator/ServantLocatorI.java index 8dfb7665841..7ac1f491cdb 100644 --- a/java-compat/test/src/main/java/test/Ice/servantLocator/ServantLocatorI.java +++ b/java-compat/test/src/main/java/test/Ice/servantLocator/ServantLocatorI.java @@ -57,6 +57,11 @@ public final class ServantLocatorI implements Ice.ServantLocator return null; } + if(current.id.name.equals("invalidReturnValue") || current.id.name.equals("invalidReturnType")) + { + return null; + } + test(current.id.name.equals("locate") || current.id.name.equals("finished")); if(current.id.name.equals("locate")) { diff --git a/java/test/src/main/java/test/Ice/servantLocator/AMDServantLocatorI.java b/java/test/src/main/java/test/Ice/servantLocator/AMDServantLocatorI.java index 4509e1da798..15496d61798 100644 --- a/java/test/src/main/java/test/Ice/servantLocator/AMDServantLocatorI.java +++ b/java/test/src/main/java/test/Ice/servantLocator/AMDServantLocatorI.java @@ -57,6 +57,11 @@ public final class AMDServantLocatorI implements ServantLocator return new ServantLocator.LocateResult(); } + if(current.id.name.equals("invalidReturnValue") || current.id.name.equals("invalidReturnType")) + { + return new ServantLocator.LocateResult(); + } + test(current.id.name.equals("locate") || current.id.name.equals("finished")); if(current.id.name.equals("locate")) { diff --git a/java/test/src/main/java/test/Ice/servantLocator/ServantLocatorI.java b/java/test/src/main/java/test/Ice/servantLocator/ServantLocatorI.java index 8217d99e124..ac488ff2681 100644 --- a/java/test/src/main/java/test/Ice/servantLocator/ServantLocatorI.java +++ b/java/test/src/main/java/test/Ice/servantLocator/ServantLocatorI.java @@ -57,6 +57,11 @@ public final class ServantLocatorI implements ServantLocator return new ServantLocator.LocateResult(); } + if(current.id.name.equals("invalidReturnValue") || current.id.name.equals("invalidReturnType")) + { + return new ServantLocator.LocateResult(); + } + test(current.id.name.equals("locate") || current.id.name.equals("finished")); if(current.id.name.equals("locate")) { diff --git a/js/test/Ice/servantLocator/ServantLocatorI.js b/js/test/Ice/servantLocator/ServantLocatorI.js index c2267194751..55ea4529846 100644 --- a/js/test/Ice/servantLocator/ServantLocatorI.js +++ b/js/test/Ice/servantLocator/ServantLocatorI.js @@ -41,6 +41,11 @@ return null; } + if(current.id.name == "invalidReturnValue" || current.id.name == "invalidReturnType") + { + return null; + } + test(current.id.name == "locate" || current.id.name == "finished"); if(current.id.name == "locate") { diff --git a/objective-c/test/Ice/servantLocator/ServantLocatorI.m b/objective-c/test/Ice/servantLocator/ServantLocatorI.m index fd94a5c6411..ca028a5a18e 100644 --- a/objective-c/test/Ice/servantLocator/ServantLocatorI.m +++ b/objective-c/test/Ice/servantLocator/ServantLocatorI.m @@ -36,6 +36,11 @@ return 0; } + if([current.id_.name isEqual:@"invalidReturnValue"] || [current.id_.name isEqual:@"invalidReturnType"]) + { + return 0; + } + test([current.id_.name isEqual:@"locate"] || [current.id_.name isEqual:@"finished"]); if([current.id_.name isEqual:@"locate"]) { diff --git a/python/modules/IcePy/ObjectAdapter.cpp b/python/modules/IcePy/ObjectAdapter.cpp index 03e6167182a..166ba647a53 100644 --- a/python/modules/IcePy/ObjectAdapter.cpp +++ b/python/modules/IcePy/ObjectAdapter.cpp @@ -17,6 +17,7 @@ #include <Ice/ObjectAdapter.h> #include <Ice/Router.h> #include <Ice/ServantLocator.h> +#include <Ice/Logger.h> #include <pythread.h> @@ -177,7 +178,11 @@ IcePy::ServantLocatorWrapper::locate(const Ice::Current& current, Ice::LocalObje { if(PyTuple_GET_SIZE(res.get()) > 2) { - PyErr_WarnEx(PyExc_RuntimeWarning, STRCAST("invalid return value for ServantLocator::locate"), 1); + const Ice::CommunicatorPtr com = current.adapter->getCommunicator(); + if(com->getProperties()->getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1) > 0) + { + com->getLogger()->warning("invalid return value for ServantLocator::locate"); + } return 0; } servantObj = PyTuple_GET_ITEM(res.get(), 0); @@ -196,7 +201,11 @@ IcePy::ServantLocatorWrapper::locate(const Ice::Current& current, Ice::LocalObje // if(!PyObject_IsInstance(servantObj, _objectType)) { - PyErr_WarnEx(PyExc_RuntimeWarning, STRCAST("return value of ServantLocator::locate is not an Ice object"), 1); + const Ice::CommunicatorPtr com = current.adapter->getCommunicator(); + if(com->getProperties()->getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1) > 0) + { + com->getLogger()->warning("return value of ServantLocator::locate is not an Ice object"); + } return 0; } diff --git a/python/modules/IcePy/Operation.cpp b/python/modules/IcePy/Operation.cpp index d343d244c2a..0570640aca4 100644 --- a/python/modules/IcePy/Operation.cpp +++ b/python/modules/IcePy/Operation.cpp @@ -1371,9 +1371,7 @@ Operation::marshalResult(Ice::OutputStream& os, PyObject* result) { ostringstream ostr; ostr << "operation `" << fixIdent(name) << "' should return a tuple of length " << numResults; - string str = ostr.str(); - PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(str.c_str()), 1); - throw Ice::MarshalException(__FILE__, __LINE__); + throw Ice::MarshalException(__FILE__, __LINE__, ostr.str()); } // @@ -1407,12 +1405,18 @@ Operation::marshalResult(Ice::OutputStream& os, PyObject* result) PyObject* arg = PyTuple_GET_ITEM(t.get(), info->pos); if((!info->optional || arg != Unset) && !info->type->validate(arg)) { - // TODO: Provide the parameter name instead? - ostringstream ostr; - ostr << "invalid value for out argument " << (info->pos + 1) << " in operation `" << dispatchName << "'"; - string str = ostr.str(); - PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(str.c_str()), 1); - throw Ice::MarshalException(__FILE__, __LINE__); + try + { + PyException().raise(); + } + catch(const Ice::UnknownException& ex) + { + // TODO: Provide the parameter name instead? + ostringstream ostr; + ostr << "invalid value for out argument " << (info->pos + 1) << " in operation `" << dispatchName; + ostr << "':\n" << ex.unknown; + throw Ice::MarshalException(__FILE__, __LINE__, ostr.str()); + } } } if(returnType) @@ -1420,11 +1424,16 @@ Operation::marshalResult(Ice::OutputStream& os, PyObject* result) PyObject* res = PyTuple_GET_ITEM(t.get(), 0); if((!returnType->optional || res != Unset) && !returnType->type->validate(res)) { - ostringstream ostr; - ostr << "invalid return value for operation `" << dispatchName << "'"; - string str = ostr.str(); - PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(str.c_str()), 1); - throw Ice::MarshalException(__FILE__, __LINE__); + try + { + PyException().raise(); + } + catch(const Ice::UnknownException& ex) + { + ostringstream ostr; + ostr << "invalid return value for operation `" << dispatchName << "':\n" << ex.unknown; + throw Ice::MarshalException(__FILE__, __LINE__, ostr.str()); + } } } @@ -3777,7 +3786,6 @@ Upcall::dispatchImpl(PyObject* servant, const string& dispatchName, PyObject* ar ostr << "servant for identity " << communicator->identityToString(current.id) << " does not define operation `" << dispatchName << "'"; string str = ostr.str(); - PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(str.c_str()), 1); Ice::UnknownException ex(__FILE__, __LINE__); ex.unknown = str; throw ex; @@ -3793,7 +3801,6 @@ Upcall::dispatchImpl(PyObject* servant, const string& dispatchName, PyObject* ar ostr << "_iceDispatch method not found for identity " << communicator->identityToString(current.id) << " and operation `" << dispatchName << "'"; string str = ostr.str(); - PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(str.c_str()), 1); Ice::UnknownException ex(__FILE__, __LINE__); ex.unknown = str; throw ex; @@ -4108,9 +4115,7 @@ IcePy::BlobjectUpcall::response(PyObject* result) // if(!PyTuple_Check(result) || PyTuple_GET_SIZE(result) != 2) { - string str = "operation `ice_invoke' should return a tuple of length 2"; - PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(str.c_str()), 1); - throw Ice::MarshalException(__FILE__, __LINE__); + throw Ice::MarshalException(__FILE__, __LINE__, "operation `ice_invoke' should return a tuple of length 2"); } PyObject* arg = PyTuple_GET_ITEM(result, 0); @@ -4121,11 +4126,7 @@ IcePy::BlobjectUpcall::response(PyObject* result) #if PY_VERSION_HEX >= 0x03000000 if(!PyBytes_Check(arg)) { - ostringstream ostr; - ostr << "invalid return value for operation `ice_invoke'"; - string str = ostr.str(); - PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(str.c_str()), 1); - throw Ice::MarshalException(__FILE__, __LINE__); + throw Ice::MarshalException(__FILE__, __LINE__, "invalid return value for operation `ice_invoke'"); } Py_ssize_t sz = PyBytes_GET_SIZE(arg); @@ -4139,11 +4140,7 @@ IcePy::BlobjectUpcall::response(PyObject* result) #else if(!PyBuffer_Check(arg)) { - ostringstream ostr; - ostr << "invalid return value for operation `ice_invoke'"; - string str = ostr.str(); - PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(str.c_str()), 1); - throw Ice::MarshalException(__FILE__, __LINE__); + throw Ice::MarshalException(__FILE__, __LINE__, "invalid return value for operation `ice_invoke'"); } char* charBuf = 0; diff --git a/python/test/Ice/exceptions/AllTests.py b/python/test/Ice/exceptions/AllTests.py index 3aa59ab16f4..762058d61ef 100644 --- a/python/test/Ice/exceptions/AllTests.py +++ b/python/test/Ice/exceptions/AllTests.py @@ -813,6 +813,25 @@ def allTests(helper, communicator): print("ok") + sys.stdout.write("catching unknown non-Ice exception with futures... ") + sys.stdout.flush() + try: + try: + thrower.throwMarshalException(context={"response":""}) + except Ice.UnknownLocalException as ex: + test("::Ice::MarshalException" in str(ex)) + try: + thrower.throwMarshalException(context={"param":""}) + except Ice.UnknownLocalException as ex: + test("::Ice::MarshalException" in str(ex)) + try: + thrower.throwMarshalException() + except Ice.UnknownLocalException as ex: + test("::Ice::MarshalException" in str(ex)) + except Ice.OperationNotExistException: + pass + print("ok") + sys.stdout.write("catching exact types with AMI mapping... ") sys.stdout.flush() diff --git a/python/test/Ice/exceptions/ServerAMD.py b/python/test/Ice/exceptions/ServerAMD.py index 8bd9a6f35a8..b9418e7e29d 100755 --- a/python/test/Ice/exceptions/ServerAMD.py +++ b/python/test/Ice/exceptions/ServerAMD.py @@ -136,6 +136,12 @@ class ThrowerI(Test.Thrower): f.set_exception(Test.A()) return f + def throwMarshalException(self, current): + if "return" in current.ctx: + return Ice.Future.completed(("", 0)) + if "param" in current.ctx: + return Ice.Future.completed((0, "")) + return Ice.Future.completed(None) class ServerAMD(TestHelper): diff --git a/python/test/Ice/exceptions/Test.ice b/python/test/Ice/exceptions/Test.ice index adbcdcf3c9c..c9513cfa3ec 100644 --- a/python/test/Ice/exceptions/Test.ice +++ b/python/test/Ice/exceptions/Test.ice @@ -71,6 +71,8 @@ interface Thrower void throwAfterResponse(); void throwAfterException() throws A; + + int throwMarshalException(out int p); } interface WrongOperation diff --git a/python/test/Ice/exceptions/TestI.py b/python/test/Ice/exceptions/TestI.py index d4ea54c5df1..65860b6cad6 100644 --- a/python/test/Ice/exceptions/TestI.py +++ b/python/test/Ice/exceptions/TestI.py @@ -101,3 +101,9 @@ class ThrowerI(Test.Thrower): # Only relevant for AMD. # raise Test.A() + + def throwMarshalException(self, current): + if "return" in current.ctx: + return ("", 0) + if "param" in current.ctx: + return (0, "") diff --git a/python/test/Ice/servantLocator/AllTests.py b/python/test/Ice/servantLocator/AllTests.py index 2aabd9103de..19bcf51e56f 100644 --- a/python/test/Ice/servantLocator/AllTests.py +++ b/python/test/Ice/servantLocator/AllTests.py @@ -217,4 +217,16 @@ def allTests(helper, communicator): test(False) print("ok") + sys.stdout.write("testing invalid locate return values ... ") + sys.stdout.flush() + try: + communicator.stringToProxy("invalidReturnValue:{0}".format(helper.getTestEndpoint())).ice_ping() + except Ice.ObjectNotExistException: + pass + try: + communicator.stringToProxy("invalidReturnType:{0}".format(helper.getTestEndpoint())).ice_ping() + except Ice.ObjectNotExistException: + pass + print ("ok") + return obj diff --git a/python/test/Ice/servantLocator/TestAMDI.py b/python/test/Ice/servantLocator/TestAMDI.py index 0fc254af645..dae02342251 100644 --- a/python/test/Ice/servantLocator/TestAMDI.py +++ b/python/test/Ice/servantLocator/TestAMDI.py @@ -101,6 +101,12 @@ class ServantLocatorI(Ice.ServantLocator): if current.id.name == "unknown": return None + if current.id.name == "invalidReturnValue": + return (45, 12) + + if current.id.name == "invalidReturnType": + return "invalid" + test(current.id.name == "locate" or current.id.name == "finished") if current.id.name == "locate": self.exception(current) diff --git a/python/test/Ice/servantLocator/TestI.py b/python/test/Ice/servantLocator/TestI.py index 9e2345b0fe9..0ffe602ea49 100644 --- a/python/test/Ice/servantLocator/TestI.py +++ b/python/test/Ice/servantLocator/TestI.py @@ -89,6 +89,12 @@ class ServantLocatorI(Ice.ServantLocator): if current.id.name == "unknown": return None + if current.id.name == "invalidReturnValue": + return (45, 12) + + if current.id.name == "invalidReturnType": + return "invalid" + test(current.id.name == "locate" or current.id.name == "finished") if current.id.name == "locate": self.exception(current) diff --git a/swift/test/Ice/servantLocator/ServantLocatorI.swift b/swift/test/Ice/servantLocator/ServantLocatorI.swift index a4ed8c5ee9f..99e7220620c 100644 --- a/swift/test/Ice/servantLocator/ServantLocatorI.swift +++ b/swift/test/Ice/servantLocator/ServantLocatorI.swift @@ -43,6 +43,11 @@ class ServantLocatorI: Ice.ServantLocator { return (nil, nil) } + if(curr.id.name == "invalidReturnValue" || curr.id.name == "invalidReturnType") { + return (nil, nil) + } + + try _helper.test(curr.id.name == "locate" || curr.id.name == "finished") if curr.id.name == "locate" { |