Add extra requirements when testing update payloads.
In order to verify that real Chrome OS devices with test
images are actually sending valid Omaha Requests that will get served
an update in the field, we need to check for additional items
(hardware_class and track being set). Since there are non-Chrome OS
devices that can use the devserver, we only enforce this for devices
that interact with the devserver through a devserver proxy i.e. started
with --remote. This is only used in our automated tests.
BUG=chromium:344939
TEST=Unittests which cover this functionality.
Change-Id: I201df9ac5bf5d571b5368137e50fcc397a82befd
Reviewed-on: https://chromium-review.googlesource.com/193979
Reviewed-by: Chris Sosa <sosa@chromium.org>
Commit-Queue: Chris Sosa <sosa@chromium.org>
Tested-by: Chris Sosa <sosa@chromium.org>
diff --git a/autoupdate.py b/autoupdate.py
index 1eacf44..ba4ba8f 100644
--- a/autoupdate.py
+++ b/autoupdate.py
@@ -660,6 +660,28 @@
return (curr_host_info.attrs.pop('forced_update_label', None),
client_version, board)
+ @classmethod
+ def _CheckOmahaRequest(cls, app):
+ """Checks |app| component of Omaha Request for correctly formed data.
+
+ Raises:
+ common_util.DevServerHTTPError: if any check fails. All 400 error codes to
+ indicate a bad HTTP request.
+ """
+ if not app:
+ raise common_util.DevServerHTTPError(
+ 400, 'Missing app component in Omaha Request')
+
+ hardware_class = app.getAttribute('hardware_class')
+ if not hardware_class:
+ raise common_util.DevServerHTTPError(
+ 400, 'hardware_class is required in Omaha Request')
+
+ track = app.getAttribute('track')
+ if not track or not track.endswith('-channel'):
+ raise common_util.DevServerHTTPError(
+ 400, 'Omaha requests need an update channel')
+
def _GetStaticUrl(self):
"""Returns the static url base that should prefix all payload responses."""
x_forwarded_host = cherrypy.request.headers.get('X-Forwarded-Host')
@@ -848,6 +870,9 @@
try:
# Are we provisioning a remote or local payload?
if self.remote_payload:
+
+ self._CheckOmahaRequest(app)
+
# If no explicit label was provided, use the value of --payload.
if not label:
label = self.payload_path
@@ -883,7 +908,7 @@
if self.public_key:
public_key_data = base64.b64encode(open(self.public_key, 'r').read())
- update_response = autoupdate_lib.GetUpdateResponse(
+ update_response = autoupdate_lib.GetUpdateResponse(
metadata_obj.sha1, metadata_obj.sha256, metadata_obj.size, url,
metadata_obj.is_delta_format, metadata_obj.metadata_size,
signed_metadata_hash, public_key_data, protocol, self.critical_update)
diff --git a/autoupdate_unittest.py b/autoupdate_unittest.py
index 7451e38..13c1f2c 100755
--- a/autoupdate_unittest.py
+++ b/autoupdate_unittest.py
@@ -30,8 +30,19 @@
<event eventresult="%(event_result)d" eventtype="%(event_type)d" />
</client_test>"""
+# Test request with additional fields needed for full Omaha protocol.
+_FULL_TEST_REQUEST = """
+<client_test xmlns:o="http://www.google.com/update2/request" updaterversion="%(client)s" protocol="3.0">
+ <app version="%(version)s" track="%(track)s" board="%(board)s"
+ hardware_class="Test Device" />
+ <updatecheck />
+ <event eventresult="%(event_result)d" eventtype="%(event_type)d" />
+</client_test>"""
+
#pylint: disable=W0212
class AutoupdateTest(mox.MoxTestBase):
+ """Tests for the autoupdate.Autoupdate class."""
+
def setUp(self):
mox.MoxTestBase.setUp(self)
self.mox.StubOutWithMock(common_util, 'GetFileSize')
@@ -50,7 +61,7 @@
self.test_dict = {
'client': 'ChromeOSUpdateEngine-1.0',
'version': 'ForcedUpdate',
- 'track': 'unused_var',
+ 'track': 'test-channel',
'board': self.test_board,
'event_result': 2,
'event_type': 3
@@ -313,7 +324,8 @@
payload_path=remote_payload_path,
remote_payload=True)
- test_data = _TEST_REQUEST % self.test_dict
+ incomplete_test_data = _TEST_REQUEST % self.test_dict
+ complete_test_data = _FULL_TEST_REQUEST % self.test_dict
au_mock._GetRemotePayloadAttrs(remote_url).AndReturn(
autoupdate.UpdateMetadata(self.sha1, self.sha256, self.size, False,
@@ -323,7 +335,11 @@
'3.0', False).AndReturn(self.payload)
self.mox.ReplayAll()
- self.assertEqual(au_mock.HandleUpdatePing(test_data), self.payload)
+ # This should fail because of missing fields.
+ self.assertRaises(common_util.DevServerHTTPError,
+ au_mock.HandleUpdatePing, incomplete_test_data)
+ # This should have enough information.
+ self.assertEqual(au_mock.HandleUpdatePing(complete_test_data), self.payload)
self.mox.VerifyAll()
diff --git a/common_util.py b/common_util.py
index 7b3c7b2..f60014c 100644
--- a/common_util.py
+++ b/common_util.py
@@ -7,6 +7,7 @@
import ast
import base64
import binascii
+import cherrypy
import distutils.version
import errno
import hashlib
@@ -32,6 +33,19 @@
pass
+class DevServerHTTPError(cherrypy.HTTPError):
+ """Exception class to log the HTTPResponse before routing it to cherrypy."""
+ def __init__(self, status, message):
+ """CherryPy error with logging.
+
+ Args:
+ status: HTTPResponse status.
+ message: Message associated with the response.
+ """
+ cherrypy.HTTPError.__init__(self, status, message)
+ _Log('HTTPError status: %s message: %s', status, message)
+
+
def MkDirP(directory):
"""Thread-safely create a directory like mkdir -p."""
try:
diff --git a/devserver.py b/devserver.py
index 96f8b87..7caaadc 100755
--- a/devserver.py
+++ b/devserver.py
@@ -96,19 +96,6 @@
"""Exception class used by this module."""
-class DevServerHTTPError(cherrypy.HTTPError):
- """Exception class to log the HTTPResponse before routing it to cherrypy."""
- def __init__(self, status, message):
- """CherryPy error with logging.
-
- Args:
- status: HTTPResponse status.
- message: Message associated with the response.
- """
- cherrypy.HTTPError.__init__(self, status, message)
- _Log('HTTPError status: %s message: %s', status, message)
-
-
def _LeadingWhiteSpaceCount(string):
"""Count the amount of leading whitespace in a string.
@@ -321,7 +308,7 @@
label = label.strip()
if label:
return updater.HandleSetUpdatePing(ip, label)
- raise DevServerHTTPError(400, 'No label provided.')
+ raise common_util.DevServerHTTPError(400, 'No label provided.')
@cherrypy.expose
@@ -653,13 +640,13 @@
return _PrintDocStringAsHTML(self.latestbuild)
if 'target' not in kwargs:
- raise DevServerHTTPError(500, 'Error: target= is required!')
+ raise common_util.DevServerHTTPError(500, 'Error: target= is required!')
try:
return common_util.GetLatestBuildVersion(
updater.static_dir, kwargs['target'],
milestone=kwargs.get('milestone'))
except common_util.CommonUtilError as errmsg:
- raise DevServerHTTPError(500, str(errmsg))
+ raise common_util.DevServerHTTPError(500, str(errmsg))
@cherrypy.expose
def controlfiles(self, **kwargs):
@@ -691,7 +678,7 @@
return _PrintDocStringAsHTML(self.controlfiles)
if 'build' not in kwargs:
- raise DevServerHTTPError(500, 'Error: build= is required!')
+ raise common_util.DevServerHTTPError(500, 'Error: build= is required!')
if 'control_path' not in kwargs:
if 'suite_name' in kwargs and kwargs['suite_name']:
@@ -776,8 +763,8 @@
relative_path = xbuddy.XBuddy.ParseBoolean(boolean_string)
if return_dir and relative_path:
- raise DevServerHTTPError(500, 'Cannot specify both return_dir and '
- 'relative_path')
+ raise common_util.DevServerHTTPError(
+ 500, 'Cannot specify both return_dir and relative_path')
# For updates, we optimize downloading of test images.
file_name = None
@@ -999,7 +986,11 @@
'devserver.')
group.add_option('--remote_payload',
action='store_true', default=False,
- help='Payload is being served from a remote machine')
+ help='Payload is being served from a remote machine. With '
+ 'this setting enabled, this devserver instance serves as '
+ 'just an Omaha server instance. In this mode, the '
+ 'devserver enforces a few extra components of the Omaha '
+ 'protocol e.g. hardware class being sent etc.')
group.add_option('-u', '--urlbase',
metavar='URL',
help='base URL for update images, other than the '
