libopenmetaverse
  1. libopenmetaverse
  2. LIBOMV-567

Packets with Variable blocks that are sent null are not being handled by the packet splitter

    Details

    • Type: Bug Bug
    • Status: Resolved Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.7.0
    • Fix Version/s: 0.7.0
    • Component/s: Networking
    • Labels:
      None
    • Severity:
      High
    • Environment:
      All
    • Steps to Reproduce:
      Hide

      Run TestClient thru WinGridProxy

      touch prim-UUID

      Notice no ObjectGrabPacket is sent

      Show
      Run TestClient thru WinGridProxy touch prim-UUID Notice no ObjectGrabPacket is sent

      Description

      See LIBOMV-566 for originally reported issue:

      What was happening is our Grab Packet was not populating the SurfaceInfo variable block of the packet and the packet was therefore being dropped.

      Repro: remove SurfaceInfo block from degrab packet (or grab packet), use touch command in TestClient, no packet is sent.

      public void DeGrab(uint objectLocalID, Vector3 uvCoord, Vector3 stCoord, int faceIndex, Vector3 position,
      Vector3 normal, Vector3 binormal)

      { ObjectDeGrabPacket degrab = new ObjectDeGrabPacket(); degrab.AgentData.AgentID = Client.Self.AgentID; degrab.AgentData.SessionID = Client.Self.SessionID; degrab.ObjectData.LocalID = objectLocalID; degrab.SurfaceInfo = new ObjectDeGrabPacket.SurfaceInfoBlock[1]; degrab.SurfaceInfo[0] = new ObjectDeGrabPacket.SurfaceInfoBlock(); degrab.SurfaceInfo[0].UVCoord = uvCoord; degrab.SurfaceInfo[0].STCoord = stCoord; degrab.SurfaceInfo[0].FaceIndex = faceIndex; degrab.SurfaceInfo[0].Position = position; degrab.SurfaceInfo[0].Normal = normal; degrab.SurfaceInfo[0].Binormal = binormal; Client.Network.SendPacket(degrab); }

      Expected Result: Packet gets packed properly and sent without the block

        Issue Links

          Activity

          Hide
          Jim Radford added a comment -

          Original issue reported by Doug Miles in LIBOMV-566,

          Show
          Jim Radford added a comment - Original issue reported by Doug Miles in LIBOMV-566 ,
          Jim Radford made changes -
          Field Original Value New Value
          Assignee Jim Radford [ jradford ] John Hurliman [ jhurliman ]
          Priority Blocker [ 1 ] Major [ 3 ]
          Description ObjectGrabPacket ToBytesMultiple() not working
          For now a workarround is

           HasVariableBlocks = false;
          in the contructor.

          Possibly other packets are broken in simular way
          See LIBOMV-566 for originally reported issue:

          What was happening is our Grab Packet was not populating the SurfaceInfo variable block of the packet and the packet was therefore being dropped.

          Repro: remove SurfaceInfo block from degrab packet (or grab packet), use touch command in TestClient, no packet is sent.

          public void DeGrab(uint objectLocalID, Vector3 uvCoord, Vector3 stCoord, int faceIndex, Vector3 position,
                      Vector3 normal, Vector3 binormal)
                  {
                      ObjectDeGrabPacket degrab = new ObjectDeGrabPacket();
                      degrab.AgentData.AgentID = Client.Self.AgentID;
                      degrab.AgentData.SessionID = Client.Self.SessionID;

                      degrab.ObjectData.LocalID = objectLocalID;
                      
                      degrab.SurfaceInfo = new ObjectDeGrabPacket.SurfaceInfoBlock[1];
                      degrab.SurfaceInfo[0] = new ObjectDeGrabPacket.SurfaceInfoBlock();
                      degrab.SurfaceInfo[0].UVCoord = uvCoord;
                      degrab.SurfaceInfo[0].STCoord = stCoord;
                      degrab.SurfaceInfo[0].FaceIndex = faceIndex;
                      degrab.SurfaceInfo[0].Position = position;
                      degrab.SurfaceInfo[0].Normal = normal;
                      degrab.SurfaceInfo[0].Binormal = binormal;

                      Client.Network.SendPacket(degrab);
                  }

          Expected Result: Packet gets packed properly and sent without the block
          Severity Showstopper High
          Jim Radford made changes -
          Link This issue causes LIBOMV-566 [ LIBOMV-566 ]
          Hide
          John Hurliman added a comment -

          Packets with anything null in them should not be working with any of the packet serialization methods. All of the null checks were left out of mapgenerator because it is faster to set things to empty arrays instead of doing several null checks when packets are queued and sent out. The actual bug here is that part of libomv is not properly constructing one or more packets.

          Show
          John Hurliman added a comment - Packets with anything null in them should not be working with any of the packet serialization methods. All of the null checks were left out of mapgenerator because it is faster to set things to empty arrays instead of doing several null checks when packets are queued and sent out. The actual bug here is that part of libomv is not properly constructing one or more packets.
          Hide
          Jim Radford added a comment -

          The behavior we had before was to allow the packet to be sent even with a null block,
          the behaviour now is to silently drop the packet without warning.
          I think the correct behavior is to throw an exception which makes it the responsibility of the person updating the Packets.cs via the MapGenerator tool responsible for implementing the changes in the template. I've seen on several occasions the new template being put into place and a new Packets.cs generated, but the additional blocks not implemented in our various methods that generate the packet. An Exception being thrown would at least give is a warning that there are methods that generate packets while its still in trunk and before a release is pushed out.

          Thoughts?

          Show
          Jim Radford added a comment - The behavior we had before was to allow the packet to be sent even with a null block, the behaviour now is to silently drop the packet without warning. I think the correct behavior is to throw an exception which makes it the responsibility of the person updating the Packets .cs via the MapGenerator tool responsible for implementing the changes in the template. I've seen on several occasions the new template being put into place and a new Packets .cs generated, but the additional blocks not implemented in our various methods that generate the packet. An Exception being thrown would at least give is a warning that there are methods that generate packets while its still in trunk and before a release is pushed out. Thoughts?
          Hide
          John Hurliman added a comment -

          Yes. I'm curious why no exception is thrown right now. Is it handling null blocks but generating invalid packets, or is the exception not being logged for some reason?

          Show
          John Hurliman added a comment - Yes. I'm curious why no exception is thrown right now. Is it handling null blocks but generating invalid packets, or is the exception not being logged for some reason?
          Hide
          Jim Radford added a comment -

          as far as I can see it just drops the packet, it never hits the server (or at least never hits the proxy)

          Show
          Jim Radford added a comment - as far as I can see it just drops the packet, it never hits the server (or at least never hits the proxy)
          John Hurliman made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Hide
          John Hurliman added a comment -

          Resolved in r2886, thanks!

          Show
          John Hurliman added a comment - Resolved in r2886, thanks!
          John Hurliman made changes -
          Status In Progress [ 3 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          View full commit
          Resolving [LIBOMV-567]. libomv now throws a helpful error and a stack trace when a packet cannot be serialized because of null blocks git-svn-id: http://libopenmetaverse.googlecode.com/svn/libopenmetaverse/trunk@2886 52acb1d6-8a22-11de-b505-999d5b087335

            People

            • Assignee:
              John Hurliman
              Reporter:
              Douglas R Miles
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: