509267 - jar: protocol file listing shows zips as empty

Status: VERIFIED FIXED

(Core :: Networking: JAR, defect)

Description

[Jesse Ruderman]

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).

Comment 1

[Alfred Kayser]

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.

Comment 2

[Alfred Kayser]

Attachment: Patch to fix the directory scanning/matching issue

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.

Comment 3

[Alfred Kayser]

Attachment: V2: Remove tabs

Comment 4

[Alfred Kayser]

This should be blocking for next release, as it is a regression of current functionality and a working patch is available.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Just wondering, what caused this regression?

Comment 6

[Benjamin Smedberg]

Not a serious regression. Also this needs a test.

Comment 7

[(dormant account)]

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.

Comment 8

[Alfred Kayser]

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...

Comment 9

[Alfred Kayser]

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...)

Comment 10

[Alfred Kayser]

Marking fixed by bug 514872.