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

Unable to use domain name with hyphens #1047

Closed
4 tasks done
sheehanisageek opened this issue May 14, 2020 · 2 comments
Closed
4 tasks done

Unable to use domain name with hyphens #1047

sheehanisageek opened this issue May 14, 2020 · 2 comments
Labels
Merged Fixed and merged into Maste rbranch

Comments

@sheehanisageek
Copy link

In raising this issue, I confirm the following:

  • I have read and understood the contributors guide.
  • The issue I am reporting can be replicated.
  • The issue I am reporting can be is directly related to the pivpn installer script.
  • The issue I am reporting isn't a duplicate (see FAQs, closed issues, and open issues).

Describe the bug
The validDomain function does not permit/validate domains with hyphens.

To Reproduce
Steps to reproduce the behavior:

  1. Run installer
  2. Select "Use a public DNS" when prompted "Will clients use a Public IP or DNS Name to connect to your server (press space to select)?"
  3. Type in any DNS name that includes a hyphen
  4. See error

Expected behavior
The validDomain function should permit/validate domains with hyphens.

@sheehanisageek
Copy link
Author

sheehanisageek commented May 14, 2020

Example domain name would be "this.is-a-test.net".

Existing expression does not match domains like the above. Here is the expression I used as a workaround.

(?=^.{4,253}$)(^(?:[a-zA-Z0-9](?:(?:[a-zA-Z0-9\-]){0,61}[a-zA-Z0-9])?\.)+[a-zA-Z]{2,}$)

Applicable code around line 1548 of the installer. I replaced the if statement with the following PCRE interpreted grep.

echo $domain | grep -P '(?=^.{4,253}$)(^(?:[a-zA-Z0-9](?:(?:[a-zA-Z0-9\-]){0,61}[a-zA-Z0-9])?\.)+[a-zA-Z]{2,}$)' &> /dev/null

@orazioedoardo
Copy link
Member

I've applied your suggestion in the test branch (using Bash's here strings to avoid a pipe).

@orazioedoardo orazioedoardo added Merged Fixed and merged into Maste rbranch and removed Fix in Test Branch labels Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merged Fixed and merged into Maste rbranch
Projects
None yet
Development

No branches or pull requests

2 participants