From 49ec85ae154a8062e6fcf8bad95ecc3b611e7812 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Mon, 14 Nov 2011 15:24:02 +0000 Subject: [PATCH 1/5] Do a ScenePresence null check in HGMessageTransferModule.SendIMToScene() to stop a NullReferenceException being thrown if an HG IM is sent to a simulator running multiple regions This is an attempt to address http://opensimulator.org/mantis/view.php?id=5791 --- .../Avatar/InstantMessage/HGMessageTransferModule.cs | 3 ++- OpenSim/Region/Framework/Scenes/ScenePresence.cs | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/OpenSim/Region/CoreModules/Avatar/InstantMessage/HGMessageTransferModule.cs b/OpenSim/Region/CoreModules/Avatar/InstantMessage/HGMessageTransferModule.cs index 321a70522b..560d9136ca 100644 --- a/OpenSim/Region/CoreModules/Avatar/InstantMessage/HGMessageTransferModule.cs +++ b/OpenSim/Region/CoreModules/Avatar/InstantMessage/HGMessageTransferModule.cs @@ -225,12 +225,13 @@ namespace OpenSim.Region.CoreModules.Avatar.InstantMessage foreach (Scene scene in m_Scenes) { ScenePresence sp = scene.GetScenePresence(toAgentID); - if(!sp.IsChildAgent) + if(sp != null && !sp.IsChildAgent) { scene.EventManager.TriggerIncomingInstantMessage(gim); successful = true; } } + if (!successful) { // If the message can't be delivered to an agent, it diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs index 3fee642afe..9ebb6dd608 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs @@ -2854,6 +2854,8 @@ namespace OpenSim.Region.Framework.Scenes Velocity = Vector3.Zero; AbsolutePosition = pos; +// m_log.DebugFormat("[SCENE PRESENCE]: Prevented flyoff for {0} at {1}", Name, AbsolutePosition); + AddToPhysicalScene(isFlying); } } From ff36a1bc7bcd1d0fe5aecb4c5358dbb072c7ff6e Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Mon, 14 Nov 2011 16:06:06 +0000 Subject: [PATCH 2/5] If a friends identifier which is too short is given to HGFriendsModule.GetOnlineFriends() then spit out a warning rather than failing on the String.Substring(). This is to progress http://opensimulator.org/mantis/view.php?id=5789 --- OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs | 2 ++ .../Region/CoreModules/Avatar/Friends/HGFriendsModule.cs | 9 ++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs index 626ebb5939..133da0f425 100644 --- a/OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs +++ b/OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs @@ -11088,6 +11088,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP Utils.BytesToString(avatarInterestUpdate.PropertiesData.LanguagesText)); return true; } + private bool HandleGrantUserRights(IClientAPI sender, Packet Pack) { GrantUserRightsPacket GrantUserRights = @@ -11108,6 +11109,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP GrantUserRights.Rights[0].RelatedRights); return true; } + private bool HandlePlacesQuery(IClientAPI sender, Packet Pack) { PlacesQueryPacket placesQueryPacket = diff --git a/OpenSim/Region/CoreModules/Avatar/Friends/HGFriendsModule.cs b/OpenSim/Region/CoreModules/Avatar/Friends/HGFriendsModule.cs index dda67f9c2b..02b417f946 100644 --- a/OpenSim/Region/CoreModules/Avatar/Friends/HGFriendsModule.cs +++ b/OpenSim/Region/CoreModules/Avatar/Friends/HGFriendsModule.cs @@ -141,7 +141,14 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends { List fList = new List(); foreach (string s in friendList) - fList.Add(s.Substring(0, 36)); + { + if (s.Length < 36) + m_log.WarnFormat( + "[HGFRIENDS MODULE]: Ignoring friend {0} ({1} chars) for {2} since identifier too short", + s, s.Length, userID); + else + fList.Add(s.Substring(0, 36)); + } PresenceInfo[] presence = PresenceService.GetAgents(fList.ToArray()); foreach (PresenceInfo pi in presence) From de895ee54a14f21ee0609e6a3636a31407495aa3 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Mon, 14 Nov 2011 17:18:51 +0000 Subject: [PATCH 3/5] Add very simple FriendsModuleTests.TestNoFriends() --- .../Avatar/Friends/FriendsModule.cs | 4 +- .../Avatar/Friends/Tests/FriendModuleTests.cs | 78 +++++++++++++++++++ OpenSim/Tests/Common/Mock/TestClient.cs | 11 ++- prebuild.xml | 1 + 4 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs diff --git a/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs b/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs index c952ebf8fc..28a2f7362d 100644 --- a/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs +++ b/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs @@ -165,7 +165,9 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends // Instantiate the request handler IHttpServer server = MainServer.GetHttpServer((uint)mPort); - server.AddStreamHandler(new FriendsRequestHandler(this)); + + if (server != null) + server.AddStreamHandler(new FriendsRequestHandler(this)); } if (m_FriendsService == null) diff --git a/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs new file mode 100644 index 0000000000..b52093a98d --- /dev/null +++ b/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs @@ -0,0 +1,78 @@ +/* + * Copyright (c) Contributors, http://opensimulator.org/ + * See CONTRIBUTORS.TXT for a full list of copyright holders. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of the OpenSimulator Project nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE DEVELOPERS ``AS IS'' AND ANY + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE CONTRIBUTORS BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +using System; +using System.Collections.Generic; +using Nini.Config; +using NUnit.Framework; +using OpenMetaverse; +using OpenSim.Framework; +using OpenSim.Region.CoreModules.Avatar.Friends; +using OpenSim.Region.Framework.Scenes; +using OpenSim.Tests.Common; +using OpenSim.Tests.Common.Mock; + +namespace OpenSim.Region.CoreModules.Avatar.Friends.Tests +{ + [TestFixture] + public class FriendsModuleTests + { + private FriendsModule m_fm; + private TestScene m_scene; + + [SetUp] + public void Init() + { + IConfigSource config = new IniConfigSource(); + config.AddConfig("Modules"); + // Not strictly necessary since FriendsModule assumes it is the default (!) + config.Configs["Modules"].Set("FriendsModule", "FriendsModule"); + config.AddConfig("Friends"); + config.Configs["Friends"].Set("Connector", "OpenSim.Services.FriendsService.dll"); + config.AddConfig("FriendsService"); + config.Configs["FriendsService"].Set("StorageProvider", "OpenSim.Data.Null.dll"); + + m_scene = SceneHelpers.SetupScene(); + m_fm = new FriendsModule(); + SceneHelpers.SetupSceneModules(m_scene, config, m_fm); + } + + [Test] + public void TestNoFriends() + { + TestHelpers.InMethod(); +// log4net.Config.XmlConfigurator.Configure(); + + UUID userId = TestHelpers.ParseTail(0x1); + + ScenePresence sp = SceneHelpers.AddScenePresence(m_scene, userId); + + Assert.That(((TestClient)sp.ControllingClient).OfflineNotificationsReceived.Count, Is.EqualTo(0)); + Assert.That(((TestClient)sp.ControllingClient).OnlineNotificationsReceived.Count, Is.EqualTo(0)); + } + } +} \ No newline at end of file diff --git a/OpenSim/Tests/Common/Mock/TestClient.cs b/OpenSim/Tests/Common/Mock/TestClient.cs index 4636961c5e..3ba9ed4e5d 100644 --- a/OpenSim/Tests/Common/Mock/TestClient.cs +++ b/OpenSim/Tests/Common/Mock/TestClient.cs @@ -56,6 +56,10 @@ namespace OpenSim.Tests.Common.Mock private IScene m_scene; + // Properties so that we can get at received data for test purposes + public List OfflineNotificationsReceived { get; private set; } + public List OnlineNotificationsReceived { get; private set; } + // disable warning: public events, part of the public API #pragma warning disable 67 @@ -440,6 +444,9 @@ namespace OpenSim.Tests.Common.Mock m_circuitCode = agentData.circuitcode; m_scene = scene; CapsSeedUrl = agentData.CapsPath; + + OfflineNotificationsReceived = new List(); + OnlineNotificationsReceived = new List(); } /// @@ -827,12 +834,12 @@ namespace OpenSim.Tests.Common.Mock public void SendAgentOffline(UUID[] agentIDs) { - + OfflineNotificationsReceived.AddRange(agentIDs); } public void SendAgentOnline(UUID[] agentIDs) { - + OnlineNotificationsReceived.AddRange(agentIDs); } public void SendSitResponse(UUID TargetID, Vector3 OffsetPos, Quaternion SitOrientation, bool autopilot, diff --git a/prebuild.xml b/prebuild.xml index 4e4bc61b24..7013598537 100644 --- a/prebuild.xml +++ b/prebuild.xml @@ -2915,6 +2915,7 @@ + From 348d15707d129c99bc79cd35d544bc62b9a6f1b2 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Mon, 14 Nov 2011 18:08:02 +0000 Subject: [PATCH 4/5] Add test for adding a friend whilst online --- OpenSim/Data/Null/NullFriendsData.cs | 10 ++++++++- .../Avatar/Friends/FriendsModule.cs | 15 ++++++++----- .../Avatar/Friends/Tests/FriendModuleTests.cs | 21 ++++++++++++++++++- .../Framework/Interfaces/IFriendsModule.cs | 14 +++++++++++++ 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/OpenSim/Data/Null/NullFriendsData.cs b/OpenSim/Data/Null/NullFriendsData.cs index d90788a111..0a4b2426a8 100644 --- a/OpenSim/Data/Null/NullFriendsData.cs +++ b/OpenSim/Data/Null/NullFriendsData.cs @@ -56,13 +56,21 @@ namespace OpenSim.Data.Null /// public FriendsData[] GetFriends(string userID) { - List lst = m_Data.FindAll(delegate (FriendsData fdata) + List lst = m_Data.FindAll(fdata => { return fdata.PrincipalID == userID.ToString(); }); if (lst != null) + { + lst.ForEach(f => + { + FriendsData f2 = m_Data.Find(candidateF2 => f.Friend == candidateF2.PrincipalID); + if (f2 != null) { f.Data["TheirFlags"] = f2.Data["Flags"]; } + }); + return lst.ToArray(); + } return new FriendsData[0]; } diff --git a/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs b/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs index 28a2f7362d..f4281e5ea8 100644 --- a/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs +++ b/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs @@ -576,9 +576,14 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends private void OnApproveFriendRequest(IClientAPI client, UUID agentID, UUID friendID, List callingCardFolders) { - m_log.DebugFormat("[FRIENDS]: {0} accepted friendship from {1}", agentID, friendID); + m_log.DebugFormat("[FRIENDS]: {0} accepted friendship from {1}", client.AgentId, friendID); - StoreFriendships(agentID, friendID); + AddFriend(client, friendID); + } + + public void AddFriend(IClientAPI client, UUID friendID) + { + StoreFriendships(client.AgentId, friendID); // Update the local cache RefetchFriends(client); @@ -588,7 +593,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends // // Try Local - if (LocalFriendshipApproved(agentID, client.Name, friendID)) + if (LocalFriendshipApproved(client.AgentId, client.Name, friendID)) { client.SendAgentOnline(new UUID[] { friendID }); return; @@ -602,7 +607,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends if (friendSession != null) { GridRegion region = GridService.GetRegionByUUID(m_Scenes[0].RegionInfo.ScopeID, friendSession.RegionID); - m_FriendsSimConnector.FriendshipApproved(region, agentID, client.Name, friendID); + m_FriendsSimConnector.FriendshipApproved(region, client.AgentId, client.Name, friendID); client.SendAgentOnline(new UUID[] { friendID }); } } @@ -869,7 +874,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends } /// - /// Update loca cache only + /// Update local cache only /// /// /// diff --git a/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs index b52093a98d..3ad691a519 100644 --- a/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs +++ b/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs @@ -70,9 +70,28 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends.Tests UUID userId = TestHelpers.ParseTail(0x1); ScenePresence sp = SceneHelpers.AddScenePresence(m_scene, userId); - + Assert.That(((TestClient)sp.ControllingClient).OfflineNotificationsReceived.Count, Is.EqualTo(0)); Assert.That(((TestClient)sp.ControllingClient).OnlineNotificationsReceived.Count, Is.EqualTo(0)); } + + [Test] + public void TestAddFriendWhileOnline() + { + TestHelpers.InMethod(); +// log4net.Config.XmlConfigurator.Configure(); + + UUID userId = TestHelpers.ParseTail(0x1); + UUID user2Id = TestHelpers.ParseTail(0x2); + + ScenePresence sp = SceneHelpers.AddScenePresence(m_scene, userId); + ScenePresence sp2 = SceneHelpers.AddScenePresence(m_scene, user2Id); + + // This friendship is currently only one-way, which might be pathalogical in Second Life. + m_fm.AddFriend(sp.ControllingClient, user2Id); + + Assert.That(((TestClient)sp.ControllingClient).OfflineNotificationsReceived.Count, Is.EqualTo(0)); + Assert.That(((TestClient)sp.ControllingClient).OnlineNotificationsReceived.Count, Is.EqualTo(1)); + } } } \ No newline at end of file diff --git a/OpenSim/Region/Framework/Interfaces/IFriendsModule.cs b/OpenSim/Region/Framework/Interfaces/IFriendsModule.cs index d4a6857188..8143164283 100644 --- a/OpenSim/Region/Framework/Interfaces/IFriendsModule.cs +++ b/OpenSim/Region/Framework/Interfaces/IFriendsModule.cs @@ -33,6 +33,20 @@ namespace OpenSim.Region.Framework.Interfaces { public interface IFriendsModule { + /// + /// Add a friend for the given user. + /// + /// + /// This is a one-way friendship. To make a two way friendship you will need to call this again with the + /// client and friend reversed. + /// + /// Ultimately, it would be more useful to take in a user account here rather than having to have a user + /// present in the scene. + /// + /// + /// + void AddFriend(IClientAPI client, UUID friendID); + uint GetFriendPerms(UUID PrincipalID, UUID FriendID); bool SendFriendsOnlineIfNeeded(IClientAPI client); } From a64def8b73d4665b58852bbd75ba3ab010db7432 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Mon, 14 Nov 2011 18:16:14 +0000 Subject: [PATCH 5/5] minor: remove some mono compiler warnings --- .../Agent/AssetTransaction/AgentAssetsTransactions.cs | 2 -- OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs | 6 ------ .../CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs | 2 +- .../Inventory/RemoteXInventoryServiceConnector.cs | 3 +-- 4 files changed, 2 insertions(+), 11 deletions(-) diff --git a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs index eed7cd590a..b557ffe5f2 100644 --- a/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs +++ b/OpenSim/Region/CoreModules/Agent/AssetTransaction/AgentAssetsTransactions.cs @@ -46,7 +46,6 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction // Fields private bool m_dumpAssetsToFile; private Scene m_Scene; - private UUID UserID; private Dictionary XferUploaders = new Dictionary(); // Methods @@ -54,7 +53,6 @@ namespace OpenSim.Region.CoreModules.Agent.AssetTransaction bool dumpAssetsToFile) { m_Scene = scene; - UserID = agentID; m_dumpAssetsToFile = dumpAssetsToFile; } diff --git a/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs b/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs index f4281e5ea8..31ffc4dea8 100644 --- a/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs +++ b/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs @@ -354,18 +354,12 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends continue; } - PresenceInfo presence = null; - PresenceInfo[] presences = PresenceService.GetAgents(new string[] { fid }); - if (presences != null && presences.Length > 0) - presence = presences[0]; im.offline = 0; - im.fromAgentID = fromAgentID.Guid; im.fromAgentName = firstname + " " + lastname; im.imSessionID = im.fromAgentID; im.message = FriendshipMessage(fid); - // Finally LocalFriendshipOffered(agentID, im); } diff --git a/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs index 3ad691a519..942091c4c1 100644 --- a/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs +++ b/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs @@ -85,7 +85,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends.Tests UUID user2Id = TestHelpers.ParseTail(0x2); ScenePresence sp = SceneHelpers.AddScenePresence(m_scene, userId); - ScenePresence sp2 = SceneHelpers.AddScenePresence(m_scene, user2Id); + SceneHelpers.AddScenePresence(m_scene, user2Id); // This friendship is currently only one-way, which might be pathalogical in Second Life. m_fm.AddFriend(sp.ControllingClient, user2Id); diff --git a/OpenSim/Region/CoreModules/ServiceConnectorsOut/Inventory/RemoteXInventoryServiceConnector.cs b/OpenSim/Region/CoreModules/ServiceConnectorsOut/Inventory/RemoteXInventoryServiceConnector.cs index 73ab4e31cc..10ab6d0e6b 100644 --- a/OpenSim/Region/CoreModules/ServiceConnectorsOut/Inventory/RemoteXInventoryServiceConnector.cs +++ b/OpenSim/Region/CoreModules/ServiceConnectorsOut/Inventory/RemoteXInventoryServiceConnector.cs @@ -50,8 +50,7 @@ namespace OpenSim.Region.CoreModules.ServiceConnectorsOut.Inventory /// public Scene Scene { get; set; } - private bool m_Enabled = false; - private Scene m_Scene; + private bool m_Enabled; private XInventoryServicesConnector m_RemoteConnector; private IUserManagement m_UserManager;