From 5ba2120362ceb49b89e0536108792fd7c824e28a Mon Sep 17 00:00:00 2001
From: Jack Neus <jackneus@google.com>
Date: Wed, 9 Jun 2021 15:21:25 +0000
Subject: [PATCH] repo: properly handle NoneType in Default/Remote equality
 checks

BUG=none
TEST=none

Change-Id: I4ccdbbc7ba4b6f6e20c6959db1b46fdb44ea2819
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/308982
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: Jack Neus <jackneus@google.com>
---
 manifest_xml.py            | 10 ++++++++++
 tests/test_manifest_xml.py | 31 +++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/manifest_xml.py b/manifest_xml.py
index 30e96584..09cae6f5 100644
--- a/manifest_xml.py
+++ b/manifest_xml.py
@@ -122,9 +122,13 @@ class _Default(object):
   sync_tags = True
 
   def __eq__(self, other):
+    if not isinstance(other, _Default):
+      return False
     return self.__dict__ == other.__dict__
 
   def __ne__(self, other):
+    if not isinstance(other, _Default):
+      return True
     return self.__dict__ != other.__dict__
 
 
@@ -147,12 +151,18 @@ class _XmlRemote(object):
     self.resolvedFetchUrl = self._resolveFetchUrl()
 
   def __eq__(self, other):
+    if not isinstance(other, _XmlRemote):
+      return False
     return self.__dict__ == other.__dict__
 
   def __ne__(self, other):
+    if not isinstance(other, _XmlRemote):
+      return True
     return self.__dict__ != other.__dict__
 
   def _resolveFetchUrl(self):
+    if self.fetchUrl is None:
+      return ''
     url = self.fetchUrl.rstrip('/')
     manifestUrl = self.manifestUrl.rstrip('/')
     # urljoin will gets confused over quite a few things.  The ones we care
diff --git a/tests/test_manifest_xml.py b/tests/test_manifest_xml.py
index bd74780d..2a16900a 100644
--- a/tests/test_manifest_xml.py
+++ b/tests/test_manifest_xml.py
@@ -607,3 +607,34 @@ class ContactinfoElementTests(ManifestParseTestCase):
         '<?xml version="1.0" ?><manifest>'
         f'<contactinfo bugurl="{bugurl}"/>'
         '</manifest>')
+
+
+class DefaultElementTests(ManifestParseTestCase):
+  """Tests for <default>."""
+
+  def test_default(self):
+    """Check default settings."""
+    a = manifest_xml._Default()
+    a.revisionExpr = 'foo'
+    a.remote = manifest_xml._XmlRemote(name='remote')
+    b = manifest_xml._Default()
+    b.revisionExpr = 'bar'
+    self.assertEqual(a, a)
+    self.assertNotEqual(a, b)
+    self.assertNotEqual(b, a.remote)
+    self.assertNotEqual(a, 123)
+    self.assertNotEqual(a, None)
+
+
+class RemoteElementTests(ManifestParseTestCase):
+  """Tests for <remote>."""
+
+  def test_remote(self):
+    """Check remote settings."""
+    a = manifest_xml._XmlRemote(name='foo')
+    b = manifest_xml._XmlRemote(name='bar')
+    self.assertEqual(a, a)
+    self.assertNotEqual(a, b)
+    self.assertNotEqual(a, manifest_xml._Default())
+    self.assertNotEqual(a, 123)
+    self.assertNotEqual(a, None)