Bug 148423 – [WebKit2] add some protection code at IndexedDB

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKit2

[Jincheol Jo]

Metadata pointer seems to need a null protection code.
Its member is accessed in next line.
I guess it can trigger crash.

[Jincheol Jo]

Created attachment 259832 [details]
Patch

[Brady Eidson]

Are you just offering this patch because of code inspection, or do you actually have a reproducible case of the null deref crash?

[Jincheol Jo]

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

[Brady Eidson]

(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"?

[Brady Eidson]

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

[Jincheol Jo]

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

[Michael Catanzaro]

Comment on attachment 259832 [details]
Patch

Removing this from the request queue, pending a testcase.