Summary: | [resultsdbpy] Return new-style commits | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jonathan Bedard <jbedard> | ||||||||||||||
Component: | Tools / Tests | Assignee: | Jonathan Bedard <jbedard> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | dewei_zhu, webkit-bug-importer, zhifei_fang | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=221860 | ||||||||||||||||
Attachments: |
|
Description
Jonathan Bedard
2021-03-16 10:40:22 PDT
Created attachment 424278 [details]
Patch
Created attachment 424399 [details]
Patch
Created attachment 424554 [details]
Patch
Comment on attachment 424554 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424554&action=review r=me > Tools/Scripts/libraries/resultsdbpy/resultsdbpy/controller/commit_controller.py:32 > + if bool(ref) + bool(uuid) + bool(timestamp) > 1: Is this equivalent to `any([ref, uuid, timestamp])`? > Tools/Scripts/libraries/webkitscmpy/webkitscmpy/mocks/remote/git_hub.py:-26 > -from datetime import datetime Why do we defer the import here? Comment on attachment 424554 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424554&action=review >> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/controller/commit_controller.py:32 >> + if bool(ref) + bool(uuid) + bool(timestamp) > 1: > > Is this equivalent to `any([ref, uuid, timestamp])`? Not quite. We need at most one of these arguments. >> Tools/Scripts/libraries/webkitscmpy/webkitscmpy/mocks/remote/git_hub.py:-26 >> -from datetime import datetime > > Why do we defer the import here? Because date time is a bit of a mess, and we can end up having trouble installing the module on some Python 2 configurations. Since we only need it for testing, it makes the most sense to defer the import. Comment on attachment 424554 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424554&action=review > Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/commit.js:114 > + <<a href="mailto:${escapeHTML(cell.commit.author.emails[0])}">${escapeHTML(cell.commit.author.emails[0])}</a>> <br>` : ''} Since the author.email is in an attribute, just escape html tag may not be enough, it can be easily inject something like this: " onMouseover= "javascript:alert('You are hacked!') Then you will end up with something like <a href="mailto:" onmouseover="alert('You are hacked')"> Need to consider " as welll, it can easily close href and create a new attribute, and easily inject with some unwanted code > Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/timeline.js:195 > + <<a href="mailto:${escapeHTML(node.label.author.emails[0])}">${escapeHTML(node.label.author.emails[0])}</a>>` : ''} Ditto Created attachment 424601 [details]
Patch
Committed r275195: <https://commits.webkit.org/r275195> All reviewed patches have been landed. Closing bug and clearing flags on attachment 424601 [details]. Reopening to attach new patch. Created attachment 424664 [details]
Patch for landing
ChangeLog entry in Tools/ChangeLog contains OOPS!. Created attachment 424666 [details]
Patch for landing
Committed r275226: <https://commits.webkit.org/r275226> All reviewed patches have been landed. Closing bug and clearing flags on attachment 424666 [details]. |