code review 54360043: archive/zip: new zip using utf8 filename.

archive/zip: new zip using utf8 filename as default.

[chai2010, 2014-01-20 01:53:07]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go/

[bradfitz, 2014-01-20 06:49:32]

What if the filename isn't UTF-8?  We never enforced that.  Why
unconditionally advertise/promise that?



On Sun, Jan 19, 2014 at 5:53 PM, <chaishushan@gmail.com> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> archive/zip: new zip using utf8 filename.
>
> Please review this at https://codereview.appspot.com/54360043/
>
> Affected files (+2, -1 lines):
>   M src/pkg/archive/zip/writer.go
>
>
> Index: src/pkg/archive/zip/writer.go
> ===================================================================
> --- a/src/pkg/archive/zip/writer.go
> +++ b/src/pkg/archive/zip/writer.go
> @@ -187,7 +187,8 @@
>                 }
>         }
>
> -       fh.Flags |= 0x8 // we will write a data descriptor
> +       fh.Flags |= 0x8       // we will write a data descriptor
> +       fh.Flags |= (1 << 11) // using UTF-8 for filename and comment
>
>         fh.CreatorVersion = fh.CreatorVersion&0xff00 | zipVersion20 //
> preserve compatibility byte
>         fh.ReaderVersion = zipVersion20
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>

[chai2010, 2014-01-20 08:11:10]

If the filename contains chinese charactors, the local zip tools (7z/winrar)
will decode the filename as GBK string on chinese windows [1].

I think the archive/zip.Writer.Create's name is utf8 string,
so we need set UTF-8 flag as default.

If we want create a GBK filename, we can use
archive/zip.Writer.CreateHeader,
and then modify the archive/zip.FileHeader's flag.

[1]:
Unfortunately, the chinese windows7 will always ingore the utf8 flag.
It only support GBK filename on chinese windows.



2014/1/20 Brad Fitzpatrick <bradfitz@golang.org>

> What if the filename isn't UTF-8?  We never enforced that.  Why
> unconditionally advertise/promise that?
>
>
>
> On Sun, Jan 19, 2014 at 5:53 PM, <chaishushan@gmail.com> wrote:
>
>> Reviewers: golang-codereviews,
>>
>> Message:
>> Hello golang-codereviews@googlegroups.com,
>>
>> I'd like you to review this change to
>> https://code.google.com/p/go/
>>
>>
>> Description:
>> archive/zip: new zip using utf8 filename.
>>
>> Please review this at https://codereview.appspot.com/54360043/
>>
>> Affected files (+2, -1 lines):
>>   M src/pkg/archive/zip/writer.go
>>
>>
>> Index: src/pkg/archive/zip/writer.go
>> ===================================================================
>> --- a/src/pkg/archive/zip/writer.go
>> +++ b/src/pkg/archive/zip/writer.go
>> @@ -187,7 +187,8 @@
>>                 }
>>         }
>>
>> -       fh.Flags |= 0x8 // we will write a data descriptor
>> +       fh.Flags |= 0x8       // we will write a data descriptor
>> +       fh.Flags |= (1 << 11) // using UTF-8 for filename and comment
>>
>>         fh.CreatorVersion = fh.CreatorVersion&0xff00 | zipVersion20 //
>> preserve compatibility byte
>>         fh.ReaderVersion = zipVersion20
>>
>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "golang-codereviews" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to golang-codereviews+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/groups/opt_out.
>>
>
>


-- 
chaishushan
http://my.oschina.net/chai2010
https://bitbucket.org/chai2010

[gobot, 2014-01-22 00:12:29]

R=adg@golang.org (assigned by bradfitz@golang.org)

[adg, 2014-01-28 03:26:46]

A Go string is not necessarily utf-8. See: http://blog.golang.org/strings

I don't think we can make this change anyway, because of the compatibility
promise. Who knows what code this might break.

[dave_cheney.net, 2014-01-31 00:03:46]

On 2014/01/28 03:26:46, adg wrote:
> A Go string is not necessarily utf-8. See: http://blog.golang.org/strings
> 
> I don't think we can make this change anyway, because of the compatibility
> promise. Who knows what code this might break.

not lgtm.

Please start by raising an issue, golang.org/issue/new, laying out the problem.

R=close