Issue 20142007: Catch the sign zip error earlier to prevent invalid private key crash Chrome.

Issue 20142007: Catch the sign zip error earlier to prevent invalid private key crash Chrome. (Closed)

Created:
7 years, 5 months ago by zhchbin

Modified:
7 years, 4 months ago

Reviewers:
rafaelw, not at google - send to devlin

CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Visibility:
Public.

More Reviews

Description

Catch the sign zip error earlier to prevent invalid private key crash Chrome. BUG=263968 TEST=unit_tests --gtest_filter=ExtensionServiceTest.PackExtension

Patch Set 1 #

Patch Set 2 : Add test case for packing extension with invalid private key. #

Created: 7 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+13 lines, -2 lines)			Patch
M	chrome/browser/extensions/extension_creator.cc	View		1 chunk	+6 lines, -1 line	0 comments	Download
M	chrome/browser/extensions/extension_service_unittest.cc	View	1	1 chunk	+6 lines, -0 lines	0 comments	Download
A +	chrome/test/data/extensions/invalid.pem	View	1	1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 8 (0 generated)

Expand Messages | Collapse Messages

zhchbin

On 2013/07/25 14:25:44, kalman wrote: > lgtm with a test case Done. Please confirm.

7 years, 5 months ago (2013-07-26 02:52:47 UTC) #3

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zhchbin@gmail.com/20142007/4001

7 years, 5 months ago (2013-07-26 07:25:18 UTC) #4

zhchbin

The test case I added will hit a NOTREACHED() exactly while running unit_tests. [2440:3616:0726/010935:6627952:FATAL:signature_creator_nss.cc(87)] Check ...

7 years, 5 months ago (2013-07-26 09:23:51 UTC) #5

not at google - send to devlin

lgtm eh just don't add the new test. factoring this code to make it testable ...

7 years, 5 months ago (2013-07-26 16:24:21 UTC) #6

not at google - send to devlin

not lgtm - turns out that NOTREACHED is ok - but it looks like somebody ...

7 years, 5 months ago (2013-07-26 19:59:35 UTC) #7

zhchbin

7 years, 5 months ago (2013-07-27 00:11:09 UTC) #8

On 2013/07/26 19:59:35, kalman wrote:
> not lgtm - turns out that NOTREACHED is ok - but it looks like somebody has
> another patch out. https://codereview.chromium.org/20794003/

Another patch: https://codereview.chromium.org/20794003/ remove
NOTREACHED()...And these patches somehow are the same. By the way, please take a
look at the difference of the test case. I think it will be more graceful to put
it in the ExtensionServiceTest.PackExtension 

Anyway, thanks for your time.

Expand Messages | Collapse Messages