Bug 35733 - webkit-patch doesn't work well with security bug
Summary: webkit-patch doesn't work well with security bug
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Fumitoshi Ukai
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-04 04:40 PST by Fumitoshi Ukai
Modified: 2010-03-08 22:20 PST (History)
4 users (show)

See Also:


Attachments
Patch (1.24 KB, patch)
2010-03-05 01:01 PST, Fumitoshi Ukai
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fumitoshi Ukai 2010-03-04 04:40:09 PST
security bug is restricted to access.
Even if I could browse the security bug, webkit-patch fails to prepare or post.

$ ./WebKitTools/Scripts/webkit-patch prepare XXXXX
  Running status to find changed, added, or removed files.
  Reviewing diff to determine which lines changed.
  Extracting affected function names from source files.
  Change author: Fumitoshi Ukai <ukai@chromium.org>.
  Bug XXXXX has no bug description. Maybe you set wrong bug ID?
  The bug URL: https://bugs.webkit.org/show_bug.cgi?id=XXXXX&ctype=xml
ERROR: Unable to prepare ChangeLogs.

$ ./WebKitTools/Scripts/webkit-patch post
Running check-webkit-style
Total errors found: 0 in 8 files
Was that diff correct? [Y/n]: y
Fetching: https://bugs.webkit.org/show_bug.cgi?id=XXXXX&ctype=xml
Traceback (most recent call last):
  File "./WebKitTools/Scripts/webkit-patch", line 109, in <module>
    WebKitPatch().main()
  File "/Users/ukai/src/chromium-webkit/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/multicommandtool.py", line 299, in main
  File "/Users/ukai/src/chromium-webkit/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/multicommandtool.py", line 113, in check_arguments_and_execute
  File "/Users/ukai/src/chromium-webkit/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/commands/abstractsequencedcommand.py", line 43, in execute
  File "/Users/ukai/src/chromium-webkit/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/stepsequence.py", line 66, in run_and_handle_errors
  File "/Users/ukai/src/chromium-webkit/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/stepsequence.py", line 60, in _run
  File "/Users/ukai/src/chromium-webkit/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/steps/obsoletepatches.py", line 46, in run
  File "/Users/ukai/src/chromium-webkit2/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/bugzilla.py", line 426, in fetch_bug
    return Bug(self.fetch_bug_dictionary(bug_id), self)
  File "/Users/ukai/src/chromium-webkit2/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/bugzilla.py", line 421, in fetch_bug_dictionary
    return self._parse_bug_page(self._fetch_bug_page(bug_id))
  File "/Users/ukai/src/chromium-webkit2/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/bugzilla.py", line 404, in _parse_bug_page
    bug["title"] = unicode(soup.find("short_desc").string)
AttributeError: 'NoneType' object has no attribute 'string'
Comment 1 Fumitoshi Ukai 2010-03-04 21:22:06 PST
land also fails

$ TZ=America/Los_Angeles ./WebKitTools/Scripts/webkit-patch land 
Fetching: https://bugs.webkit.org/show_bug.cgi?id=XXXX&ctype=xml
Traceback (most recent call last):
  File "./WebKitTools/Scripts/webkit-patch", line 109, in <module>
    WebKitPatch().main()
  File "/Users/ukai/src/chromium-webkit/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/multicommandtool.py", line 299, in main
  File "/Users/ukai/src/chromium-webkit/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/multicommandtool.py", line 113, in check_arguments_and_execute
  File "/Users/ukai/src/chromium-webkit/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/commands/abstractsequencedcommand.py", line 43, in execute
  File "/Users/ukai/src/chromium-webkit/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/stepsequence.py", line 66, in run_and_handle_errors
  File "/Users/ukai/src/chromium-webkit/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/stepsequence.py", line 60, in _run
  File "/Users/ukai/src/chromium-webkit/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/steps/updatechangelogswithreviewer.py", line 63, in run
  File "/Users/ukai/src/chromium-webkit/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/steps/updatechangelogswithreviewer.py", line 45, in _guess_reviewer_from_bug
  File "/Users/ukai/src/chromium-webkit2/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/bugzilla.py", line 426, in fetch_bug
    return Bug(self.fetch_bug_dictionary(bug_id), self)
  File "/Users/ukai/src/chromium-webkit2/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/bugzilla.py", line 421, in fetch_bug_dictionary
    return self._parse_bug_page(self._fetch_bug_page(bug_id))
  File "/Users/ukai/src/chromium-webkit2/src/third_party/WebKit/WebKitTools/Scripts/webkitpy/bugzilla.py", line 404, in _parse_bug_page
    bug["title"] = unicode(soup.find("short_desc").string)
AttributeError: 'NoneType' object has no attribute 'string'
Comment 2 Adam Barth 2010-03-04 21:34:17 PST
Yeah.  We need to call bugzilla.py's authenticate method more often.  The patches are pretty simple to write one you have a reproducing test case.  You can see other examples in bugzilla.py.
Comment 3 Fumitoshi Ukai 2010-03-05 01:01:01 PST
Created attachment 50091 [details]
Patch
Comment 4 Fumitoshi Ukai 2010-03-05 01:04:32 PST
(In reply to comment #2)
> Yeah.  We need to call bugzilla.py's authenticate method more often.  The
> patches are pretty simple to write one you have a reproducing test case.  You
> can see other examples in bugzilla.py.

I'm not sure if we always authenticate before accessing bugzilla.
This patch set 1 would fix the post and land.
For prepare, webkit-patch calls prepare-ChangesLogs and it just uses curl to access bugzilla.  How could we fix it?  Use --netrc ?
Comment 5 Adam Barth 2010-03-05 05:36:24 PST
Comment on attachment 50091 [details]
Patch

Interesting approach.  In general, we prefer a test for every change to webkit-patch, especially for low frequency bugs like this one.

I'm not sure of the best solution for prepare-ChangeLog.  It's outside of our python infrastructure, so it doesn't have access to our identify information...
Comment 6 WebKit Commit Bot 2010-03-05 14:05:15 PST
Attachment 50091 [details] was posted by a committer and has review+, assigning to Fumitoshi Ukai for commit.
Comment 7 Alexey Proskuryakov 2010-03-06 17:25:23 PST
ChangeLog should say what was fixed here. "Doesn't work well" is not a sufficient explanation.
Comment 8 Adam Barth 2010-03-08 09:43:44 PST
Are you looking for a resolution here, or just giving advice for the future?
Comment 9 Fumitoshi Ukai 2010-03-08 20:21:22 PST
Committed r55704: <http://trac.webkit.org/changeset/55704>
Comment 10 Fumitoshi Ukai 2010-03-08 20:25:43 PST
(In reply to comment #8)
> Are you looking for a resolution here, or just giving advice for the future?

Sorry for the delay.
I just landed this patch, and create new bug for prepare-ChangeLog
https://bugs.webkit.org/show_bug.cgi?id=35901

Shoud we develop webkitpy version of prepare-ChangeLog?
Comment 11 Eric Seidel (no email) 2010-03-08 22:20:33 PST
If it offered other benefits as part of a re-write, sure.  But I think we have bigger fish to fry for the moment.  Most critical in terms of python re-writes is to finish the new-run-webkit-tests (python) to run-webkit-tests conversion.