Bug 223262 – [resultsdbpy] Return new-style commits

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Jonathan Bedard]

When requesting commit information, the results database should return new-styled commits containing identifiers.

[Radar WebKit Bug Importer]

<rdar://problem/75483361>

[Jonathan Bedard]

Created attachment 424278 [details]
Patch

[Jonathan Bedard]

Created attachment 424399 [details]
Patch

[Jonathan Bedard]

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?

[Jonathan Bedard]

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.

[Zhifei Fang]

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
> +                            &#60<a href="mailto:${escapeHTML(cell.commit.author.emails[0])}">${escapeHTML(cell.commit.author.emails[0])}</a>&#62 <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
> +                            &#60<a href="mailto:${escapeHTML(node.label.author.emails[0])}">${escapeHTML(node.label.author.emails[0])}</a>&#62` : ''}

Ditto

[Jonathan Bedard]

Created attachment 424601 [details]
Patch

[EWS]

Committed r275195: <https://commits.webkit.org/r275195>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424601 [details].

[Jonathan Bedard]

Reopening to attach new patch.

[Jonathan Bedard]

Created attachment 424664 [details]
Patch for landing

[EWS]

ChangeLog entry in Tools/ChangeLog contains OOPS!.

[Jonathan Bedard]

Created attachment 424666 [details]
Patch for landing

[EWS]

Committed r275226: <https://commits.webkit.org/r275226>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424666 [details].