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

Give an error for addresses lacking a hostname #408

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gperciva
Copy link
Member

@gperciva gperciva commented Aug 4, 2024

A source or destination address of the form ":1234" is parsed as a hostname of "" which is guaranteed to never work. While we don't want to give special error messages for all possible invalid addresses, this particular case can easily be produced from a broken shell script (with a hostname variable accidentally not set), so it's worth having a special error message.

Suggested by: Ross Richardson

@gperciva
Copy link
Member Author

gperciva commented Aug 4, 2024

This is an alternate solution to #407. (The commit message was tweaked accordingly.)

@gperciva
Copy link
Member Author

gperciva commented Aug 7, 2024

Examples of message:

$ ./spipe/spipe -t ":1234" -k /dev/null
spipe: No hostname in ":1234"
spipe: Error resolving socket address: :1234
$ ./spipe/spipe -t :1234 -k /dev/null
spipe: No hostname in ":1234"
spipe: Error resolving socket address: :1234

@cperciva
Copy link
Member

cperciva commented Aug 8, 2024

Ok, this adds a more helpful error message, but doesn't solve the problem that spiped -e -s '[127.0.0.1]:12345' -t ':12346' -k /COPYRIGHT -D daemonizes before it performs a DNS lookup and then spews error messages forever.

Can you write a sock_validate function which checks if an address is syntactically valid (i.e. if it might work) without actually doing the address resolution? Then we can call that from spiped.

@gperciva
Copy link
Member Author

gperciva commented Aug 8, 2024

Oh! Oops, ok, I get it now. Will do.

@gperciva gperciva force-pushed the no-hostname-sock branch 2 times, most recently from 0f59685 to a36347f Compare August 8, 2024 02:58
@gperciva
Copy link
Member Author

gperciva commented Aug 8, 2024

Updated.

I also added a check for opt_b.

A source or destination address of the form ":1234" is parsed as a
hostname of "" which is guaranteed to never work.  While we don't want
to give special error messages for all possible invalid addresses, this
particular case can easily be produced from a broken shell script (with
a hostname variable accidentally not set), so it's worth having a
special error message.

Suggested by:	Ross Richardson
We want to check that the addresses are syntactically valid before
spiped daemonizes.

It's not strictly necessary for spipe, but we might as well check it so
that we can produce a usage() if necessary.

Suggested by:	Ross Richardson
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants