Player Host header fix

[cavemandaveman]

The Player should only append the port to the Host header if it is not a default port for the requested service.

This is closer to how Networking creates Host headers.

However, this only accounts for HTTP and HTTPS as default ports, while Networking supports more defaults, such as FTP, RTSP, MMS. So this is more of a hot fix, since to mirror how Networking builds request would require more refactoring here, or a longer/uglier if statement.

See this thread for further detail.

[philippe44]

I don't think this test is correct. You probably mean

$host .= ":$port" if $port !~ /(80|443)/;
or
$host .= ":$port" if $port != 80 && $port != 443;

[cavemandaveman]

I was trying to mirror this bit of code as closely as possible. I do not know perl very well, but this worked in a simple test of mine.

Which variant do you think is better? I can update the PR accordingly.

[philippe44]

Well, I would then simply keep the original line as I think the intention is just to not add port when using HTTPS, right? (I've not followed the whole discussion on the forum)

my $host = ($port == 80 || $port == 443) ? $server : "$server:$port";

Still, I think that for correctness we should check 80+http and 443+https, so something like

my $host = (($url =~ /^http:/ && $port == 80) || ($url =~ /^https:/ && $port == 443)) ? $server : "$server:$port";

[michaelherger]

@philippe44's version looks better. I'll do that.

BTW: the comment in the referenced code was even more interesting... https://github.com/Logitech/slimserver/blob/fde5c7622c426708524f5f882c784031c5d02a60/Slim/Networking/Async/HTTP.pm#L247

Host needs port number unless we're using default (http - 80, https - 443).
Note that both curl & wget suppress the default port number. Source comment in wget suggests
that broken server-side software often doesn't recognize the port argument.