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

causes

Bug - A problem which impairs or prevents the functions of the product. LIBOMV-566 ObjectGrabPacket ToBytesMultiple() not working

  • Blocker - Blocks development and/or testing work, production could not run.
  • Resolved - A resolution has been taken, and it is awaiting verification by reporter. From here issues are either reopened, or are closed.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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 ]
John Hurliman
15/Jun/09 3:42 AM
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
master
+8
-1
OpenMetaverse/Simulator.cs
+1153
-823
OpenMetaverse/_Packets_.cs
+13
-9
Programs/mapgenerator/mapgenerator.cs
jhurliman
15/Jun/09 3:42 AM
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: file:///data/svn/omf/libopenmetaverse/trunk@2886 454cea33-f7e8-409b-8d0d-70907e6fec76
trunk
+8
-1
OpenMetaverse/Simulator.cs
+1153
-823
OpenMetaverse/_Packets_.cs
+13
-9
Programs/mapgenerator/mapgenerator.cs

People

  • Assignee:
    John Hurliman
    Reporter:
    Douglas R Miles
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: