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

Use https in network checks #153

Merged
merged 2 commits into from
Aug 20, 2023
Merged

Conversation

evolutionxbox
Copy link
Contributor

  • Move from icanhasip.com to icanhazip.com (Cloudflare)
  • Use https in network checks using the -k flag
  • Update README.md

Resolves #151

Move from icanhasip.com to icanhazip.com (Cloudflare)
Use https in network checks using the `-k` flag
@actuallymentor
Copy link
Owner

Thanks for the PR @evolutionxbox! The https is not too relevant here (since we're just checking if we're online), but I'm not against switching to https.

Why did you disable verification using -k though?

@earthsound
Copy link

If it's getting switched to https, it'd be better to use -3 since both domains support TLSv1.3.
-k just makes the connection insecure.

@Tokarak
Copy link

Tokarak commented Jul 22, 2023

First of all wget --spider seems optimised for this task. wget isn't packaged by default on MacOS, so it's probably best to stick to curl.

  • -3 uses SSlv3 (insecure, according to the manage) It's better to use default TLS. There isn't really a good reason for -k to be there either.
  • -s is a minor optimisation.
  • I assume the update script cares whether Github works server-side, so -fL should be added there.

@earthsound
Copy link

Yes, meant to say --tlsv1.3

@actuallymentor
Copy link
Owner

@Tokarak yes the intent of those calls is just a naive "are we online" check.

Sounds like removing the -k is the most important bit, the rest doesn't really matter.

It could even just be a curl -I since all I care about is whether we can talk to the url.

@evolutionxbox, could you change the -k to -I (that's an i, not an L) so I can merge this?

@actuallymentor
Copy link
Owner

@evolutionxbox could you change the -k to -I so I can merge? I don't mind doing it myself but want you to have the props on your github profile for your contribution :)

@evolutionxbox
Copy link
Contributor Author

@actuallymentor I didn't mean to ignore the comments. I have updated the PR ✅

@actuallymentor actuallymentor merged commit db8dbf8 into actuallymentor:main Aug 20, 2023
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.

Incorrect URL for liveness check?
4 participants