[omniORB] 4.0.X and 4.1.X Alignment bug when parsing GIOP 1.2 Reply when using service contexts

[Jonathan Biggar]

GIOP 1.2 requires the REPLY body to be aligned on an 8 byte boundary.
omniORB has the code to do this alignment for normal replies and system
exceptions, but is missing the alignment call for user exceptions and
location forward replies.

This bug only manifests itself if service contexts are used to transmit
additional data from the CORBA server back to the client, and then only
if the service context length doesn't leave the alignment on an 8 byte
boundary already.

This bug appears in omniORB 4.0.X and 4.1.[01].

Here is a patch relative to omniORB 4.0.5 for the functions
giopImpl12::sendUserException and giopImpl12::sendLocationForwardReply:

+++ giopImpl12.cc 2007-12-10 16:36:37.000000000 -0800
@@ -1704,6 +1704,8 @@
// Service context
giop_s.service_contexts() >>= s;

+ s.alignOutput(omni::ALIGN_8);
+
// RepoId
CORBA::ULong(repoid_size) >>= s;
s.put_octet_array((const CORBA::Octet*) repoid, repoid_size);
@@ -1751,6 +1753,7 @@
// Service context
operator>>= ((CORBA::ULong)0,s);

+ s.alignOutput(omni::ALIGN_8);

// object reference
CORBA::Object::_marshalObjRef(obj,s);

[Jonathan Biggar]

Post by Jonathan Biggar
GIOP 1.2 requires the REPLY body to be aligned on an 8 byte boundary.
omniORB has the code to do this alignment for normal replies and system
exceptions, but is missing the alignment call for user exceptions and
location forward replies.
This bug only manifests itself if service contexts are used to transmit
additional data from the CORBA server back to the client, and then only
if the service context length doesn't leave the alignment on an 8 byte
boundary already.
This bug appears in omniORB 4.0.X and 4.1.[01].
Here is a patch relative to omniORB 4.0.5 for the functions

This patch was incomplete. There's a couple of other places that
alignOuput() needed to be called. Here's the complete patch:

--- giopImpl12.cc.orig 2007-12-10 16:29:23.000000000 -0800
+++ giopImpl12.cc 2007-12-11 08:50:54.000000000 -0800
@@ -1686,6 +1686,8 @@
operator>>= ((CORBA::ULong)0,cs);
operator>>= ((CORBA::ULong)0,cs);
giop_s.service_contexts() >>= cs;
+ cs.alignOutput(omni::ALIGN_8);
+
CORBA::ULong(repoid_size) >>= cs;
cs.put_octet_array((const CORBA::Octet*) repoid, repoid_size);
ex._NP_marshal(cs);
@@ -1704,6 +1706,8 @@
// Service context
giop_s.service_contexts() >>= s;

+ s.alignOutput(omni::ALIGN_8);
+
// RepoId
CORBA::ULong(repoid_size) >>= s;
s.put_octet_array((const CORBA::Octet*) repoid, repoid_size);
@@ -1735,6 +1739,8 @@
operator>>= ((CORBA::ULong)0,cs);
operator>>= ((CORBA::ULong)0,cs);
+ cs.alignOutput(omni::ALIGN_8);
+
CORBA::Object::_marshalObjRef(obj,cs);
outputSetFragmentSize(g,cs.total()-12);
*((CORBA::ULong*)(hdr + 8)) = cs.total() - 12;
@@ -1751,6 +1757,7 @@
// Service context
operator>>= ((CORBA::ULong)0,s);

+ s.alignOutput(omni::ALIGN_8);

// object reference
CORBA::Object::_marshalObjRef(obj,s);

[Duncan Grisby]

Post by Jonathan Biggar
This patch was incomplete. There's a couple of other places that

Thanks for the patch. I've checked the fix in to the omni4_1_develop
branch. I actually only checked in the fix for user exceptions, since
the service contexts in LocationForward replies are always set empty,
meaning the alignment is already always correct.

Cheers,

Duncan.