Code review - Issue 54360043: code review 54360043: archive/zip: new zip using utf8 filename.https://codereview.appspot.com/2014-01-31T00:03:46+00:00rietveld
Message from unknown
2014-01-20T01:49:17+00:00chai2010urn:md5:5ab9f417249c3539d5c697eb27aa9d03
Message from unknown
2014-01-20T01:52:25+00:00chai2010urn:md5:d14bb7c13076aeb4ad2f9a1bb7039079
Message from unknown
2014-01-20T01:52:56+00:00chai2010urn:md5:f71d1484b0c1e5c19e439d9a99d0a8b5
Message from chaishushan@gmail.com
2014-01-20T01:53:07+00:00chai2010urn:md5:ad08d8ab63247c610f07ee1a7698cada
Hello golang-codereviews@googlegroups.com,
I'd like you to review this change to
https://code.google.com/p/go/
Message from unknown
2014-01-20T02:33:10+00:00chai2010urn:md5:1faecfae5ac71febf5fe0f609f7ea2ff
Message from bradfitz@golang.org
2014-01-20T06:49:32+00:00bradfitzurn:md5:c4f258992d1708196d1319b5b23c8c96
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.
>
Message from chaishushan@gmail.com
2014-01-20T08:11:10+00:00chai2010urn:md5:6cfc7f1ea10b299136b0e7c86df43d60
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
Message from gobot@golang.org
2014-01-22T00:12:29+00:00goboturn:md5:3c161c82197b713f9fcca402bce49741
R=adg@golang.org (assigned by bradfitz@golang.org)
Message from adg@golang.org
2014-01-28T03:26:46+00:00adgurn:md5:fe26168c7044423f8b5dbb01e758c060
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.
Message from dave@cheney.net
2014-01-31T00:03:46+00:00dfcurn:md5:0a1c05b47afdc26730adc2db700d8fee
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