Repository: caddyserver/caddy (language: go)

Issue:
Warn for `ws://` and `wss://` site and upstream addresses
I've seen that commonly, users try to use `ws://` or `wss://` either as site addresses or proxy upstream addresses. Neither of these work in Caddy, they're specific to browsers for use with the `new WebSocket(url)` JS API.

We should warn/error when those are used in the config, and suggest using `http://` or `https://` (or omitting the scheme) respectively.




Discussion / hints:
Hey @francislavoie I would like to take this up. Can you please assign this to me?
You can just open up a PR when you have something 👍
I went through the code and was able to understand that an adapter is being used to adapt the config provided and convert it to json. I found there are multiple layers of abstractions which is making it slightly challenging to understand the flow of adaption. Would you like to point out some specific things that I should look for?

@francislavoie @mholt 
@singhalkarun Maybe somewhere around here?

https://github.com/caddyserver/caddy/blob/d6f86cccf5fa5b4eb30141da390cf2439746c5da/modules/caddyhttp/reverseproxy/caddyfile.go#L162

I wonder if we should just return an error if it's not http or https. Are there reasons to allow other schemes here?
> I wonder if we should just return an error if it's not http or https. Are there reasons to allow other schemes here?

I think we should throw an error. If we try to use wss as proxy url prefix in nginx, it also throws error (saying invalid url prefix). Also, I believe we should only allow 3 vaues as scheme, **http**, **https** and **empty scheme**. 

Raising a PR as per this. 


Fix patch:
diff --git a/caddyconfig/httpcaddyfile/addresses.go b/caddyconfig/httpcaddyfile/addresses.go
index b6a8ac07254..658da48ed64 100644
--- a/caddyconfig/httpcaddyfile/addresses.go
+++ b/caddyconfig/httpcaddyfile/addresses.go
@@ -197,6 +197,17 @@ func (st *ServerType) listenerAddrsForServerBlockKey(sblock serverBlock, key str
 	}
 	addr = addr.Normalize()
 
+	switch addr.Scheme {
+	case "wss":
+		return nil, fmt.Errorf("the scheme wss:// is only supported in browsers; use https:// instead")
+	case "ws":
+		return nil, fmt.Errorf("the scheme ws:// is only supported in browsers; use http:// instead")
+	case "https", "http", "":
+		// Do nothing or handle the valid schemes
+	default:
+		return nil, fmt.Errorf("unsupported URL scheme %s://", addr.Scheme)
+	}
+
 	// figure out the HTTP and HTTPS ports; either
 	// use defaults, or override with user config
 	httpPort, httpsPort := strconv.Itoa(caddyhttp.DefaultHTTPPort), strconv.Itoa(caddyhttp.DefaultHTTPSPort)
diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go
index a986f89f250..674776f87a8 100644
--- a/modules/caddyhttp/reverseproxy/caddyfile.go
+++ b/modules/caddyhttp/reverseproxy/caddyfile.go
@@ -155,6 +155,18 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 		// the underlying JSON does not yet support different
 		// transports (protocols or schemes) to each backend,
 		// so we remember the last one we see and compare them
+
+		switch pa.scheme {
+		case "wss":
+			return d.Errf("the scheme wss:// is only supported in browsers; use https:// instead")
+		case "ws":
+			return d.Errf("the scheme ws:// is only supported in browsers; use http:// instead")
+		case "https", "http", "h2c", "":
+			// Do nothing or handle the valid schemes
+		default:
+			return d.Errf("unsupported URL scheme %s://", pa.scheme)
+		}
+
 		if commonScheme != "" && pa.scheme != commonScheme {
 			return d.Errf("for now, all proxy upstreams must use the same scheme (transport protocol); expecting '%s://' but got '%s://'",
 				commonScheme, pa.scheme)
@@ -193,7 +205,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 	for _, up := range d.RemainingArgs() {
 		err := appendUpstream(up)
 		if err != nil {
-			return err
+			return fmt.Errorf("parsing upstream '%s': %w", up, err)
 		}
 	}
 
@@ -217,7 +229,7 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 			for _, up := range args {
 				err := appendUpstream(up)
 				if err != nil {
-					return err
+					return fmt.Errorf("parsing upstream '%s': %w", up, err)
 				}
 			}
 


Test patch:
diff --git a/caddytest/integration/caddyfile_test.go b/caddytest/integration/caddyfile_test.go
index 3f3ba0ae2a7..205bc5b914b 100644
--- a/caddytest/integration/caddyfile_test.go
+++ b/caddytest/integration/caddyfile_test.go
@@ -135,3 +135,352 @@ func TestReplIndex(t *testing.T) {
 	// act and assert
 	tester.AssertGetResponse("http://localhost:9080/", 200, "")
 }
+
+func TestInvalidPrefix(t *testing.T) {
+	type testCase struct {
+		config, expectedError string
+	}
+
+	failureCases := []testCase{
+		{
+			config:        `wss://localhost`,
+			expectedError: `the scheme wss:// is only supported in browsers; use https:// instead`,
+		},
+		{
+			config:        `ws://localhost`,
+			expectedError: `the scheme ws:// is only supported in browsers; use http:// instead`,
+		},
+		{
+			config:        `someInvalidPrefix://localhost`,
+			expectedError: "unsupported URL scheme someinvalidprefix://",
+		},
+		{
+			config:        `h2c://localhost`,
+			expectedError: `unsupported URL scheme h2c://`,
+		},
+		{
+			config:        `localhost, wss://localhost`,
+			expectedError: `the scheme wss:// is only supported in browsers; use https:// instead`,
+		},
+		{
+			config: `localhost {
+  				reverse_proxy ws://localhost"
+            }`,
+			expectedError: `the scheme ws:// is only supported in browsers; use http:// instead`,
+		},
+		{
+			config: `localhost {
+  				reverse_proxy someInvalidPrefix://localhost"
+			}`,
+			expectedError: `unsupported URL scheme someinvalidprefix://`,
+		},
+	}
+
+	for _, failureCase := range failureCases {
+		caddytest.AssertLoadError(t, failureCase.config, "caddyfile", failureCase.expectedError)
+	}
+}
+
+func TestValidPrefix(t *testing.T) {
+	type testCase struct {
+		rawConfig, expectedResponse string
+	}
+
+	successCases := []testCase{
+		{
+			"localhost",
+			`{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":443"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"localhost"
+									]
+								}
+							],
+							"terminal": true
+						}
+					]
+				}
+			}
+		}
+	}
+}`,
+		},
+		{
+			"https://localhost",
+			`{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":443"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"localhost"
+									]
+								}
+							],
+							"terminal": true
+						}
+					]
+				}
+			}
+		}
+	}
+}`,
+		},
+		{
+			"http://localhost",
+			`{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":80"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"localhost"
+									]
+								}
+							],
+							"terminal": true
+						}
+					]
+				}
+			}
+		}
+	}
+}`,
+		},
+		{
+			`localhost {
+			reverse_proxy http://localhost:3000
+		 }`,
+			`{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":443"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"localhost"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"handle": [
+												{
+													"handler": "reverse_proxy",
+													"upstreams": [
+														{
+															"dial": "localhost:3000"
+														}
+													]
+												}
+											]
+										}
+									]
+								}
+							],
+							"terminal": true
+						}
+					]
+				}
+			}
+		}
+	}
+}`,
+		},
+		{
+			`localhost {
+			reverse_proxy https://localhost:3000
+		 }`,
+			`{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":443"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"localhost"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"handle": [
+												{
+													"handler": "reverse_proxy",
+													"transport": {
+														"protocol": "http",
+														"tls": {}
+													},
+													"upstreams": [
+														{
+															"dial": "localhost:3000"
+														}
+													]
+												}
+											]
+										}
+									]
+								}
+							],
+							"terminal": true
+						}
+					]
+				}
+			}
+		}
+	}
+}`,
+		},
+		{
+			`localhost {
+			reverse_proxy h2c://localhost:3000
+		 }`,
+			`{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":443"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"localhost"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"handle": [
+												{
+													"handler": "reverse_proxy",
+													"transport": {
+														"protocol": "http",
+														"versions": [
+															"h2c",
+															"2"
+														]
+													},
+													"upstreams": [
+														{
+															"dial": "localhost:3000"
+														}
+													]
+												}
+											]
+										}
+									]
+								}
+							],
+							"terminal": true
+						}
+					]
+				}
+			}
+		}
+	}
+}`,
+		},
+		{
+			`localhost {
+			reverse_proxy localhost:3000
+		 }`,
+			`{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":443"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"localhost"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"handle": [
+												{
+													"handler": "reverse_proxy",
+													"upstreams": [
+														{
+															"dial": "localhost:3000"
+														}
+													]
+												}
+											]
+										}
+									]
+								}
+							],
+							"terminal": true
+						}
+					]
+				}
+			}
+		}
+	}
+}`,
+		},
+	}
+
+	for _, successCase := range successCases {
+		caddytest.AssertAdapt(t, successCase.rawConfig, "caddyfile", successCase.expectedResponse)
+	}
+}