Merge pull request #2937 from SCADA-LTS/fix/#2708_Fixed_Narrowly_avoi…

…ded_an_infinite_loop_in_execute_for_HttpSenderRT2 #2708 Fixed 'Narrowly avoided an infinite loop in execute' for HttpSe…
SCADA-LTS · Jul 3, 2024 · 5293ed2 · 5293ed2
2 parents dfbba5e + efaced9
commit 5293ed2
Show file tree

Hide file tree

Showing 10 changed files with 113 additions and 16 deletions.
diff --git a/src/com/serotonin/mango/Common.java b/src/com/serotonin/mango/Common.java
@@ -35,6 +35,8 @@
 import org.apache.commons.httpclient.HttpClient;
 import org.apache.commons.httpclient.UsernamePasswordCredentials;
 import org.apache.commons.httpclient.auth.AuthScope;
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.commons.httpclient.methods.PostMethod;
 import org.apache.commons.httpclient.params.HttpClientParams;
 import org.apache.commons.httpclient.params.HttpConnectionManagerParams;
 import org.directwebremoting.WebContext;
@@ -60,6 +62,7 @@
 import com.serotonin.util.StringUtils;
 import com.serotonin.web.i18n.LocalizableMessage;
 import org.scada_lts.serial.SerialPortUtils;
+import org.scada_lts.utils.SystemSettingsUtils;
 import org.springframework.security.core.GrantedAuthority;
 
 public class Common {
@@ -465,16 +468,16 @@ public synchronized static String encrypt(String plaintext) {
 	//
 	// HttpClient
 	public static HttpClient getHttpClient() {
-		return getHttpClient(30000); // 30 seconds.
+		int timeout = SystemSettingsUtils.getHttpTimeoutMs();
+		return getHttpClient(timeout); // 15 seconds.
 	}
 
 	public static HttpClient getHttpClient(int timeout) {
 		HttpConnectionManagerParams managerParams = new HttpConnectionManagerParams();
 		managerParams.setConnectionTimeout(timeout);
 		managerParams.setSoTimeout(timeout);
 
-		HttpClientParams params = new HttpClientParams();
-		params.setSoTimeout(timeout);
+		HttpClientParams params = createHttpClientParams(timeout);
 
 		HttpClient client = new HttpClient();
 		client.getHttpConnectionManager().setParams(managerParams);
@@ -541,4 +544,24 @@ public static String generateXid(String prefix) {
 		return prefix + StringUtils.generateRandomString(6, "0123456789");
 	}
 
+	public static GetMethod createGetMethod(String url) {
+		GetMethod getMethod = new GetMethod(url);
+		getMethod.setFollowRedirects(SystemSettingsUtils.isHttpFollowRedirects());
+		return getMethod;
+	}
+
+	public static PostMethod createPostMethod(String url) {
+		PostMethod postMethod = new PostMethod(url);
+		postMethod.setFollowRedirects(SystemSettingsUtils.isHttpFollowRedirects());
+		return postMethod;
+	}
+
+	private static HttpClientParams createHttpClientParams(int timeout) {
+		HttpClientParams params = new HttpClientParams();
+		params.setSoTimeout(timeout);
+		params.setParameter(HttpClientParams.REJECT_RELATIVE_REDIRECT, SystemSettingsUtils.isHttpRejectRelativeRedirect());
+		params.setParameter(HttpClientParams.MAX_REDIRECTS, SystemSettingsUtils.getHttpMaxRedirects());
+		params.setParameter(HttpClientParams.ALLOW_CIRCULAR_REDIRECTS, SystemSettingsUtils.isHttpAllowCircularRedirects());
+		return params;
+	}
 }
diff --git a/src/com/serotonin/mango/rt/dataSource/http/HttpImageDataSourceRT.java b/src/com/serotonin/mango/rt/dataSource/http/HttpImageDataSourceRT.java
@@ -40,7 +40,6 @@
 import com.serotonin.mango.rt.dataImage.types.ImageValue;
 import com.serotonin.mango.rt.dataSource.DataSourceRT;
 import com.serotonin.mango.rt.dataSource.PollingDataSource;
-import com.serotonin.mango.rt.maint.work.WorkItem;
 import com.serotonin.mango.vo.dataSource.http.HttpImageDataSourceVO;
 import com.serotonin.mango.vo.dataSource.http.HttpImagePointLocatorVO;
 import com.serotonin.util.image.BoxScaledImage;
@@ -50,6 +49,8 @@
 import com.serotonin.web.i18n.LocalizableException;
 import com.serotonin.web.i18n.LocalizableMessage;
 
+import static com.serotonin.mango.Common.createGetMethod;
+
 /**
  * @author Matthew Lohbihler
  */
@@ -247,7 +248,7 @@ public static byte[] getData(String url, int timeoutSeconds, int retries, int re
             LocalizableMessage message;
 
             try {
-                method = new GetMethod(url);
+                method = createGetMethod(url);
                 int responseCode = client.executeMethod(method);
 
                 if (responseCode == HttpStatus.SC_OK) {

diff --git a/src/com/serotonin/mango/rt/dataSource/http/HttpRetrieverDataSourceRT.java b/src/com/serotonin/mango/rt/dataSource/http/HttpRetrieverDataSourceRT.java
@@ -41,6 +41,8 @@
 import java.util.Collections;
 import java.util.List;
 
+import static com.serotonin.mango.Common.createGetMethod;
+
 /**
  * @author Matthew Lohbihler
  */
@@ -131,7 +133,7 @@ protected void doPoll(long time) {
     }
 
     private static GetMethod createMethodForClient(String url, List<KeyValuePair> staticHeaders) {
-        GetMethod method = new GetMethod(url);
+        GetMethod method = createGetMethod(url);
         if (!staticHeaders.isEmpty()) {
             for (KeyValuePair kvp : staticHeaders) {
                 if (kvp.getKey().equals("Authorization")) {

diff --git a/src/com/serotonin/mango/rt/dataSource/pachube/PachubeDataSourceRT.java b/src/com/serotonin/mango/rt/dataSource/pachube/PachubeDataSourceRT.java
@@ -59,6 +59,8 @@
 import com.serotonin.web.i18n.LocalizableException;
 import com.serotonin.web.i18n.LocalizableMessage;
 
+import static com.serotonin.mango.Common.createGetMethod;
+
 public class PachubeDataSourceRT extends PollingDataSource {
     public static final int DATA_RETRIEVAL_FAILURE_EVENT = 1;
     public static final int PARSE_EXCEPTION_EVENT = 2;
@@ -212,7 +214,7 @@ public static Map<String, PachubeValue> getData(HttpClient client, int feedId, S
         GetMethod method = null;
 
         try {
-            method = new GetMethod("http://www.pachube.com/api/feeds/" + feedId + ".json");
+            method = createGetMethod("http://www.pachube.com/api/feeds/" + feedId + ".json");
             method.addRequestHeader(new Header(HEADER_API_KEY, apiKey));
             method.addRequestHeader("User-Agent", "Scada-LTS M2M Pachube data source");
 

diff --git a/src/com/serotonin/mango/rt/maint/VersionCheck.java b/src/com/serotonin/mango/rt/maint/VersionCheck.java
@@ -52,6 +52,8 @@
 import com.serotonin.web.http.HttpUtils;
 import com.serotonin.web.i18n.LocalizableMessage;
 
+import static com.serotonin.mango.Common.createPostMethod;
+
 /**
  * @author Matthew Lohbihler
  * 
@@ -147,7 +149,7 @@ private static String newVersionCheckImpl(String notifLevel)
 			throws Exception {
 		HttpClient httpClient = Common.getHttpClient();
 
-		PostMethod postMethod = new PostMethod(
+		PostMethod postMethod = createPostMethod(
 				Common.getGroveUrl(Common.GroveServlets.VERSION_CHECK));
 
 		postMethod.addParameter("instanceId", getInstanceId());

diff --git a/src/com/serotonin/mango/rt/publish/httpSender/HttpSenderRT.java b/src/com/serotonin/mango/rt/publish/httpSender/HttpSenderRT.java
@@ -50,6 +50,9 @@
 import org.apache.commons.httpclient.methods.RequestEntity;
 import org.apache.commons.httpclient.methods.StringRequestEntity;
 
+import static com.serotonin.mango.Common.createGetMethod;
+import static com.serotonin.mango.Common.createPostMethod;
+
 /**
  * @author Matthew Lohbihler
  */
@@ -130,7 +133,7 @@ private boolean send(List<PublishQueueEntry<HttpPointVO>> list) {
 
             HttpMethodBase method;
             if (vo.isUsePost()) {
-                PostMethod post = new PostMethod(vo.getUrl());
+                PostMethod post = createPostMethod(vo.getUrl());
                 post.addParameters(params);
                 if (vo.isUseJSON()) {
                     try {
@@ -143,7 +146,7 @@ private boolean send(List<PublishQueueEntry<HttpPointVO>> list) {
                 method = post;
             }
             else {
-                GetMethod get = new GetMethod(vo.getUrl());
+                GetMethod get = createGetMethod(vo.getUrl());
                 get.setQueryString(params);
                 method = get;
             }

diff --git a/src/com/serotonin/mango/util/MangoGroveLogAppender.java b/src/com/serotonin/mango/util/MangoGroveLogAppender.java
@@ -32,7 +32,7 @@
 
 import com.serotonin.mango.Common;
 import org.scada_lts.dao.SystemSettingsDAO;
-import com.serotonin.mango.rt.maint.VersionCheck;
+import static com.serotonin.mango.Common.createPostMethod;
 
 /**
  * @author Matthew Lohbihler
@@ -57,7 +57,7 @@ protected void append(LoggingEvent event) {
         }
 
         HttpClient client = Common.getHttpClient();
-        PostMethod method = new PostMethod(Common.getGroveUrl(Common.GroveServlets.MANGO_LOG));
+        PostMethod method = createPostMethod(Common.getGroveUrl(Common.GroveServlets.MANGO_LOG));
         method.addParameter("productId", "Scada-LTS");
         method.addParameter("productVersion", Common.getVersion());
         method.addParameter("level", event.getLevel().toString());

diff --git a/src/com/serotonin/mango/web/dwr/beans/HttpSenderTester.java b/src/com/serotonin/mango/web/dwr/beans/HttpSenderTester.java
@@ -32,6 +32,9 @@
 import com.serotonin.web.http.HttpUtils;
 import org.apache.commons.httpclient.methods.StringRequestEntity;
 
+import static com.serotonin.mango.Common.createGetMethod;
+import static com.serotonin.mango.Common.createPostMethod;
+
 /**
  * @author Matthew Lohbihler
  */
@@ -56,7 +59,7 @@ public HttpSenderTester(String url, boolean usePost, List<KeyValuePair> staticHe
     public void run() {
         HttpMethodBase method;
         if (usePost) {
-            PostMethod post = new PostMethod(url);
+            PostMethod post = createPostMethod(url);
             post.addParameters(convertToNVPs(staticParameters));
             if (staticHeaders.stream().filter(o -> o.getKey().equals("Content-Type")).findAny().filter(o -> o.getValue().equals("application/json")).isPresent()) {
                 try {
@@ -69,11 +72,10 @@ public void run() {
             method = post;
         }
         else {
-            GetMethod get = new GetMethod(url);
+            GetMethod get = createGetMethod(url);
             get.setQueryString(convertToNVPs(staticParameters));
             method = get;
         }
-        method.setFollowRedirects(false);
         // Add a recognizable header
         method.addRequestHeader("User-Agent", HttpSenderRT.USER_AGENT);
 

diff --git a/src/org/scada_lts/utils/SystemSettingsUtils.java b/src/org/scada_lts/utils/SystemSettingsUtils.java
@@ -58,6 +58,12 @@ private SystemSettingsUtils() {}
     public static final String WORK_ITEMS_CONFIG_BATCH_WRITE_BEHIND_MAX_INSTANCES_KEY = "workitems.config.BatchWriteBehind.maxInstances";
     public static final String WORK_ITEMS_CONFIG_BATCH_WRITE_BEHIND_SPAWN_THRESHOLD_KEY = "workitems.config.BatchWriteBehind.spawnThreshold";
 
+    public static final String HTTP_PROTOCOL_REJECT_RELATIVE_REDIRECT_KEY = "http.protocol.reject-relative-redirect";
+    public static final String HTTP_PROTOCOL_METHOD_FOLLOW_REDIRECTS_KEY = "http.protocol.method.follow-redirects";
+    public static final String HTTP_PROTOCOL_MAX_REDIRECTS_KEY = "http.protocol.max-redirects";
+    public static final String HTTP_PROTOCOL_ALLOW_CIRCULAR_REDIRECTS_KEY = "http.protocol.allow-circular-redirects";
+    public static final String HTTP_PROTOCOL_TIMEOUT_MS_KEY = "http.protocol.timeout-ms";
+
     private static final org.apache.commons.logging.Log LOG = LogFactory.getLog(SystemSettingsUtils.class);
 
     public static DataPointSyncMode getDataPointSynchronizedMode() {
@@ -460,4 +466,54 @@ public static int getWorkItemsConfigBatchWriteBehindSpawnThreshold() {
             return 255;
         }
     }
+
+    public static int getHttpMaxRedirects() {
+        try {
+            String eventPendingLimit = ScadaConfig.getInstance().getConf().getProperty(HTTP_PROTOCOL_MAX_REDIRECTS_KEY, "100");
+            return Integer.parseInt(eventPendingLimit);
+        } catch (Exception e) {
+            LOG.error(e.getMessage());
+            return 100;
+        }
+    }
+
+    public static boolean isHttpAllowCircularRedirects() {
+        try {
+            String eventPendingCache = ScadaConfig.getInstance().getConf().getProperty(HTTP_PROTOCOL_ALLOW_CIRCULAR_REDIRECTS_KEY, "false");
+            return Boolean.parseBoolean(eventPendingCache);
+        } catch (Exception e) {
+            LOG.error(e.getMessage());
+            return false;
+        }
+    }
+
+    public static boolean isHttpRejectRelativeRedirect() {
+        try {
+            String eventPendingCache = ScadaConfig.getInstance().getConf().getProperty(HTTP_PROTOCOL_REJECT_RELATIVE_REDIRECT_KEY, "true");
+            return Boolean.parseBoolean(eventPendingCache);
+        } catch (Exception e) {
+            LOG.error(e.getMessage());
+            return true;
+        }
+    }
+
+    public static boolean isHttpFollowRedirects() {
+        try {
+            String eventPendingCache = ScadaConfig.getInstance().getConf().getProperty(HTTP_PROTOCOL_METHOD_FOLLOW_REDIRECTS_KEY, "false");
+            return Boolean.parseBoolean(eventPendingCache);
+        } catch (Exception e) {
+            LOG.error(e.getMessage());
+            return false;
+        }
+    }
+
+    public static int getHttpTimeoutMs() {
+        try {
+            String eventPendingLimit = ScadaConfig.getInstance().getConf().getProperty(HTTP_PROTOCOL_TIMEOUT_MS_KEY, "15001");
+            return Integer.parseInt(eventPendingLimit);
+        } catch (Exception e) {
+            LOG.error(e.getMessage());
+            return 15001;
+        }
+    }
 }
diff --git a/webapp-resources/env.properties b/webapp-resources/env.properties
@@ -177,4 +177,10 @@ thread-pool-executor.high-priority.time-unit-enum-value=SECONDS
 
 workitems.config.BatchWriteBehind.maxRows=2000
 workitems.config.BatchWriteBehind.maxInstances=5
-workitems.config.BatchWriteBehind.spawnThreshold=10000
+workitems.config.BatchWriteBehind.spawnThreshold=10000
+
+http.protocol.reject-relative-redirect=true
+http.protocol.method.follow-redirects=false
+http.protocol.max-redirects=100
+http.protocol.allow-circular-redirects=false
+http.protocol.timeout-ms=15000