-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Conversation
the weird url-but-not-actually-a-url-string doesn't handle IPv6 addresses without extra quoting. Building a string which you are about to parse again seems like a weird choice. Let's just use listenTCP, which is consistent with what we do elsewhere.
make it look a bit more consistent with everything else, and tweak the defaults to listen on port 80.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable, other than the defaults causing warnings on linux and macOS
self.acme_port = acme_config.get("port", 8449) | ||
self.acme_bind_addresses = acme_config.get("bind_addresses", ["127.0.0.1"]) | ||
self.acme_port = acme_config.get("port", 80) | ||
self.acme_bind_addresses = acme_config.get("bind_addresses", ['::', '0.0.0.0']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels odd that the default will cause warnings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, mostly I wanted it to be consistent with what we do for the other listeners. I kinda agree, but :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair enough, we can always clean it up later if necessary
Codecov Report
@@ Coverage Diff @@
## develop #4525 +/- ##
===========================================
+ Coverage 74.74% 74.75% +<.01%
===========================================
Files 337 337
Lines 34447 34456 +9
Branches 5615 5616 +1
===========================================
+ Hits 25749 25758 +9
+ Misses 7110 7108 -2
- Partials 1588 1590 +2 |
Handle listening for ACME requests on IPv6 addresses:
Make the default config look a bit more consistent with everything else, and
tweak the defaults to listen on port 80.