Skip to content

Conversation

@webconn
Copy link
Contributor

@webconn webconn commented Sep 20, 2021

This patch allows to use proper UNIX socket URIs such as unix:///var/run/socket.sock. This format is OK with RFC3986 and used in Docker, for example.

Current implementation makes possible only relative paths (like unix://socket.sock) and it looks like it does so by accident.

Example of how it is parsed by url.Parse(): https://play.golang.org/p/OajK5IJX2F9

package main

import (
	"fmt"
	"net/url"
)

func main() {
	val, err := url.Parse("unix:///var/run/socket.sock")
	fmt.Printf("%#v %v", val, err)
}
&url.URL{Scheme:"unix", Opaque:"", User:(*url.Userinfo)(nil), Host:"", Path:"/var/run/socket.sock", RawPath:"", ForceQuery:false, RawQuery:"", Fragment:"", RawFragment:""} <nil>

@MattBrittan
Copy link
Contributor

Unfortunately it will not be possible to merge this until you sign the ECA (see the contributing section of the readme).

This is not a feature I have used (and I don't really know all that much about the UNIX socket interface) but I am concerned that this may break existing uses (e.g. the example you gave unix://socket.sock). I may be wrong but assume whoever added this functionality had a use for it the way its written; would something like the following work (without breaking any existing usage):

var conn net.Conn
var err error
if len(uri.Host) > 0 {
    conn, err = net.DialTimeout("unix", uri.Host, timeout)
else {
    conn, err = net.DialTimeout("unix", uri.Path, timeout)
}

@webconn
Copy link
Contributor Author

webconn commented Sep 21, 2021

Good point, added uri.Host check for compatibility. Also I signed the ECA just now, may require triggering checks.

@MattBrittan
Copy link
Contributor

Looks good to me; thanks for your contribution.

@MattBrittan MattBrittan merged commit 2601804 into eclipse-paho:master Sep 21, 2021
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 this pull request may close these issues.

2 participants