Bug 156541

Summary:

Fixed crash in PrettyPatch.

Product:

WebKit

Reporter:

Konstantin Tokarev <annulen>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

NEW

Severity:

Normal

CC:

achristensen, cgarcia, darin, dbates, lforschler, ossy, tonikitoo

Priority:

Version:

WebKit Local Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	achristensen: review+, dbates: commit-queue-

Konstantin Tokarev

Reported 2016-04-13 09:25:12 PDT

PrettyPatch crashed when parsing https://bugs.webkit.org/attachment.cgi?id=276320

Attachments
Patch (2.16 KB, patch) 2016-04-13 09:28 PDT, Konstantin Tokarev	achristensen: review+ dbates: commit-queue-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Konstantin Tokarev

Comment 1 2016-04-13 09:28:30 PDT

Created attachment 276328 [details] Patch

Alex Christensen

Comment 2 2016-04-13 11:40:29 PDT

This looks like it might be good. Sorry for my unfamiliarity with these tests, but how do I run them?

Konstantin Tokarev

Comment 3 2016-04-13 11:47:03 PDT

I've run it with ruby -IWebsites/bugs.webkit.org/PrettyPatch Websites/bugs.webkit.org/PrettyPatch/PrettyPatch_test.rb 4 tests were failing before my patch already

Alex Christensen

Comment 4 2016-04-13 11:51:16 PDT

Thanks, this certainly fixes the crash, but does it correctly process the patch?

Konstantin Tokarev

Comment 5 2016-04-13 12:19:27 PDT

No. It actually parses '--' in header as a removal mark

Daniel Bates

Comment 6 2016-04-13 12:22:20 PDT

Do we have crash report? If this is a crash then is there a Ruby bug for it?

Konstantin Tokarev

Comment 7 2016-04-13 12:26:54 PDT

This is not crash in ruby interpreter, only uncaught exception in script

Daniel Bates

Comment 8 2016-04-13 12:35:00 PDT

(In reply to comment #7) > This is not crash in ruby interpreter, only uncaught exception in script Then "crash" is not correct word to use here. Please update both the bug title of this Bugzilla bug and ChangeLog bug title and description to convey that this fix is for an exception - even better include the name of the exception. If you do not include the name of the exception thrown in the bug title then please explain in English the cause of the exception in the ChangeLog description.

Csaba Osztrogonác

Comment 9 2016-05-10 07:37:06 PDT

Could you rephrase the changelog and land it?

Konstantin Tokarev

Comment 10 2016-05-10 07:58:00 PDT

Actually, I've found that it's not the proper solution to the problem (see comment #5). Script does not bail out anymore, but highlighting can also be fixed. Have you stumbled upon another "evil" patch? If this is the case I'll reword and submit it ASAP

Csaba Osztrogonác

Comment 11 2016-05-10 08:03:46 PDT

Nevermind. I only read Comment 8, not Comment 5.

Note You need to log in before you can comment on or make changes to this bug.