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

Add processing of Accept header to AudioServlet #2960

Merged
merged 2 commits into from
May 16, 2022

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented May 13, 2022

Closes #1350

This checks the Accept header of the incoming request and adjusts the content-type (if possible). This is the best we can do to support clients that need a non-standard MIME type.

Signed-off-by: Jan N. Klug github@klug.nrw

Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K J-N-K added the enhancement An enhancement or new feature of the Core label May 13, 2022
@J-N-K J-N-K requested a review from a team as a code owner May 13, 2022 19:20
@lolodomo
Copy link
Contributor

lolodomo commented May 15, 2022

Code looks good to me.

What could have be done too is to add an additional serve method with a new parameter to provide the MINE type to consider when the audio servlet is returning data. Like that, the squeezebox binding in its audio sink could have set it here:
https://github.com/openhab/openhab-addons/blob/da59cdd255a66275dd7ae11dd294fedca4942d30/bundles/org.openhab.binding.squeezebox/src/main/java/org/openhab/binding/squeezebox/internal/SqueezeBoxAudioSink.java#L96
Edit: to clarify, it would give the opportunity to a binding to order the openHAB audio server what MINE type to return for a particular file it serves.
WDYT ?

@J-N-K
Copy link
Member Author

J-N-K commented May 15, 2022

Why would one want to do that? The possible MIME types can be determined from Accept header (client side) and the stream.getFormat (server-side). Maybe a mis-match between those two (i.e. the determined MIME type is not present in the Accept-header should result in a 406 error.

@lolodomo
Copy link
Contributor

You're probably right.
I was thinking about the strange case where the client was not providing any Accept header. I don't know if the software of the Squeezebox is providing one.

Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks!

@kaikreuzer kaikreuzer merged commit d4a68c8 into openhab:main May 16, 2022
@kaikreuzer kaikreuzer added this to the 3.3 milestone May 16, 2022
@J-N-K J-N-K deleted the feature-acceptheaderswav branch May 16, 2022 20:32
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 12, 2023
Signed-off-by: Jan N. Klug <github@klug.nrw>
GitOrigin-RevId: d4a68c8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature of the Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discussion: AudioServlet and wav files
3 participants