Hello 馃憢
Invalid Http/Https binding can be defaulted without any log and go on
After few long minutes (hours?) of debugging we found out that invalid port configuration will not say anything and silent.
Why did it takes so long ?
Because we were looking for the https binding that was https://*:8443 on our POD, and it seems that another appsettings...json file still had an http://*:{place.holder} that was not replaced
So it seems that it defaulted to port 80 without saying anything at all.
The behavior was AccessDeniedException on a Socket binding attempt (because Pods should not run as root so any binding bellow 1024 is forbidden)
so I wonder about 3 things now :
dotnet new webapi
dotnet publish
cd ./bin/Debug/net5.0/publish/
$env:ASPNETCORE_URLS="https://*:{abc}"
./foo.exe

dotnet --infoThis appears to be a problem with BindingAdress.Parse which gets called early in the AddressBinder. Since the binder is not able to produce a better result from the input, it will eventually fall back to just binding on the port that the BindingAdress contains:
Unfortunately, BindingAddress.Parse does not appear to parse the input very well:

Calling BindingAddress.Parse("https://*:{abc}") will recognize *:{abc} as the host and assume the default HTTPS port 443.
Digging a bit deeper, it seems that this part of BindingAddress.Parse is responsible:
portString is correctly determined as "{abc}" but then there is an attempt to parse it which will obviously fail. Since there is no port, the default scheme port will be used and the :{abc} will be understood as part of the host.
To be honest, I don鈥檛 really understand why the logic was done like this. I would expect the parsing to fail when the identified portString does not end up being a number. After all, it existing means that there is a colon before, so the original string is a:b where b is not a number. I only see two options where a colon would be valid as part of the authority of an URI though:
@ before the host name. And for the address binder, this wouldn鈥檛 really make any sense.While supporting IPv6 addresses is obviously required in some way, I would expect the parsing to actually fail with some kind of error instead of blindly accepting the :{abc} here. Maybe we can fine tune this just a little bit so inputs like these would throw an error.
Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.
At a minimum we should log a warning.
yeah as you said Warning would be the minimal ;)
I honestly think this should throw (would it be considered a breaking change given the fact that it maybe should have thrown from day 0 ?)
especially since this is an explicit value being realllly wrong and not "a defaulted" value, like "nothing was supplied so we defaulted"
Most helpful comment
At a minimum we should log a warning.