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

Set language encoding flag when using ZIPPacker #78732

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

timothyqiu
Copy link
Member

@timothyqiu timothyqiu commented Jun 27, 2023

This PR sets the "Language Encoding Flag" when opening a new file in zip. This flag indicates that the filename and comment fields are encoded using UTF-81, so zip readers like ZIPReader can then parse the filename correctly.

Extra parameters passed to zipOpenNewFileInZip4 in this PR are the same as ones zipOpenNewFileInZip will pass to it.

Footnotes

  1. https://pkware.cachefly.net/webdocs/APPNOTE/APPNOTE-6.3.9.TXT

@akien-mga akien-mga added this to the 4.2 milestone Jun 27, 2023
@akien-mga akien-mga added cherrypick:4.0 cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Jun 27, 2023
@zedrun00
Copy link

zedrun00 commented Jun 27, 2023

Yes, the display is normal now, but ZIPReader cannot then parse the filename correctly.

E 0:00:00:0405   test.gd:76 @ read_zip_file(): File does not exist in zip archive: 中���.tscn
  <C++ 错误>       Method/function failed. Returning: PackedByteArray()
  <C++ 源文件>      modules\zip\zip_reader.cpp:92 @ read_file()
  <栈追踪>          test.gd:76 @ read_zip_file()
                 test.gd:8 @ _ready()

t

The problem originates from ZIPReader.get_files()

no_err
err

@timothyqiu
Copy link
Member Author

@zedrun00 Please make sure you are reading the zip file created by ZIPPacker. Your reproduction project attached in the original issue is always reading 7z.zip instead of godot.zip.

@zedrun00
Copy link

@zedrun00 Please make sure you are reading the zip file created by ZIPPacker. Your reproduction project attached in the original issue is always reading 7z.zip instead of godot.zip.

Yes ,this is local code:

func _ready():
	#write_zip_dir("res://zh_CN/", "res://godot.zip")
	read_zip_file("res://godot.zip", "res://test")
	#read_zip_file("res://p.zip", "res://test")

@zedrun00
Copy link

Forgive me for coming late. The bug in v4.1.dev4.official [5c2295ff5] repaired. be related to #69677

@akien-mga
Copy link
Member

So you mean that the bug is already fixed in the current 4.1 version (so also 4.1.rc1)?

But in #78732 (comment) you tested this PR and still had problems, were you testing the patch on top of the 4.0 or 4.1 branch?

@zedrun00
Copy link

zedrun00 commented Jun 28, 2023

So you mean that the bug is already fixed in the current 4.1 version (so also 4.1.rc1)?

But in #78732 (comment) you tested this PR and still had problems, were you testing the patch on top of the 4.0 or 4.1 branch?

#78732 will solve file name garbled. but garbled does not affect ZIPReader::read_file()

ZIPReader::read_file() doesn't work because of "ZIPReader::get_files()" err
pn

#69677 Fixed a memory leak when using long filepaths (>256 characters) inZIPReader::get_files()

@zedrun00
Copy link

Forgive me for my foolish English
The pr fixesed #78726
The pr should be merged.

@AThousandShips AThousandShips modified the milestones: 4.2, 4.3 Nov 1, 2023
@AThousandShips AThousandShips added cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release and removed cherrypick:4.0 labels Nov 1, 2023
@YuriSizov YuriSizov requested a review from a team December 4, 2023 16:43
@akien-mga akien-mga requested a review from bruvzg December 4, 2023 21:37
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

It's working with all archivers I have tested (seems like setting the "version made by" is enough for most software).

There are two places in the editor/export/editor_export_platform.cpp that are using zipOpenNewFileInZip4, these probably should be changed to set bit 11 as well.

When non-ASCII filenames are used, this indicates that the encoding is
UTF-8. Programs like ZIPReader can then parse the filename correctly.
@akien-mga akien-mga merged commit 375d89c into godotengine:master Dec 5, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@YuriSizov YuriSizov removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Dec 5, 2023
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.2.1.

@timothyqiu timothyqiu deleted the zip-cn branch December 5, 2023 15:04
@Kesha2447
Copy link

Will this be fixed in the upcoming releases? It still gets in the way a lot.

@AThousandShips
Copy link
Member

AThousandShips commented Dec 30, 2023

This is in 4.2.1 (it was even mentioned in the release post 🙂) it will also likely be in a future 4.1.x release

@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Jan 23, 2024
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants