raboof commented on code in PR #584:
URL: https://github.com/apache/pekko-http/pull/584#discussion_r1714829003
##########
http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/MarshallingDirectivesSpec.scala:
##########
@@ -275,4 +275,23 @@ class MarshallingDirectivesSpec extends RoutingSpec with
Inside {
}
}
}
+
+ "The marshalling infrastructure for text" should {
+ val foo = "Hällö"
+ "render text with UTF-8 encoding if no `Accept-Charset` request header is
present" in {
+ Get() ~> complete(foo) ~> check {
+ responseEntity shouldEqual HttpEntity(ContentType(`text/plain`,
`UTF-8`), foo)
+ }
+ }
+ "render text with requested encoding if an `Accept-Charset` request header
requests a non-UTF-8 encoding" in {
+ Get() ~> `Accept-Charset`(`ISO-8859-1`) ~> complete(foo) ~> check {
+ responseEntity shouldEqual HttpEntity(ContentType(`text/plain`,
`ISO-8859-1`), foo)
+ }
+ }
+ "render text with UTF-8 encoding if an `Accept-Charset` request header
requests an unknown encoding" in {
+ Get() ~> `Accept-Charset`(HttpCharset("unknown")(Nil)) ~> complete(foo)
~> check {
+ responseEntity shouldEqual HttpEntity(ContentType(`text/plain`,
`UTF-8`), foo)
+ }
+ }
Review Comment:
I realize this is a comment on the current behavior, and not on your change,
but this seems strange to me: it looks like the default string marshaller will
return the string passed to it as `text/plain` claiming it is in whatever
encoding that was requested by the user, without doing anything to make sure
the string is actually in that encoding.
This is only correct if you assume the implementation correctly took into
account the charset, which seems extremely unlikely. I wonder if it wouldn't
make more sense to leave out the optional `charset` parameter to the return
content type in this case. If people have specific requirements (which seems
somewhat unlikely) they can specify the charset explicitly.
This would be a behavior change, but it seems like a reasonable one if we
note it in the release notes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
