Modified: trunk/Tools/ChangeLog (249199 => 249200)
--- trunk/Tools/ChangeLog 2019-08-28 15:36:37 UTC (rev 249199)
+++ trunk/Tools/ChangeLog 2019-08-28 15:45:20 UTC (rev 249200)
@@ -1,3 +1,19 @@
+2019-08-28 Jonathan Bedard <[email protected]>
+
+ results.webkit.org: Sanitize all commit arguments on upload
+ https://bugs.webkit.org/show_bug.cgi?id=201189
+ <rdar://problem/54564837>
+
+ Reviewed by Aakash Jain.
+
+ * resultsdbpy/resultsdbpy/controller/commit.py:
+ (Commit.__init__): Only allow commits to be constructed with valid values.
+ * resultsdbpy/resultsdbpy/controller/commit_controller.py:
+ (CommitController.register): Strip potential API key.
+ * resultsdbpy/resultsdbpy/controller/commit_unittest.py:
+ (CommitUnittest.test_invalid): Test that commits which contain html inside the
+ repository_id, branch or commit id are rejected.
+
2019-08-02 Claudio Saavedra <[email protected]>
[GTK][WPE] Implement HSTS for the soup network backend
Modified: trunk/Tools/resultsdbpy/resultsdbpy/controller/commit.py (249199 => 249200)
--- trunk/Tools/resultsdbpy/resultsdbpy/controller/commit.py 2019-08-28 15:36:37 UTC (rev 249199)
+++ trunk/Tools/resultsdbpy/resultsdbpy/controller/commit.py 2019-08-28 15:45:20 UTC (rev 249200)
@@ -22,6 +22,7 @@
import calendar
import json
+import re
from datetime import datetime
from resultsdbpy.flask_support.util import FlaskJSONEncoder
@@ -29,6 +30,7 @@
class Commit(object):
TIMESTAMP_TO_UUID_MULTIPLIER = 100
+ MAX_KEY_LENGTH = 128
@classmethod
def from_json(cls, data):
@@ -49,10 +51,17 @@
raise ValueError(f'{argument[0]} is not defined for commit')
self.repository_id = str(repository_id)
+ if not re.match(r'^[a-zA-Z?]+$', self.repository_id) or len(self.repository_id) > self.MAX_KEY_LENGTH:
+ raise ValueError(f"'{self.repository_id}' is an invalid repository id")
+
self.branch = str(branch)
+ if not re.match(r'^[a-zA-Z0-9-.?/]+$', self.branch) or len(self.branch) > self.MAX_KEY_LENGTH:
+ raise ValueError(f"'{self.branch}' is an invalid branch name")
# An id is either a git commit or SVN revision.
self.id = str(id)
+ if not re.match(r'^[a-fA-F0-9?]+$', self.id) or len(self.id) > 40:
+ raise ValueError(f"'{self.id}' is an invalid commit id")
if isinstance(timestamp, datetime):
self.timestamp = timestamp
Modified: trunk/Tools/resultsdbpy/resultsdbpy/controller/commit_controller.py (249199 => 249200)
--- trunk/Tools/resultsdbpy/resultsdbpy/controller/commit_controller.py 2019-08-28 15:36:37 UTC (rev 249199)
+++ trunk/Tools/resultsdbpy/resultsdbpy/controller/commit_controller.py 2019-08-28 15:45:20 UTC (rev 249200)
@@ -267,6 +267,8 @@
if is_endpoint:
try:
commit = request.form or json.loads(request.get_data())
+ if 'api_key' in commit:
+ del commit['api_key']
except ValueError:
abort(400, description='Expected uploaded data to be json')
Modified: trunk/Tools/resultsdbpy/resultsdbpy/controller/commit_unittest.py (249199 => 249200)
--- trunk/Tools/resultsdbpy/resultsdbpy/controller/commit_unittest.py 2019-08-28 15:36:37 UTC (rev 249199)
+++ trunk/Tools/resultsdbpy/resultsdbpy/controller/commit_unittest.py 2019-08-28 15:45:20 UTC (rev 249200)
@@ -100,9 +100,58 @@
).uuid, 153755068501)
def test_invalid(self):
- with self.assertRaises(ValueError):
+ with self.assertRaises(ValueError) as error:
Commit(
repository_id='safari', branch='master',
id='7be4084258a452e8fe22f36287c5b321e9c8249b',
timestamp=None,
)
+ self.assertEqual(str(error.exception), 'timestamp is not defined for commit')
+
+ with self.assertRaises(ValueError) as error:
+ Commit(
+ repository_id='invalid-repo', branch='master',
+ id='7be4084258a452e8fe22f36287c5b321e9c8249b',
+ timestamp=1537550685,
+ )
+ self.assertEqual(str(error.exception), "'invalid-repo' is an invalid repository id")
+
+ with self.assertRaises(ValueError) as error:
+ Commit(
+ repository_id='i' * 129, branch='master',
+ id='7be4084258a452e8fe22f36287c5b321e9c8249b',
+ timestamp=1537550685,
+ )
+ self.assertEqual(str(error.exception), f"'{'i' * 129}' is an invalid repository id")
+
+ with self.assertRaises(ValueError) as error:
+ Commit(
+ repository_id='safari', branch='<html>invalid-branch</html>',
+ id='7be4084258a452e8fe22f36287c5b321e9c8249b',
+ timestamp=1537550685,
+ )
+ self.assertEqual(str(error.exception), "'<html>invalid-branch</html>' is an invalid branch name")
+
+ with self.assertRaises(ValueError) as error:
+ Commit(
+ repository_id='safari', branch='i' * 129,
+ id='7be4084258a452e8fe22f36287c5b321e9c8249b',
+ timestamp=1537550685,
+ )
+ self.assertEqual(str(error.exception), f"'{'i' * 129}' is an invalid branch name")
+
+ with self.assertRaises(ValueError) as error:
+ Commit(
+ repository_id='safari', branch='master',
+ id='<html>1234</html>',
+ timestamp=1537550685,
+ )
+ self.assertEqual(str(error.exception), "'<html>1234</html>' is an invalid commit id")
+
+ with self.assertRaises(ValueError) as error:
+ Commit(
+ repository_id='safari', branch='master',
+ id='0' * 41,
+ timestamp=1537550685,
+ )
+ self.assertEqual(str(error.exception), f"'{'0' * 41}' is an invalid commit id")