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

Validate ping url #1662

Merged
merged 1 commit into from
Aug 12, 2024
Merged

Conversation

nandi95
Copy link
Contributor

@nandi95 nandi95 commented Aug 11, 2024

📃 Description

I was trying to set up the ping url but I included the protocol which seems to not work with thie ping package. I added a little validation and removal of the protocol to remove this issue for others...

I'm not sure where the documentation should be updated to add this new error here: https://docs.speedtest-tracker.dev/help/error-messages

✏️ Changed

  • added ping url validation
  • removed ping url protocol

@alexjustesen alexjustesen added the 🎉 feature New feature or request label Aug 12, 2024
@alexjustesen alexjustesen merged commit c8bb0c0 into alexjustesen:main Aug 12, 2024
1 check passed
@nandi95
Copy link
Contributor Author

nandi95 commented Aug 12, 2024

@alexjustesen I just realised this will fail ip addresses, that could prove to be an issue for people pinging dns like 1.1.1.1 so perhaps don't release just yet. I'll send a patch

@alexjustesen
Copy link
Owner

I was actually going to create a separate function for myself tonight for IP addresses so if you get to it first that works too!

@nandi95
Copy link
Contributor Author

nandi95 commented Aug 12, 2024

I think Str::isUrl could be usefult here, it's used by the laravel validator too and handles ip addresses. You can define a list of accepted protocols too that should be accepted (and later stripped). I'll let you do it as the latter needs some more thought.

Alternatively the built in filter_var with with FILTER_VALIDATE_IP

@nandi95 nandi95 deleted the validate-ping-url branch August 12, 2024 13:37
@alexjustesen
Copy link
Owner

I think we can use isUrl for validating the URL string. Looks like to validate the url or an IP address a protocol is required http or https which then looks like it validates as a URL instead.

// URL validation
Str::isUrl('speedtest-tracker.dev'); // false
Str::isUrl('https://speedtest-tracker.dev'); // true

// IP address validation
Str::isUrl('1.1.1.1'); // false
Str::isUrl('https://1.1.1.1'); // true

Might be better to use filter_var() and validate the IP address so a protocol isn't required, example:

filter_var('1.1.1.1', FILTER_VALIDATE_IP, FILTER_FLAG_IPV4 || FILTER_FLAG_IPV6); // true

@nandi95 nandi95 mentioned this pull request Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants