Closed Bug 509267 Opened 15 years ago Closed 15 years ago

jar: protocol file listing shows zips as empty

Categories

(Core :: Networking: JAR, defect)

defect
Not set
major

Tracking

()

VERIFIED FIXED

People

(Reporter: jruderman, Assigned: alfredkayser)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

jar:https://bugzilla.mozilla.org/attachment.cgi?id=205693!/ (bug 230606)
jar:https://bugzilla.mozilla.org/attachment.cgi?id=352799!/ (bug 468211)

These appear empty.  They should let me see the contents of the zips (but not run scripts in them).
Or more simply on a local file on your disk, such as:
jar:file:///c:/tmp/uw-downloads.zip!/

This does work in 3.5, but no longer in current nightlies.
I am hacking libjar for some other bugs and stumble upon this one,
and tried to find the cause, but it seems the wildcard matching doesn't work anymore. 

There may be a simpler solution to get filter the entries.
OS: Mac OS X → All
Hardware: x86 → All
Instead of using complex wildcard matching, which fails, and thus causes this 'empty listing', use more simple matching logic, which also removes the need for escaping the directory name for the wildcard thing.
Assignee: nobody → alfredkayser
Attachment #394782 - Flags: review?(benjamin)
Attached patch V2: Remove tabsSplinter Review
Attachment #394782 - Attachment is obsolete: true
Attachment #394783 - Flags: review?(benjamin)
Attachment #394782 - Flags: review?(benjamin)
This should be blocking for next release, as it is a regression of current functionality and a working patch is available.
Status: NEW → ASSIGNED
Flags: blocking1.9.2?
Just wondering, what caused this regression?
Not a serious regression. Also this needs a test.
Flags: in-testsuite?
Flags: blocking1.9.2?
Flags: blocking1.9.2-
Attachment #394783 - Flags: review?(benjamin) → review?(tglek)
Comment on attachment 394783 [details] [diff] [review]
V2: Remove tabs

>diff --git a/modules/libjar/nsJARInputStream.cpp b/modules/libjar/nsJARInputStream.cpp
>--- a/modules/libjar/nsJARInputStream.cpp
>+++ b/modules/libjar/nsJARInputStream.cpp
>@@ -110,57 +110,33 @@ nsJARInputStream::InitDirectory(nsJAR* a
> 
>     // Mark it as closed, in case something fails in initialisation
>     mClosed = PR_TRUE;
>     mDirectory = PR_TRUE;
>     
>     // Keep the zipReader for getting the actual zipItems
>     mJar = aJar;
>     nsZipFind *find;
>-    nsresult rv;
>-    // We can get aDir's contents as strings via FindEntries
>-    // with the following pattern (see nsIZipReader.findEntries docs)
>-    // assuming dirName is properly escaped:
>-    //
>-    //   dirName + "?*~" + dirName + "?*/?*"
>-    nsDependentCString dirName(aDir);
>-    mNameLen = dirName.Length();
> 
>-    // iterate through dirName and copy it to escDirName, escaping chars
>-    // which are special at the "top" level of the regexp so FindEntries
>-    // works correctly
>-    nsCAutoString escDirName;
>-    const char* curr = dirName.BeginReading();
>-    const char* end  = dirName.EndReading();
>-    while (curr != end) {
>-        switch (*curr) {
>-            case '*':
>-            case '?':
>-            case '$':
>-            case '[':
>-            case ']':
>-            case '^':
>-            case '~':
>-            case '(':
>-            case ')':
>-            case '\\':
>-                escDirName.Append('\\');
>-                // fall through
>-            default:
>-                escDirName.Append(*curr);
>-        }
>-        ++curr;
>-    }

1a) I have no idea what this was trying to accomplish
1b) why did you remove it
2) this patch needs a testcase and will need to be updated to reflect changes made by your other patches.
The 'old' code tried to convert the directory part of a jar spec, such as:
the Sellaband/ of jar:file:///c:/tmp/uw-downloads.zip!/Sellaband/
to a wildcard pattern to select all entries that start with Sellaband/ but are not in subdirectories thereof.
'Sellaband/?*' to select all that match 'Sellaband/'
and then ~Sellaband/*?/*?' to filter out any subdirectory entries.

But, the dirname itself needs to be escaped to prevent problems with wildcard characters in the dirname itself (such as ~).

Overall, this pattern doesn't seem to work anymore with nsWildcard.
(so that should be a bug).

But, a more simple solution would be to just a memcmp(dirname,  name, dirnamelen) to check for the directory, and then to check that it is not a subdirectory entry (by testing for '/' following by non-zero). This is much simpler, and much faster than the Wildcard code, and not dependent to the fuzzy logic of Wildcard (see the comments of that code).


As for the test, indeed a good test is needed here... 
I am thinking about how to tackle that...
Blocks: 511736
The original issue (in nsWildcard) has been fixed:
Bug 514872 - nsWildCard ~ regexp was broken. r=jwalden+bmo (Jeff Walden).

Attached patch is still valid in terms of making this code simpler and safer (no longer dependent on nsWildcard, which performs lots of copies and compares...)
Marking fixed by bug 514872.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Attachment #394783 - Flags: review?(tglek)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.