fix: remove same origin check and return proper errors from upgrader …

…function (SigNoz#5724) * chore: test websockets * chore: test websockets * chore: test websockets * chore: test websockets * chore: cleanup PR * chore: added back check origin function and loggings to check why the mismatch * chore: the uuid should update on every new request * chore: experiment with delaying the query deletion * chore: experiment with delaying the query deletion * chore: experiment with delaying the query deletion * chore: upgrade nginx conf * chore: upgrade nginx conf * chore: upgrade nginx conf * chore: experiment with delaying the query deletion * chore: cleanup PR * chore: remove print statements
innerfuse · Aug 23, 2024 · a2ac49b · a2ac49b
1 parent 33541a2
commit a2ac49b
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 21 deletions.
diff --git a/deploy/docker/common/nginx-config.conf b/deploy/docker/common/nginx-config.conf
@@ -1,3 +1,8 @@
+map $http_upgrade $connection_upgrade {
+    default upgrade;
+    ''      close;
+}
+
 server {
     listen       3301;
     server_name  _;
@@ -42,6 +47,14 @@ server {
         proxy_read_timeout 600s; 
     }
 
+    location /ws {
+        proxy_pass  http://query-service:8080/ws;
+        proxy_http_version 1.1;
+        proxy_set_header Upgrade "websocket";
+        proxy_set_header Connection "upgrade";
+        proxy_read_timeout 86400;
+    }
+
     # redirect server error pages to the static page /50x.html
     #
     error_page   500 502 503 504  /50x.html;

diff --git a/frontend/src/api/common/getQueryStats.ts b/frontend/src/api/common/getQueryStats.ts
@@ -46,4 +46,17 @@ export function getQueryStats(props: GetQueryStatsProps): void {
 			setData(event?.data);
 		}
 	});
+
+	socket.addEventListener('error', (event) => {
+		console.error(event);
+	});
+
+	socket.addEventListener('close', (event) => {
+		// 1000 is a normal closure status code
+		if (event.code !== 1000) {
+			console.error('WebSocket closed with error:', event);
+		} else {
+			console.error('WebSocket closed normally.');
+		}
+	});
 }
diff --git a/frontend/src/container/LogsExplorerViews/index.tsx b/frontend/src/container/LogsExplorerViews/index.tsx
@@ -263,7 +263,10 @@ function LogsExplorerViews({
 		},
 		undefined,
 		listQueryKeyRef,
-		{},
+		{
+			...(!isEmpty(queryId) &&
+				selectedPanelType !== PANEL_TYPES.LIST && { 'X-SIGNOZ-QUERY-ID': queryId }),
+		},
 	);
 
 	const getRequestData = useCallback(
@@ -352,7 +355,7 @@ function LogsExplorerViews({
 
 	useEffect(() => {
 		setQueryId(v4());
-	}, [isError, isSuccess]);
+	}, [data]);
 
 	useEffect(() => {
 		if (

diff --git a/pkg/query-service/app/http_handler.go b/pkg/query-service/app/http_handler.go
@@ -9,7 +9,6 @@ import (
 	"io"
 	"math"
 	"net/http"
-	"net/url"
 	"regexp"
 	"slices"
 	"strconv"
@@ -214,24 +213,8 @@ func NewAPIHandler(opts APIHandlerOpts) (*APIHandler, error) {
 	}
 
 	aH.Upgrader = &websocket.Upgrader{
-		// Same-origin check is the server's responsibility in websocket spec.
 		CheckOrigin: func(r *http.Request) bool {
-			// Based on the default CheckOrigin implementation in websocket package.
-			originHeader := r.Header.Get("Origin")
-			if len(originHeader) < 1 {
-				return false
-			}
-			origin, err := url.Parse(originHeader)
-			if err != nil {
-				return false
-			}
-
-			// Allow cross origin websocket connections on localhost
-			if strings.HasPrefix(origin.Host, "localhost") {
-				return true
-			}
-
-			return origin.Host == r.Host
+			return true
 		},
 	}
 
@@ -3764,7 +3747,7 @@ func (aH *APIHandler) GetQueryProgressUpdates(w http.ResponseWriter, r *http.Req
 		// Shouldn't happen unless query progress requested after query finished
 		zap.L().Warn(
 			"couldn't subscribe to query progress",
-			zap.String("queryId", queryId), zap.Any("error", err),
+			zap.String("queryId", queryId), zap.Any("error", apiErr),
 		)
 		return
 	}