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

adresss does not support non fdqn domain #18

Closed
nsteinmetz opened this issue Feb 6, 2018 · 5 comments · Fixed by #19
Closed

adresss does not support non fdqn domain #18

nsteinmetz opened this issue Feb 6, 2018 · 5 comments · Fixed by #19

Comments

@nsteinmetz
Copy link
Contributor

Hi,

Testing withing docker, I was to do:

ENTRYPOINT ["/usr/local/bin/waitforit","-address=http://backend:8182/backend/tunnel/tunnel.nocache.js", "-timeout=20","--", "nginx", "-c", "/etc/nginx/nginx.conf", "-g", "daemon off;"]

but waitforit reports:

2018/02/06 12:16:21 Invalid connection main.Config{Host:"", Port:80, Address:"http://backend:8182/backend/tunnel/tunnel.nocache.js", Timeout:10, Retry:500}

Would it be possible to have a more flexible approach to valide domains ?

Thanks,
Nicolas

@nsteinmetz
Copy link
Contributor Author

nsteinmetz commented Feb 6, 2018

Ok, issues seems with regexAddressConn:

Following patern works better:

^([a-z]{3,}):\/\/([a-zA-Z0-9\.\-_]+):?([0-9]*)*\/([a-zA-Z0-9\/\.\-_\(\)?=&#%]*)*$

or below if you prefer to have the initial / in the path

^([a-z]{3,}):\/\/([a-zA-Z0-9\.\-_]+):?([0-9]*)*([a-zA-Z0-9\/\.\-_\(\)?=&#%]*)*$
  • https://test.domaine.tld:8080/path/to/file.jswill lead to 4 groups: sheme, host, port, path
  • https://test.domaine.tld/path/to/file.js would lead to sheme, host, empty, path
  • tcp://google.com:90 woud lead to protocol, host, port + emply group
  • http://10.10.10.10/ap_po would lead to protocol, host, empty, path

Tested with some samples from https://mathiasbynens.be/demo/url-regex - it should work for most of the basic url we can have (except unicode signs, non occidental letters)

Would it be ok for you ?

I'm not a golang dev, so I don't make a PR

@nsteinmetz
Copy link
Contributor Author

nsteinmetz commented Feb 6, 2018

Ok, so learning by doing, I would propose something like below, feel free to comment :)

package main

import (
	"regexp"
	"strconv"
)

const regexAddressConn string = `^([a-z]{3,}):\/\/([a-zA-Z0-9\.\-_]+):?([0-9]*)*([a-zA-Z0-9\/\.\-_\(\)?=&#%]*)*$`

// Connection data
type Connection struct {
	Type   string
	Scheme string
	Port   int
	Host   string
	Path   string
}

// BuildConn build a connection structure.
// This connection data can later be used as a common structure
// by the functions that will check if the target is available.
func BuildConn(cfg *Config) *Connection {
	if cfg.Host != "" {
		return &Connection{Type: "tcp", Host: cfg.Host, Port: cfg.Port}
	}

	address := cfg.Address
	if address == "" {
		return nil
	}

	match := regexp.MustCompile(regexAddressConn).FindAllStringSubmatch(address, -1)
	if len(match) < 1 {
		return nil
	}

	res := match[0]
	conn := &Connection{
		Type: res[1],
		Host: res[2],
		Path: res[4],
	}

	if conn.Type != "tcp" {
		conn.Scheme = conn.Type
		conn.Type = "tcp"
	}

	// resolve port
	if len(res[3]) == 0 {
		if conn.Scheme == "https" {
			conn.Port = 443
		} else {
			conn.Port = 80
		}
	} else {
		if port, err := strconv.Atoi(res[3]); err == nil {
			conn.Port = port
		}
	}

	return conn
}

I may still miss something as output remains the same :(

I build a side code with the code I modified and when I print conn, I have:

For URL: http://backend:8081/path/to/some/file.ext

&{tcp http 8081 backend /path/to/some/file.ext}

So I don't understand why it fails at the program level :(

@nsteinmetz
Copy link
Contributor Author

Hmm when I put all the code in one single file, it works as expected 🙄

However, when I do in your code folder :

go build main.go
# command-line-arguments
./main.go:85:12: undefined: DialConfigs

So maybe the issue is here ?

@nsteinmetz
Copy link
Contributor Author

Ok forget it ; a simple go build make it work as expected and binary seems good at last.

@killmenot
Copy link
Contributor

@maxcnunes Hi, could you please merge #19?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants