NEW 156541
Fixed crash in PrettyPatch. 
https://bugs.webkit.org/show_bug.cgi?id=156541
Summary Fixed crash in PrettyPatch.
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-
DetailsFormatted DiffDiff
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.