Bugzilla – Full Text Bug Listing
| Summary: | [archives] RSEF1301 when opening file in a ZIP archive containing backslash separators | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] Target Management | Reporter: | Krzysztof Kosmatka <krzysztof.kosmatka+eclipse> | ||||||||
| Component: | RSE | Assignee: | Martin Oberhuber <mober.at+eclipse> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | Martin Oberhuber <mober.at+eclipse> | ||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | ||||||||||
| Version: | 3.1.2 | ||||||||||
| Target Milestone: | 3.3 M4 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows XP | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Krzysztof Kosmatka
Created attachment 183743 [details]
Zip file causing the problem
Problem exists, when I try to open file img/error.gif from this zip file (from Remote Systems Explorer view)
I reproduced the problem, and I assume the issue is that your file is stored in the Zip Archive with a backslash whereas most zip files that I know of use forward slashes as path separators. What program did you use to create the zip archive?
C:\Documents and Settings\mober\My Documents\Downloads>unzip -v example.zip
Archive: example.zip
Length Method Size Ratio Date Time CRC-32 Name
-------- ------ ------- ----- ---- ---- ------ ----
9483 Defl:N 8812 7% 05.10.10 07:51 47af1022 chart.png
339 Defl:N 267 21% 05.10.10 07:51 c2e22190 img\error.gif
-------- ------- --- -------
9822 9079 8% 2 files
Looks like this is a known issue with the Java Zipfile#getEntry() method: http://java.itags.org/java-essentials/81044/ Possible workarounds in our code, from easiest to hardest: (a) When getEntry(foo) fails, fall back to trying getEntry(foo.replace('/','\\')) (b) Use getEntries() and iterate over all entries, with a pattern matcher that's less strict (c) When originally drilling down into the Zipfile, remember the path separator chars used I lean towards (a) since it's easiest to implement and the "\" character seems to be not widely used. The method will fail in case any path uses a mixture of \ and / characters as path separators: we require all / or all \ characters. It should still work properly on UNIX, where the \ character is a legal char in a file name, because we always try / separator chars first. In case method (a) should not be good enough, we could still implement a 2nd fallback to (b) just in case. Krzystof, the code to change is SystemZipHandler#safeGetEntry(String), do you want to contribute a patch? (In reply to comment #2) > What program did you use to create the zip > archive? I'm not 100% sure, but I think that it was created by Eclipse Memory Analyzer. Your (a) solution works. I'll contribute a patch. Created attachment 183823 [details]
Patch solving the problem
The patch contains solution (a)
Created attachment 183824 [details]
corrected patch
Corrected patch. The previous patch accidently contains a System.out.println
Thanks! So, the leading '\' character doesn't cause issues? That's slightly surprising because the archive directory doesn't show leading '\' characters. Also, just making sure that you actually have the right to contribute, ie your employer authorizes contributing this under EPL? Please add a statement here on bugzilla according to http://www.eclipse.org/dsdp/tm/development/committer_howto.php#external_contrib , thanks! Legal Message: I, Krzysztof Kosmatka, declare that I developed attached code from scratch, without referencing any 3rd party materials except material licensed under the EPL. I am authorized by my employer to make this contribution under the EPL. (In reply to comment #8) > Thanks! So, the leading '\' character doesn't cause issues? That's slightly > surprising because the archive directory doesn't show leading '\' characters. > No, the entry name passed to SystemZipHandler#safeGetEntry(String) doesn't contain leading '\' nor '/'. And since I call replace on this name (not on name with added leading '/'), the entry is found. Comment on attachment 183824 [details]
corrected patch
Patch applied with a slight modification: We need to use String.replace('/','\\') in order to run on J2SE-1.4. The method replacing Strings rather than chars has only been introduced in J2SE-1.5.
Released for 3.3M4. Krzysztof, I would like to add your name to the list of contributors. I also need to know who owns the Copyright (is it you personally, or the company that you work for - then I'd like to know the company name). Since I've made the patch during my work hours, I think that it is my company who owns the copyright. I'll send you an e-mail containing all required information (according to http://www.eclipse.org/dsdp/tm/development/committer_howto.php#external_contrib ) |