Bug 148423

Summary: [WebKit2] add some protection code at IndexedDB
Product: WebKit Reporter: Jincheol Jo <jincheol.jo>
Component: WebKit2Assignee: Jincheol Jo <jincheol.jo>
Status: RESOLVED WONTFIX    
Severity: Normal CC: andersca, beidson, bfulgham, gyuyoung.kim, ossy, sam, sihui_liu
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch mcatanzaro: review-

Description Jincheol Jo 2015-08-25 02:19:34 PDT 
Metadata pointer seems to need a null protection code.
Its member is accessed in next line.
I guess it can trigger crash.
Comment 1 Jincheol Jo 2015-08-25 02:24:32 PDT 
Created attachment 259832 [details]
Patch
Comment 2 Brady Eidson 2015-08-25 21:49:23 PDT 
Are you just offering this patch because of code inspection, or do you actually have a reproducible case of the null deref crash?
Comment 3 Jincheol Jo 2015-08-26 02:51:40 PDT 
(In reply to comment #2)
> Are you just offering this patch because of code inspection, or do you
> actually have a reproducible case of the null deref crash?

When I blocked all function body code of KeyedEncoder and KeyedDecoder for my test, it triggered crash in that line.
Comment 4 Brady Eidson 2015-08-26 09:08:24 PDT 
(In reply to comment #3)
> (In reply to comment #2)
> > Are you just offering this patch because of code inspection, or do you
> > actually have a reproducible case of the null deref crash?
> 
> When I blocked all function body code of KeyedEncoder and KeyedDecoder for
> my test, it triggered crash in that line.

For what test? This patch has no test.

And what do you mean "blocked all function body code of KeyedEncoder and KeyedDecoder"?
Comment 5 Brady Eidson 2015-08-26 09:20:36 PDT 
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > Are you just offering this patch because of code inspection, or do you
> > > actually have a reproducible case of the null deref crash?
> > 
> > When I blocked all function body code of KeyedEncoder and KeyedDecoder for
> > my test, it triggered crash in that line.
> 
> For what test? This patch has no test.

Since I hadn't explicitly stated this, I will now - Patches require tests unless there's a darned good reason why they're untestable.

It sounds like you have a way of triggering the crash, we'd like a test that triggers the crash along with the patch.
Comment 6 Jincheol Jo 2015-08-26 19:31:59 PDT 
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (In reply to comment #2)
> > > > Are you just offering this patch because of code inspection, or do you
> > > > actually have a reproducible case of the null deref crash?
> > > 
> > > When I blocked all function body code of KeyedEncoder and KeyedDecoder for
> > > my test, it triggered crash in that line.
> > 
> > For what test? This patch has no test.
> 
> Since I hadn't explicitly stated this, I will now - Patches require tests
> unless there's a darned good reason why they're untestable.
> 
> It sounds like you have a way of triggering the crash, we'd like a test that
> triggers the crash along with the patch.

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (In reply to comment #2)
> > > > Are you just offering this patch because of code inspection, or do you
> > > > actually have a reproducible case of the null deref crash?
> > > 
> > > When I blocked all function body code of KeyedEncoder and KeyedDecoder for
> > > my test, it triggered crash in that line.
> > 
> > For what test? This patch has no test.
> 
> Since I hadn't explicitly stated this, I will now - Patches require tests
> unless there's a darned good reason why they're untestable.
> 
> It sounds like you have a way of triggering the crash, we'd like a test that
> triggers the crash along with the patch.
Thanks for quick reply, I will prepare a test.
Comment 7 Michael Catanzaro 2015-12-31 14:42:24 PST 
Comment on attachment 259832 [details]
Patch

Removing this from the request queue, pending a testcase.