Merge lp:~paul-lucas/zorba/bug-1002867 into lp:zorba

[Paul J. Lucas]

No longer setting the charset of an overridden media type to a default value.

No longer setting the charset of an overridden media type to a default value.

[Matthias Brantner (matthias-brantner)]

It seems your investigation makes sense. However, I think that the solution should be to not do any transcoding for any non-textual content (even if the user gives one).

review: Needs Fixing

[Paul J. Lucas (paul-lucas)]

If the content is non-textual, then they shouldn't be giving a charset="...". So I don't see the problem.

[Matthias Brantner (matthias-brantner)]

Does you change imply that the default if no charset is given isn't ISO-8859-1 anymore.

review: Needs Information

[Paul J. Lucas (paul-lucas)]

If no override-media-type is given, then the default charset is still ISO 8859-1 (see line 76 in http_response_parser.cpp).

However, if the user gives an override-media-type, then there is no charset unless the user explicitly includes a "; charset=..." as part of the override-media-type; hence, the bytes will pass through unchanged.

I can't think of anything else that's sensible to do.

[Matthias Brantner (matthias-brantner)]

> If no override-media-type is given, then the default charset is still ISO
> 8859-1 (see line 76 in http_response_parser.cpp).
>
> However, if the user gives an override-media-type, then there is no charset
> unless the user explicitly includes a "; charset=..." as part of the override-
> media-type; hence, the bytes will pass through unchanged.
>
> I can't think of anything else that's sensible to do.
If the user overrides the media-type, the behavior should be no different then if the server returned one. That is, if the media type is overwritten to be a text content-type, the default encoding ISO-8859-1 should still be used. Of course, unless otherwise given by the user.

[lp:~paul-lucas/zorba/bug-1002867]

10863. By Paul J. Lucas on 2012-05-22

Now setting default of ISO 8859-1 only for text/ MIME types.

[Matthias Brantner (matthias-brantner)]

review: Approve

[David Graf (davidagraf)]

review: Approve

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/bug-1002867-2012-05-23T13-42-01.572Z/log.html

[Zorba Build Bot (zorba-buildbot)]

Validation queue job bug-1002867-2012-05-23T13-42-01.572Z is finished. The final status was:

All tests succeeded!

=== modified file 'modules/com/zorba-xquery/www/modules/http-client.xq.src/http_response_parser.cpp'
--- modules/com/zorba-xquery/www/modules/http-client.xq.src/http_response_parser.cpp	2012-05-10 04:12:09 +0000
+++ modules/com/zorba-xquery/www/modules/http-client.xq.src/http_response_parser.cpp	2012-05-22 18:26:18 +0000
@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+#include <cstring>
 #include <string>
 #include <sstream>
 #include <assert.h>
@@ -41,8 +42,20 @@
   std::string::size_type pos = s.find( ';' );
   *mime_type = s.substr( 0, pos );
 
-  // The HTTP/1.1 spec says that the default charset is ISO-8859-1.
-  *charset = "ISO-8859-1";
+  if ( std::strncmp( mime_type->c_str(), "text/", 5 ) == 0 ) {
+    //
+    // RFC 2616: "Hypertext Transfer Protocol -- HTTP/1.1," section 3.7.1,
+    // "Canonicalization and Text Defaults":
+    //
+    //    The "charset" parameter is used with some media types to define the
+    //    character set (section 3.4) of the data. When no explicit charset
+    //    parameter is provided by the sender, media subtypes of the "text"
+    //    type are defined to have a default charset value of "ISO-8859-1" when
+    //    received via HTTP.
+    //
+    *charset = "ISO-8859-1";
+  } else
+    charset->clear();
 
   if ( pos != std::string::npos ) {
     //
@@ -74,7 +87,6 @@
   theHandler(aHandler),
   theCurl(aCurl),
   theErrorThrower(aErrorThrower),
-  theCurrentCharset("ISO-8859-1"),      // HTTP/1.1 says this is the default
   theStatus(-1),
   theStreamBuffer(0),
   theInsideRead(false),
@@ -109,7 +121,8 @@
 
       std::auto_ptr<std::istream> lStream;
       try {
-        if ( transcode::is_necessary( theCurrentCharset.c_str() ) ) {
+        if ( !theCurrentCharset.empty() &&
+             transcode::is_necessary( theCurrentCharset.c_str() ) ) {
           lStream.reset(
             new transcode::stream<std::istream>(
               theCurrentCharset.c_str(), theStreamBuffer