From dca04c7b61abb7b7ea70299a192425ce3bd05937 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Fri, 22 Jun 2012 23:16:18 +0100 Subject: Avoid a race condition where an incoming request to a script external URL can trigger an exception is the URL was being removed at the same time. This involves three steps 1) Return gracefully in UrlModule.HttpRequestHandler() instead of throwing an exception when the url cannot be found in its index 2) Return true instead of false in HasEvents() if no matching request is found in the map. This call will only happen in the first place for raced requests. 3) Return a 404 in GetEvents() if the request is not in the index, rather than a blank 200 OK. Many thanks to Tom Haines in http://opensimulator.org/mantis/view.php?id=6051 for doing some of the work on this. --- .../Servers/HttpServer/PollServiceEventArgs.cs | 9 +- .../Servers/HttpServer/PollServiceHttpRequest.cs | 7 +- .../CoreModules/Scripting/LSLHttp/UrlModule.cs | 108 ++++++++++++++------- 3 files changed, 84 insertions(+), 40 deletions(-) (limited to 'OpenSim') diff --git a/OpenSim/Framework/Servers/HttpServer/PollServiceEventArgs.cs b/OpenSim/Framework/Servers/HttpServer/PollServiceEventArgs.cs index 9d512c6..3089351 100644 --- a/OpenSim/Framework/Servers/HttpServer/PollServiceEventArgs.cs +++ b/OpenSim/Framework/Servers/HttpServer/PollServiceEventArgs.cs @@ -28,6 +28,7 @@ using System; using System.Collections; using OpenMetaverse; + namespace OpenSim.Framework.Servers.HttpServer { public delegate void RequestMethod(UUID requestID, Hashtable request); @@ -44,7 +45,11 @@ namespace OpenSim.Framework.Servers.HttpServer public NoEventsMethod NoEvents; public RequestMethod Request; public UUID Id; - public PollServiceEventArgs(RequestMethod pRequest, HasEventsMethod pHasEvents, GetEventsMethod pGetEvents, NoEventsMethod pNoEvents,UUID pId) + + public PollServiceEventArgs( + RequestMethod pRequest, + HasEventsMethod pHasEvents, GetEventsMethod pGetEvents, NoEventsMethod pNoEvents, + UUID pId) { Request = pRequest; HasEvents = pHasEvents; @@ -53,4 +58,4 @@ namespace OpenSim.Framework.Servers.HttpServer Id = pId; } } -} +} \ No newline at end of file diff --git a/OpenSim/Framework/Servers/HttpServer/PollServiceHttpRequest.cs b/OpenSim/Framework/Servers/HttpServer/PollServiceHttpRequest.cs index 553a7eb..723530a 100644 --- a/OpenSim/Framework/Servers/HttpServer/PollServiceHttpRequest.cs +++ b/OpenSim/Framework/Servers/HttpServer/PollServiceHttpRequest.cs @@ -31,7 +31,6 @@ using OpenMetaverse; namespace OpenSim.Framework.Servers.HttpServer { - public class PollServiceHttpRequest { public readonly PollServiceEventArgs PollServiceArgs; @@ -39,7 +38,9 @@ namespace OpenSim.Framework.Servers.HttpServer public readonly IHttpRequest Request; public readonly int RequestTime; public readonly UUID RequestID; - public PollServiceHttpRequest(PollServiceEventArgs pPollServiceArgs, IHttpClientContext pHttpContext, IHttpRequest pRequest) + + public PollServiceHttpRequest( + PollServiceEventArgs pPollServiceArgs, IHttpClientContext pHttpContext, IHttpRequest pRequest) { PollServiceArgs = pPollServiceArgs; HttpContext = pHttpContext; @@ -48,4 +49,4 @@ namespace OpenSim.Framework.Servers.HttpServer RequestID = UUID.Random(); } } -} +} \ No newline at end of file diff --git a/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs b/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs index 61afc76..5c05500 100644 --- a/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs +++ b/OpenSim/Region/CoreModules/Scripting/LSLHttp/UrlModule.cs @@ -64,17 +64,25 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp public string uri; } + /// + /// This module provides external URLs for in-world scripts. + /// public class UrlModule : ISharedRegionModule, IUrlModule { private static readonly ILog m_log = LogManager.GetLogger( MethodBase.GetCurrentMethod().DeclaringType); - private Dictionary m_RequestMap = - new Dictionary(); + /// + /// Indexs the URL request metadata (which script requested it, outstanding requests, etc.) by the request ID + /// randomly generated when a request is received for this URL. + /// + private Dictionary m_RequestMap = new Dictionary(); - private Dictionary m_UrlMap = - new Dictionary(); + /// + /// Indexs the URL request metadata (which script requested it, outstanding requests, etc.) by the full URL + /// + private Dictionary m_UrlMap = new Dictionary(); /// /// Maximum number of external urls that can be set up by this module. @@ -224,7 +232,6 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp urlData.urlcode = urlcode; urlData.requests = new Dictionary(); - m_UrlMap[url] = urlData; string uri = "/lslhttps/" + urlcode.ToString() + "/"; @@ -286,7 +293,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp { if (m_RequestMap.ContainsKey(requestId)) { - UrlData urlData=m_RequestMap[requestId]; + UrlData urlData = m_RequestMap[requestId]; string value; if (urlData.requests[requestId].headers.TryGetValue(header,out value)) return value; @@ -295,6 +302,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp { m_log.Warn("[HttpRequestHandler] There was no http-in request with id " + requestId); } + return String.Empty; } @@ -339,6 +347,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp { RemoveUrl(url.Value); removeURLs.Add(url.Key); + foreach (UUID req in url.Value.requests.Keys) m_RequestMap.Remove(req); } @@ -349,20 +358,31 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp } } - private void RemoveUrl(UrlData data) { - m_HttpServer.RemoveHTTPHandler("", "/lslhttp/"+data.urlcode.ToString()+"/"); + m_HttpServer.RemoveHTTPHandler("", "/lslhttp/" + data.urlcode.ToString() + "/"); } private Hashtable NoEvents(UUID requestID, UUID sessionID) { Hashtable response = new Hashtable(); UrlData url; + lock (m_RequestMap) { + // We need to return a 404 here in case the request URL was removed at exactly the same time that a + // request was made. In this case, the request thread can outrace llRemoveURL() and still be polling + // for the request ID. if (!m_RequestMap.ContainsKey(requestID)) + { + response["int_response_code"] = 404; + response["str_response_string"] = ""; + response["keepalive"] = false; + response["reusecontext"] = false; + return response; + } + url = m_RequestMap[requestID]; } @@ -384,53 +404,57 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp return response; } - return response; } private bool HasEvents(UUID requestID, UUID sessionID) { - UrlData url=null; + UrlData url = null; lock (m_RequestMap) { + // We return true here because an external URL request that happened at the same time as an llRemoveURL() + // can still make it through to HttpRequestHandler(). That will return without setting up a request + // when it detects that the URL has been removed. The poller, however, will continue to ask for + // events for that request, so here we will signal that there are events and in GetEvents we will + // return a 404. if (!m_RequestMap.ContainsKey(requestID)) { - return false; + return true; } + url = m_RequestMap[requestID]; if (!url.requests.ContainsKey(requestID)) { - return false; + return true; } } - if (System.Environment.TickCount-url.requests[requestID].startTime>25000) + // Trigger return of timeout response. + if (System.Environment.TickCount - url.requests[requestID].startTime > 25000) { return true; } - if (url.requests[requestID].requestDone) - return true; - else - return false; - + return url.requests[requestID].requestDone; } + private Hashtable GetEvents(UUID requestID, UUID sessionID, string request) { - UrlData url = null; + UrlData url = null; RequestData requestData = null; lock (m_RequestMap) { if (!m_RequestMap.ContainsKey(requestID)) - return NoEvents(requestID,sessionID); + return NoEvents(requestID, sessionID); + url = m_RequestMap[requestID]; requestData = url.requests[requestID]; } if (!requestData.requestDone) - return NoEvents(requestID,sessionID); + return NoEvents(requestID, sessionID); Hashtable response = new Hashtable(); @@ -443,6 +467,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp response["reusecontext"] = false; return response; } + //put response response["int_response_code"] = requestData.responseCode; response["str_response_string"] = requestData.responseBody; @@ -459,6 +484,7 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp return response; } + public void HttpRequestHandler(UUID requestID, Hashtable request) { lock (request) @@ -483,11 +509,22 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp pathInfo = uri.Substring(pos3); - UrlData url = null; - if (!is_ssl) - url = m_UrlMap["http://" + m_ExternalHostNameForLSL + ":" + m_HttpServer.Port.ToString() + uri_tmp]; - else - url = m_UrlMap["https://" + m_ExternalHostNameForLSL + ":" + m_HttpsServer.Port.ToString() + uri_tmp]; + UrlData urlData = null; + + lock (m_UrlMap) + { + string url; + + if (is_ssl) + url = "https://" + m_ExternalHostNameForLSL + ":" + m_HttpsServer.Port.ToString() + uri_tmp; + else + url = "http://" + m_ExternalHostNameForLSL + ":" + m_HttpServer.Port.ToString() + uri_tmp; + + // Avoid a race - the request URL may have been released via llRequestUrl() whilst this + // request was being processed. + if (!m_UrlMap.TryGetValue(url, out urlData)) + return; + } //for llGetHttpHeader support we need to store original URI here //to make x-path-info / x-query-string / x-script-url / x-remote-ip headers @@ -520,11 +557,10 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp queryString = queryString + key + "=" + val + "&"; } } + if (queryString.Length > 1) queryString = queryString.Substring(0, queryString.Length - 1); - } - } //if this machine is behind DNAT/port forwarding, currently this is being @@ -532,26 +568,28 @@ namespace OpenSim.Region.CoreModules.Scripting.LSLHttp requestData.headers["x-remote-ip"] = requestData.headers["remote_addr"]; requestData.headers["x-path-info"] = pathInfo; requestData.headers["x-query-string"] = queryString; - requestData.headers["x-script-url"] = url.url; + requestData.headers["x-script-url"] = urlData.url; //requestData.ev = new ManualResetEvent(false); - lock (url.requests) + lock (urlData.requests) { - url.requests.Add(requestID, requestData); + urlData.requests.Add(requestID, requestData); } + lock (m_RequestMap) { - //add to request map - m_RequestMap.Add(requestID, url); + m_RequestMap.Add(requestID, urlData); } - url.engine.PostScriptEvent(url.itemID, "http_request", new Object[] { requestID.ToString(), request["http-method"].ToString(), request["body"].ToString() }); + urlData.engine.PostScriptEvent( + urlData.itemID, + "http_request", + new Object[] { requestID.ToString(), request["http-method"].ToString(), request["body"].ToString() }); //send initial response? // Hashtable response = new Hashtable(); return; - } catch (Exception we) { -- cgit v1.1