Is this a typo?

[philippe44]

Here, https://github.com/Logitech/slimserver/blob/public/8.3/Slim/Web/HTTP.pm#L1000, is this what we want? It does not take into account what $params might want in term of format. Should it be

my $contentType = $params->{'Content-Type'} || $Slim::Music::Info::types{$type};

or, if we are supposed to set $params, at least

$params->{'Content-Type'} ||= $Slim::Music::Info::types{$type};
my $contentType = $params->{'Content-Type'}

It bit me with the log changes I was trying to make because with no extension set in the filename, I could not set content-type in the page handler, it was always ignored. That explains why.

[michaelherger]

This is on the way out. We're setting the Content-Type header for the response here. And we are setting a variable $contentType as a shortcut to be used in the code at the same time. You can do this kind of double assignment in Perl.

[philippe44]

I understand about the double assignment 😄 but maybe it's somewhere else but what I observe is that the content-type set in $reponse of any page handler added by addPageFunction is royally ignored and the file extension (or htm by default) prevails because a few lines later, we do

# setup our defaults
$response->content_type($contentType);

[michaelherger]

Oh, now I understand. Are you using a file extension which would not exist in types.conf or something? Maybe that's the reason why I did add a custom-types.conf in some cases. But I think you're right. If you try to set the content type in the page handler, then this won't work.

Can you check existing code, whether we (in LMS) set the CT in many page handlers? I fear changing the code as you suggest (which makes sense) could potentially break some handlers which currently work by chance/coincidence/or because the dev dealt with it in some other way.

[philippe44]

I'm using some files with no extension or with a .log extension and that where I faced the issue. Now, I surely can add an extension in the URI that is used (like it is now, did that many years ago when I hit that issue but had even less idea of what I was doing - if possible) and that will take care of the issue.

I'll look at the various page handlers in LMS, but I'd assume if we make a change we'd only set the CT in case it's not already set. Failing handlers would then be ones that'd set a wrong CT but expect LMS to overload it... would be very weird

[philippe44]

I've looked at all addPageFunction in LMS and could not find any that sets CT. They are either html or rely on the served link extension (UPnP uses xml a fair bit)

[michaelherger]

I committed this to 8.4 in f58f846. Please give it a try. We can easily back-port it to 8.3 if it turns out to work fine.