Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attaching Echo instance to multiple ports does not work #1785

Closed
maciej-jezierski opened this issue Feb 20, 2021 · 5 comments
Closed

Attaching Echo instance to multiple ports does not work #1785

maciej-jezierski opened this issue Feb 20, 2021 · 5 comments
Labels

Comments

@maciej-jezierski
Copy link

Hi all, I've done the upgrade from 4.1.15 to 4.2.0. I noticed HTTP redirect to HTTPS is running in a cycle and never happens. Also noticed by my website users.

@aldas
Copy link
Contributor

aldas commented Feb 20, 2021

Could you provide example how your Echo is setup and started? Especially how your HTTP server/listener and HTTPS server/listener are setup with Echo instance.

Can you replicate redirect loop with command line tools? I'm asking this because if you are using javascript in frontend your JS implementation could cause that loop. JS in browser decides to direct user to HTTP and Echo (or whatever loadbalancer Nginx/Apache etc you have in front of echo instance) redirects user back to HTTPS from HTTP

@maciej-jezierski
Copy link
Author

maciej-jezierski commented Feb 20, 2021

I don't have Nginx/Apache upfront with the application at this moment.

Set up:

e := echo.New()
renderer := &Template{
		templates: template.Must(template.New("").Funcs(template.FuncMap{
			"safe": func(s string) template.HTMLAttr {
				return template.HTMLAttr(s)
			},
			"meta": func(s api.Tag) template.HTML {
				return template.HTML("<meta " + s.Key + "=\"" + s.Name + "\"  " + s.Type + "=\"" + s.Content + "\" data-vue-router-controlled />")
			},
		}).ParseGlob("web/dist/*.html")),
	}
	e.Renderer = renderer

	e.Pre(middleware.HTTPSRedirect())
	e.Pre(middleware.HTTPSNonWWWRedirect())

	e.Use(middleware.RemoveTrailingSlashWithConfig(middleware.TrailingSlashConfig{
		RedirectCode: http.StatusMovedPermanently,
	}))
	e.Use(middleware.GzipWithConfig(middleware.GzipConfig{
		Level: 2,
	}))

	e.Use(middleware.Recover())
	e.Use(middleware.SecureWithConfig(middleware.SecureConfig{
		ReferrerPolicy: "no-referrer",
	}))

	e.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
		return func(c echo.Context) error {
			if strings.HasSuffix(c.Request().Host, ".webkomiksy.pl") {
				return c.Redirect(http.StatusMovedPermanently, "https://webkomiksy.pl")
			}
			return next(c)
		}
	})
	go func() {
		e.Logger.Fatal(e.Start(":1321"))
	}()
	e.Logger.Fatal(e.StartTLS(":1323", fullchain.pem", "privkey.pem"))

Also, I use prerouting

sudo iptables -t nat -A PREROUTING -p tcp --dport 80 -j REDIRECT --to-port 1321
sudo iptables -t nat -A PREROUTING -p tcp --dport 443 -j REDIRECT --to-port 1323

Call with curl -v to my test server. As you can see the call is not redirected (so actually I was wrong with the loop, it just didn't hit redirect or response)

*   Trying XXX..
* TCP_NODELAY set
* Connected to XXX
> GET / HTTP/1.1
> Host: XXX
> User-Agent: curl/7.68.0
> Accept: */*

A sample response from production after downgrade

* TCP_NODELAY set
* Connected to webkomiksy.pl (51.68.137.171) port 80 (#0)
> GET / HTTP/1.1
> Host: webkomiksy.pl
> User-Agent: curl/7.68.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 301 Moved Permanently
< Location: https://webkomiksy.pl/
< Date: Sat, 20 Feb 2021 22:34:50 GMT
< Content-Length: 0
< 
* Connection #0 to host webkomiksy.pl left intact

@aldas
Copy link
Contributor

aldas commented Feb 20, 2021

@maciej-jezierski problem here is that you are attaching Echo multiple times to same *http.Server instances with e.Start(":1321") and e.StartTLS( calls using different listeners (sockets). This does not work.

If you want to serve on multiple ports you need to use at least e.StartServer() with 2 different http.Server instances or just even create two echo instances (this is probably easier)

For example:

func TestHTTPS_redirect(t *testing.T) {
	createEcho := func() *echo.Echo {
		e := echo.New()
		e.Pre(middleware.HTTPSRedirect())
		e.Pre(middleware.HTTPSNonWWWRedirect())

		e.Use(middleware.RemoveTrailingSlashWithConfig(middleware.TrailingSlashConfig{
			RedirectCode: http.StatusMovedPermanently,
		}))
		e.Use(middleware.GzipWithConfig(middleware.GzipConfig{
			Level: 2,
		}))

		e.Use(middleware.Recover())
		e.Use(middleware.SecureWithConfig(middleware.SecureConfig{
			ReferrerPolicy: "no-referrer",
		}))

		e.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
			return func(c echo.Context) error {
				if strings.HasSuffix(c.Request().Host, ".webkomiksy.pl") {
					return c.Redirect(http.StatusMovedPermanently, "https://webkomiksy.pl")
				}
				return next(c)
			}
		})
		return e
	}
	
	go func() {
		http := createEcho()
		http.Logger.Fatal(http.Start(":1321"))
	}()
	https := createEcho()
	https.Logger.Fatal(https.StartTLS(":1323", "_fixture/certs/cert.pem", "_fixture/certs/key.pem"))
}

@aldas aldas changed the title v4.2.0 HTTP to HTTPS redirect is not working Attaching Echo instance to multiple ports does not work Feb 20, 2021
@maciej-jezierski
Copy link
Author

Thanks, will test it in the next iteration

aldas added a commit to aldas/echo that referenced this issue Feb 27, 2021
…lready TLSListener set to Echo instance. (labstack#1785)

This is problem when user tries to use HTTP (e.Listener) and HTTPS (e.TLSListener) with same Echo instance.
aldas added a commit to aldas/echo that referenced this issue Feb 27, 2021
…lready TLSListener set to Echo instance. (labstack#1785)

This is problem when user tries to use HTTP (e.Listener) and HTTPS (e.TLSListener) with same Echo instance.
lammel pushed a commit that referenced this issue Feb 28, 2021
@aldas aldas added the bug label Mar 1, 2021
@aldas
Copy link
Contributor

aldas commented Mar 1, 2021

Fixed in master with PR #1793. This is now possible without modifications

@aldas aldas closed this as completed Mar 1, 2021
This was referenced Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants