Opened 5 years ago

Closed 5 years ago

#14451 closed defect (implemented)

Use "socket:/path/to/socket" syntax everywhere for AF_UNIX ports

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: 0.2.6.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

With #11485, Andrea did a cool syntax for AF_UNIX hidden-service address targets.

Why didn't all the other FooPort options think of that?

We shouldn't have a separate option for Unix vs IP address as in:

SocksPort 127.0.0.1:9050
SocksSocket /home/nickm/.tor/connect-to-this

Instead why not just have it be:

SocksPort 127.0.0.1:9050
SocksPort socket:/home/nickm/.tor/connect-to-this

Child Tickets

Change History (7)

comment:1 Changed 5 years ago by dgoulet

"socket" is a bit broad, UDP, Unix, or any of the 64 others network stack of the kernel? :)

What about this?

SocksPort unix:/home/nickm/.tor/connect-to-this

comment:2 Changed 5 years ago by dgoulet

Milestone: Tor: 0.2.???Tor: 0.2.6.x-final
Status: newneeds_review

Ok, I lost control and implemented this :). There are two commits, one to implement the support and the other one to make hidden service use the same function to parse the port.

Let me know if that makes sense, see branch bug14451_026_v1.

FYI, we can litterally remove SocksSocket option with that and we should think about it before releasing 0.2.6 or else we end up supporting that option when SocksPort works with Unix socket...

comment:3 Changed 5 years ago by andrea

This branch looks good to me.

comment:4 Changed 5 years ago by nickm

I ran into an assertion failure when I did ORPort unix:/home/nickm/foo , so I fixed it by forbidding anything besides SocksPort and ControlPort.

I also removed SocksSocket and added documentation.

I've updated some new commits in my bug14451_026_v1 branch. Have a look at those?

comment:5 Changed 5 years ago by dgoulet

Looks good! ACK.

comment:6 Changed 5 years ago by nickm

Merged; thanks!

comment:7 Changed 5 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed
Note: See TracTickets for help on using tickets.