NOISSUE - Add WebSocket support to HTTP Proxy by arvindh123 · Pull Request #87 · absmach/mproxy

arvindh123 · 2025-04-09T14:33:28Z

Pull request title should be MF-XXX - description or NOISSUE - description where XXX is ID of issue that this PR relate to.
Please review the CONTRIBUTING.md file for detailed contributing guidelines.

What does this do?

Which issue(s) does this PR fix/relate to?

Put here Resolves #XXX to auto-close the issue that your PR fixes (if such)

List any changes that modify/break current functionality

Have you included tests for your changes?

Did you document any new/modified functionality?

Notes

felixgateru

LGTM

felixgateru

I am unable to start the service now:

dborovcanin · 2025-04-17T21:39:36Z

@arvindh123 Please resolve the comment and conflicts.

dborovcanin · 2025-04-21T18:48:01Z

@arvindh123 Please resolve conflicts.

Signed-off-by: Arvindh <arvindh91@gmail.com>

dborovcanin · 2025-04-22T08:38:48Z

+const errNotAllowed = "origin - %s is not allowed"
+
+type OriginChecker interface {
+	CheckOrigin(r *http.Request) error


The idea of OriginChecker and BypassMatcher was to have the same interface signature so we can use a single interface:

type Checker interface { Check(*http.Request) error }

dborovcanin · 2025-04-22T08:43:05Z

+
+import "strings"
+
+func AddSuffixSlash(path string) string {


Let's avoid having generic helpers/common/util packags. Checkers are related to HTTP layer, while suffix is transport-layer path handler.

dborovcanin · 2025-04-22T08:45:16Z

+func (p *Proxy) handleWebSocket(w http.ResponseWriter, r *http.Request, s *session.Session) {
+	topic := r.URL.Path
+	ctx := session.NewContext(context.Background(), s)
+	if err := p.session.AuthConnect(ctx); err != nil {


Create a new ws.go file with all the WS-related handling.

dborovcanin · 2025-04-22T08:47:06Z

+	}
+	return fmt.Errorf("%s error: %w", prefix, err)
+}
+


This can be confusing as to what is UP and what is DOWN. Maybe simpler "between client and mGate" and "between mGate and server"is better.

Signed-off-by: Arvindh <arvindh91@gmail.com>

dborovcanin · 2025-04-23T09:52:36Z

+type Checkers interface {
+	ShouldBypass(r *http.Request) error
+	CheckOrigin(r *http.Request) error
+}


@arvindh123 Please use a single interface Checker with Check method. There is no need for this complication. Use two implementations - one for origin and the other for bypass- according to needs.

dborovcanin · 2025-04-23T10:06:11Z

+	switch {
+	case ok:
+		return username, password
+	case len(r.URL.Query()[authzQueryKey]) != 0:


Use r.URL.Query().Get(authzQueryKey) instead.

Signed-off-by: Arvindh <arvindh91@gmail.com>

dborovcanin

No need for two files with the checker suffix - a single file checker.go with both checkers implementations is fine, since implementations are relatively simple.

dborovcanin · 2025-04-24T10:38:03Z

+	"regexp"
+)
+
+const errNotByPassed = "route - %s is not in bypass list"


Rename to errNotBypassFmt for better self-documenting.

dborovcanin · 2025-04-24T10:39:51Z

@@ -0,0 +1,51 @@
+// Copyright (c) Abstract Machines


Use bypass instead of byPass since it's cleaner to use bypass than by pass.

dborovcanin · 2025-04-24T10:45:30Z

 		return username, password
-	case len(r.URL.Query()[authzQueryKey]) != 0:
-		password = r.URL.Query()[authzQueryKey][0]
+	case len(r.URL.Query().Get(authzQueryKey)) != 0:


No need for len, compare to empty string.

dborovcanin · 2025-04-24T10:46:18Z

@@ -0,0 +1,13 @@
+// Copyright (c) Abstract Machines


Rename the file to path.go.

dborovcanin · 2025-04-24T20:39:00Z

@arvindh123 Please address remarks here.

Signed-off-by: Arvindh <arvindh91@gmail.com>

arvindh123 force-pushed the websocket_origins branch from 7999968 to ad1a8a7 Compare April 9, 2025 14:36

felixgateru requested changes Apr 14, 2025

View reviewed changes

Comment thread pkg/common/origincheck.go Outdated

arvindh123 force-pushed the websocket_origins branch from ddd66c0 to 59aa0c1 Compare April 16, 2025 07:24

felixgateru previously approved these changes Apr 16, 2025

View reviewed changes

dborovcanin requested changes Apr 16, 2025

View reviewed changes

Comment thread pkg/common/bypass.go Outdated

arvindh123 dismissed felixgateru’s stale review via a5f8d93 April 16, 2025 11:34

dborovcanin requested changes Apr 16, 2025

View reviewed changes

Comment thread pkg/common/bypass.go Outdated

arvindh123 added this to Magistrala Apr 17, 2025

arvindh123 assigned felixgateru Apr 17, 2025

arvindh123 moved this to 🩺 Review and testing in Magistrala Apr 17, 2025

arvindh123 force-pushed the websocket_origins branch from 486bd1a to 1fd2cfd Compare April 17, 2025 14:59

arvindh123 mentioned this pull request Apr 17, 2025

NOISSUE - Update mGate version in http and ws adapters absmach/magistrala#2825

Merged

felixgateru approved these changes Apr 17, 2025

View reviewed changes

arvindh123 assigned arvindh123 and unassigned felixgateru Apr 17, 2025

felixgateru requested changes Apr 17, 2025

View reviewed changes

github-project-automation bot moved this from 🩺 Review and testing to 🚧 In Progress in Magistrala Apr 17, 2025

This comment was marked as duplicate.

Sign in to view

felixgateru reviewed Apr 17, 2025

View reviewed changes

Comment thread pkg/http/http.go Outdated

arvindh123 force-pushed the websocket_origins branch from 2d5f3da to 369ec37 Compare April 18, 2025 05:59

dborovcanin changed the title ~~NOISSUE: Add WebSocket support to HTTP Proxy~~ NOISSUE - Add WebSocket support to HTTP Proxy Apr 21, 2025

arvindh123 force-pushed the websocket_origins branch from 2335a5a to 7697147 Compare April 22, 2025 05:20

arvindh123 added 6 commits April 22, 2025 13:34

remove print line

f7222b3

Signed-off-by: Arvindh <arvindh91@gmail.com>

add websocket to http proxy

055c834

Signed-off-by: Arvindh <arvindh91@gmail.com>

add websocket to http proxy

a4b314e

Signed-off-by: Arvindh <arvindh91@gmail.com>

initalized NewOriginChecker

54a860c

Signed-off-by: Arvindh <arvindh91@gmail.com>

add toolchain

daca640

Signed-off-by: Arvindh <arvindh91@gmail.com>

remove toolchain

08a9742

Signed-off-by: Arvindh <arvindh91@gmail.com>

arvindh123 added 11 commits April 22, 2025 13:34

format files

24dabae

Signed-off-by: Arvindh <arvindh91@gmail.com>

fix url path

33a6f53

Signed-off-by: Arvindh <arvindh91@gmail.com>

fix targetUrl

33b3378

Signed-off-by: Arvindh <arvindh91@gmail.com>

fix mqtt ws

68f6904

Signed-off-by: Arvindh <arvindh91@gmail.com>

add env for listen and target portocol,host,port,path

c9edf69

Signed-off-by: Arvindh <arvindh91@gmail.com>

upgrade liniter

0949901

Signed-off-by: Arvindh <arvindh91@gmail.com>

fix linter errors

cbf2969

Signed-off-by: Arvindh <arvindh91@gmail.com>

upgrade liniter

b2c948b

Signed-off-by: Arvindh <arvindh91@gmail.com>

fix websocket proxy

01cb32c

Signed-off-by: Arvindh <arvindh91@gmail.com>

update getPass func

1972a33

Signed-off-by: Arvindh <arvindh91@gmail.com>

websocket process

c0b8fce

Signed-off-by: Arvindh <arvindh91@gmail.com>

arvindh123 force-pushed the websocket_origins branch 2 times, most recently from f22b8fc to d3a1484 Compare April 22, 2025 08:40

fix linter errors

c5324d4

Signed-off-by: Arvindh <arvindh91@gmail.com>

arvindh123 force-pushed the websocket_origins branch from d3a1484 to c5324d4 Compare April 22, 2025 08:40

dborovcanin requested changes Apr 22, 2025

View reviewed changes

move checkers to single interface

fe28d8d

Signed-off-by: Arvindh <arvindh91@gmail.com>

arvindh123 force-pushed the websocket_origins branch from 211b0f1 to fe28d8d Compare April 22, 2025 09:52

update prefix

5b1db62

Signed-off-by: Arvindh <arvindh91@gmail.com>

arvindh123 force-pushed the websocket_origins branch from cd88db4 to 5b1db62 Compare April 22, 2025 17:58

dborovcanin requested changes Apr 23, 2025

View reviewed changes

dborovcanin reviewed Apr 23, 2025

View reviewed changes

modified checker

e77af86

Signed-off-by: Arvindh <arvindh91@gmail.com>

dborovcanin requested changes Apr 24, 2025

View reviewed changes

consolidate and format function names

e40a4bc

Signed-off-by: Arvindh <arvindh91@gmail.com>

arvindh123 force-pushed the websocket_origins branch from c592a92 to e40a4bc Compare April 25, 2025 07:45

dborovcanin approved these changes Apr 25, 2025

View reviewed changes

dborovcanin merged commit d194345 into absmach:main Apr 25, 2025
1 check passed

github-project-automation bot moved this from 🚧 In Progress to ✅ Done in Magistrala Apr 25, 2025

Conversation

arvindh123 commented Apr 9, 2025

What does this do?

Which issue(s) does this PR fix/relate to?

List any changes that modify/break current functionality

Have you included tests for your changes?

Did you document any new/modified functionality?

Notes

Uh oh!

Uh oh!

felixgateru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

felixgateru left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as duplicate.

Uh oh!

Uh oh!

dborovcanin commented Apr 17, 2025

Uh oh!

dborovcanin commented Apr 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dborovcanin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dborovcanin commented Apr 24, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants