Bug 12111 - Uninitialized variable in -[WebDefaultPolicyDelegate webView:decidePolicyForMIMEType:request:frame:decisionListener:]
Summary: Uninitialized variable in -[WebDefaultPolicyDelegate webView:decidePolicyForM...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks: 16700
  Show dependency treegraph
 
Reported: 2007-01-04 06:45 PST by Jim Correia
Modified: 2008-01-01 15:37 PST (History)
3 users (show)

See Also:


Attachments
Patch v1 (1.26 KB, patch)
2007-01-04 23:05 PST, David Kilzer (:ddkilzer)
beidson: review+
Details | Formatted Diff | Diff
Patch v2 (change false to NO) (1.26 KB, patch)
2007-01-04 23:21 PST, David Kilzer (:ddkilzer)
beidson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jim Correia 2007-01-04 06:45:56 PST
-[WebDefaultPolicyDelegate webView:decidePolicyForMIMEType:request:frame:decisionListener:]

has the following code in its implementation:

    if ([[request URL] isFileURL]) {
        BOOL isDirectory;
        [[NSFileManager defaultManager] fileExistsAtPath:[[request URL] path] isDirectory:&isDirectory];
        
        if (isDirectory) {
            [listener ignore];


isDirectory is only filled in by the NSFileManager method in the case that the file exists. In all other cases we are using an uninitialized variable. Initializing the variable is sufficient to solve the problem.

See also rdar://problem/4908081
Comment 1 David Kilzer (:ddkilzer) 2007-01-04 11:05:39 PST
(In reply to comment #0)
> isDirectory is only filled in by the NSFileManager method in the case that the
> file exists. In all other cases we are using an uninitialized variable.
> Initializing the variable is sufficient to solve the problem.

Does the BOOL type not default to 'false' if not explicitly set?
Comment 2 Alexey Proskuryakov 2007-01-04 11:56:27 PST
Confirmed (automatic BOOL variables aren't automatically initialized).

Is this just a potential problem, or there are real life circumstances where the delegate gets called for a non-existent object?
Comment 3 Jim Correia 2007-01-04 14:08:50 PST
Yes, it is a real life problem in my application. (I provided more details in radar, which I realize is only readable by Apple people.)

Essentially I'm using a custom URL protocol to load web archives to avoid the problem where WebKit will load the original resource from the network or filesystem (which may no longer exist.) I'm not rewriting the URLs, only providing data, so the web policy delegate will see the original file url which doesn't point to an existing path.
Comment 4 David Kilzer (:ddkilzer) 2007-01-04 23:05:16 PST
Created attachment 12235 [details]
Patch v1

The obvious fix.
Comment 5 Brady Eidson 2007-01-04 23:11:42 PST
Comment on attachment 12235 [details]
Patch v1

surly you mean "BOOL isDirectory = NO"?
otherwise r+
Comment 6 David Kilzer (:ddkilzer) 2007-01-04 23:21:50 PST
Created attachment 12236 [details]
Patch v2 (change false to NO)

Now with correct-style points!
Comment 7 David Kilzer (:ddkilzer) 2007-01-04 23:38:48 PST
Committed revision 18612.

Comment 8 Jeff Johnson 2008-01-01 14:52:40 PST
This was not a good fix.  -[NSFileManager fileExistsAtPath:isDirectory] may or may not modify the variable isDirectory; that's a private implementation detail of NSFileManager.

What should be done is to check the return value of [[NSFileManager defaultManager] fileExistsAtPath:[[request URL] path] isDirectory:&isDirectory] before using isDirectory.

Comment 9 David Kilzer (:ddkilzer) 2008-01-01 15:09:07 PST
(In reply to comment #8)
> This was not a good fix.  -[NSFileManager fileExistsAtPath:isDirectory] may or
> may not modify the variable isDirectory; that's a private implementation detail
> of NSFileManager.
> 
> What should be done is to check the return value of [[NSFileManager
> defaultManager] fileExistsAtPath:[[request URL] path] isDirectory:&isDirectory]
> before using isDirectory.

Please open a new bug.

Comment 10 David Kilzer (:ddkilzer) 2008-01-01 15:36:37 PST
(In reply to comment #9)
> (In reply to comment #8)
> > This was not a good fix.  -[NSFileManager fileExistsAtPath:isDirectory] may or
> > may not modify the variable isDirectory; that's a private implementation detail
> > of NSFileManager.
> > 
> > What should be done is to check the return value of [[NSFileManager
> > defaultManager] fileExistsAtPath:[[request URL] path] isDirectory:&isDirectory]
> > before using isDirectory.
> 
> Please open a new bug.

Bug 16700.

http://trac.webkit.org/projects/webkit/changeset/18612

http://developer.apple.com/documentation/Cocoa/Reference/Foundation/Classes/NSFileManager_Class/Reference/Reference.html#//apple_ref/occ/instm/NSFileManager/fileExistsAtPath:isDirectory: