Merge lp:~danilo/linaro-license-protection/bug-1075999 into lp:~linaro-automation/linaro-license-protection/trunk

[Данило Шеган]

When loading a directory listing which contains subdirectories without BUILD-INFO file, we'd get an exception as seen in bug 1075999.

The problem is that BuildInfo(path) looks for BUILD-INFO.txt file in
path/BUILD-INFO.txt if path is a directory, and in dirname(path)/BUILD-INFO.txt
if it's not. However, the pre-check in is_protected() only checks for
dirname(path)/BUILD-INFO.txt, and for a subdirectory without the trailing "/",
this returns the parent directory, and no BUILD-INFO.txt file is found.

I am not entirely sure of the intent of this code, so I am fixing it to work
and look for BUILD-INFO/EULA/OPEN-EULA inside the subdirectory. I've added
a few tests to ensure this works as I intended, but I plan to rework this to
be a bit saner and understandable (iow, encapsulated in the right places).

Tests
=====
To run the new tests

  ./manage.py test license_protected_downloads.HowtoViewTests

QA
==

Create a directory with a file, BUILD-INFO.txt that opens it up and an empty
subdirectory. It should not crash when you browse into it.

[lp:~danilo/linaro-license-protection/bug-1075999]

149. By Данило Шеган on 2012-11-08

Use the same logic for finding build-info file in is_protected as used in BuildInfo constructor.

[Stevan Radaković (stevanr)]

Looks good, thanks for the fix.
Approve +1.

review: Approve

=== modified file 'license_protected_downloads/tests/__init__.py'
--- license_protected_downloads/tests/__init__.py	2012-10-16 07:09:04 +0000
+++ license_protected_downloads/tests/__init__.py	2012-11-08 09:45:23 +0000
@@ -2,7 +2,10 @@
 from license_protected_downloads.tests.test_models import LicenseTestCase
 from license_protected_downloads.tests.test_pep8 import TestPep8
 from license_protected_downloads.tests.test_pyflakes import TestPyflakes
-from license_protected_downloads.tests.test_views import ViewTests
+from license_protected_downloads.tests.test_views import (
+    HowtoViewTests,
+    ViewTests,
+    )
 from license_protected_downloads.tests.test_openid_auth import TestOpenIDAuth
 from license_protected_downloads.tests.test_custom_commands \
  import SetsuperuserCommandTest
@@ -13,6 +16,7 @@
 __test__ = {
     'LicenseTestCase': LicenseTestCase,
     'ViewTests': ViewTests,
+    'HowtoViewTests': HowtoViewTests,
     'BuildInfoTests': BuildInfoTests,
     'TestPep8': TestPep8,
     'TestPyflakes': TestPyflakes,

=== modified file 'license_protected_downloads/tests/test_views.py'
--- license_protected_downloads/tests/test_views.py	2012-10-10 18:59:02 +0000
+++ license_protected_downloads/tests/test_views.py	2012-11-08 09:45:23 +0000
@@ -6,6 +6,7 @@
 import os
 import unittest
 import urlparse
+import shutil
 import tempfile
 
 from license_protected_downloads import bzr_version
@@ -21,6 +22,41 @@
 TESTSERVER_ROOT = os.path.join(THIS_DIRECTORY, "testserver_root")
 
 
+class temporary_directory(object):
+    """Creates a context manager for a temporary directory."""
+
+    def __enter__(self):
+        self.root = tempfile.mkdtemp()
+        return self
+
+    def __exit__(self, *args):
+        shutil.rmtree(self.root)
+
+    def make_file(self, name, data=None, with_buildinfo=False):
+        """Creates a file in this temporary directory."""
+        full_path = os.path.join(self.root, name)
+        dir_name = os.path.dirname(full_path)
+        try:
+            os.makedirs(dir_name)
+        except os.error:
+            pass
+        if with_buildinfo:
+            buildinfo_name = os.path.join(dir_name, 'BUILD-INFO.txt')
+            base_name = os.path.basename(full_path)
+            with open(buildinfo_name, 'w') as buildinfo_file:
+                buildinfo_file.write(
+                    'Format-Version: 0.1\n\n'
+                    'Files-Pattern: %s\n'
+                    'License-Type: open\n' % base_name)
+        target = open(full_path, "w")
+        if data is None:
+            return target
+        else:
+            target.write(data)
+            target.close()
+            return full_path
+
+
 class ViewTests(TestCase):
     def setUp(self):
         self.client = Client()
@@ -637,5 +673,63 @@
         fname = os.path.join(TESTSERVER_ROOT, "../file")
         self.assertFalse(is_same_parent_dir(TESTSERVER_ROOT, fname))
 
+
+class HowtoViewTests(TestCase):
+    def setUp(self):
+        self.client = Client()
+        self.old_served_paths = settings.SERVED_PATHS
+        settings.SERVED_PATHS = [os.path.join(THIS_DIRECTORY,
+                                             "testserver_root")]
+
+    def tearDown(self):
+        settings.SERVED_PATHS = self.old_served_paths
+
+    def test_no_howtos(self):
+        with temporary_directory() as serve_root:
+            settings.SERVED_PATHS = [serve_root.root]
+            serve_root.make_file(
+                "build/9/build.tar.bz2", with_buildinfo=True)
+            response = self.client.get('/build/9/')
+            self.assertEqual(response.status_code, 200)
+            self.assertContains(response, 'build.tar.bz2')
+
+    def test_howtos_without_license(self):
+        with temporary_directory() as serve_root:
+            settings.SERVED_PATHS = [serve_root.root]
+            serve_root.make_file(
+                "build/9/build.tar.bz2", with_buildinfo=True)
+            serve_root.make_file(
+                "build/9/howto/HOWTO_test.txt", data=".h1 HowTo Test")
+            response = self.client.get('/build/9/')
+            self.assertEqual(response.status_code, 200)
+            self.assertContains(response, 'build.tar.bz2')
+
+    def test_howtos_with_license_in_buildinfo(self):
+        with temporary_directory() as serve_root:
+            settings.SERVED_PATHS = [serve_root.root]
+            serve_root.make_file(
+                "build/9/build.tar.bz2", with_buildinfo=True)
+            serve_root.make_file(
+                "build/9/howto/HOWTO_test.txt", data=".h1 HowTo Test",
+                with_buildinfo=True)
+            response = self.client.get('/build/9/')
+            self.assertEqual(response.status_code, 200)
+            self.assertContains(response, 'howto')
+
+    def test_howtos_with_license_in_openeula(self):
+        with temporary_directory() as serve_root:
+            settings.SERVED_PATHS = [serve_root.root]
+            serve_root.make_file(
+                "build/9/build.tar.bz2", with_buildinfo=True)
+            serve_root.make_file(
+                "build/9/howto/HOWTO_test.txt", data=".h1 HowTo Test",
+                with_buildinfo=False)
+            serve_root.make_file(
+                "build/9/howto/OPEN-EULA.txt", with_buildinfo=False)
+            response = self.client.get('/build/9/')
+            self.assertEqual(response.status_code, 200)
+            self.assertContains(response, 'howto')
+
+
 if __name__ == '__main__':
     unittest.main()

=== modified file 'license_protected_downloads/views.py'
--- license_protected_downloads/views.py	2012-10-25 13:04:34 +0000
+++ license_protected_downloads/views.py	2012-11-08 09:45:23 +0000
@@ -215,9 +215,13 @@
 def is_protected(path):
     build_info = None
     max_index = 1
-    buildinfo_path = os.path.join(os.path.dirname(path), "BUILD-INFO.txt")
-    open_eula_path = os.path.join(os.path.dirname(path), "OPEN-EULA.txt")
-    eula_path = os.path.join(os.path.dirname(path), "EULA.txt")
+    base_path = path
+    if not os.path.isdir(base_path):
+        base_path = os.path.dirname(base_path)
+
+    buildinfo_path = os.path.join(base_path, "BUILD-INFO.txt")
+    open_eula_path = os.path.join(base_path, "OPEN-EULA.txt")
+    eula_path = os.path.join(base_path, "EULA.txt")
 
     if os.path.isfile(buildinfo_path):
         try:
@@ -255,7 +259,7 @@
         openid_teams = False
         with open(license_file, "r") as infile:
             license_text = infile.read()
-    elif _check_special_eula(os.path.dirname(path) + "/*"):
+    elif _check_special_eula(base_path + "/*"):
         return "OPEN"
     else:
         return []