Fix permission handling on project generation from template files

scrapy/commands/startproject.py

@@ -1,10 +1,10 @@
 import re
 import os
-import stat
 import string
 from importlib import import_module
 from os.path import join, exists, abspath
 from shutil import ignore_patterns, move, copy2, copystat
+from stat import S_IWUSR as OWNER_WRITE_PERMISSION
 
 import scrapy
 from scrapy.commands import ScrapyCommand
@@ -78,30 +78,12 @@ class Command(ScrapyCommand):
                 self._copytree(srcname, dstname)
             else:
                 copy2(srcname, dstname)
+                current_permissions = os.stat(dstname).st_mode
+                os.chmod(dstname, current_permissions | OWNER_WRITE_PERMISSION)
+
         copystat(src, dst)
-        self._set_rw_permissions(dst)
-
-    def _set_rw_permissions(self, path):
-        """
-        Sets permissions of a directory tree to +rw and +rwx for folders.
-        This is necessary if the start template files come without write
-        permissions.
-        """
-        mode_rw = (stat.S_IRUSR
-                   | stat.S_IWUSR
-                   | stat.S_IRGRP
-                   | stat.S_IROTH)
-
-        mode_x = (stat.S_IXUSR
-                  | stat.S_IXGRP
-                  | stat.S_IXOTH)
-
-        os.chmod(path, mode_rw | mode_x)
-        for root, dirs, files in os.walk(path):
-            for dir in dirs:
-                os.chmod(join(root, dir), mode_rw | mode_x)
-            for file in files:
-                os.chmod(join(root, file), mode_rw)
+        current_permissions = os.stat(dst).st_mode
+        os.chmod(dst, current_permissions | OWNER_WRITE_PERMISSION)
 
     def run(self, args, opts):
         if len(args) not in (1, 2):

tests/sample_data/read_only_templates/project/module/items.py.tmpl

@@ -0,0 +1,12 @@
+# Define here the models for your scraped items
+#
+# See documentation in:
+# https://docs.scrapy.org/en/latest/topics/items.html
+
+import scrapy
+
+
+class ${ProjectName}Item(scrapy.Item):
+    # define the fields for your item here like:
+    # name = scrapy.Field()
+    pass

tests/sample_data/read_only_templates/project/module/middlewares.py.tmpl

@@ -0,0 +1,103 @@
+# Define here the models for your spider middleware
+#
+# See documentation in:
+# https://docs.scrapy.org/en/latest/topics/spider-middleware.html
+
+from scrapy import signals
+
+# useful for handling different item types with a single interface
+from itemadapter import is_item, ItemAdapter
+
+
+class ${ProjectName}SpiderMiddleware:
+    # Not all methods need to be defined. If a method is not defined,
+    # scrapy acts as if the spider middleware does not modify the
+    # passed objects.
+
+    @classmethod
+    def from_crawler(cls, crawler):
+        # This method is used by Scrapy to create your spiders.
+        s = cls()
+        crawler.signals.connect(s.spider_opened, signal=signals.spider_opened)
+        return s
+
+    def process_spider_input(self, response, spider):
+        # Called for each response that goes through the spider
+        # middleware and into the spider.
+
+        # Should return None or raise an exception.
+        return None
+
+    def process_spider_output(self, response, result, spider):
+        # Called with the results returned from the Spider, after
+        # it has processed the response.
+
+        # Must return an iterable of Request, or item objects.
+        for i in result:
+            yield i
+
+    def process_spider_exception(self, response, exception, spider):
+        # Called when a spider or process_spider_input() method
+        # (from other spider middleware) raises an exception.
+
+        # Should return either None or an iterable of Request or item objects.
+        pass
+
+    def process_start_requests(self, start_requests, spider):
+        # Called with the start requests of the spider, and works
+        # similarly to the process_spider_output() method, except
+        # that it doesn’t have a response associated.
+
+        # Must return only requests (not items).
+        for r in start_requests:
+            yield r
+
+    def spider_opened(self, spider):
+        spider.logger.info('Spider opened: %s' % spider.name)
+
+
+class ${ProjectName}DownloaderMiddleware:
+    # Not all methods need to be defined. If a method is not defined,
+    # scrapy acts as if the downloader middleware does not modify the
+    # passed objects.
+
+    @classmethod
+    def from_crawler(cls, crawler):
+        # This method is used by Scrapy to create your spiders.
+        s = cls()
+        crawler.signals.connect(s.spider_opened, signal=signals.spider_opened)
+        return s
+
+    def process_request(self, request, spider):
+        # Called for each request that goes through the downloader
+        # middleware.
+
+        # Must either:
+        # - return None: continue processing this request
+        # - or return a Response object
+        # - or return a Request object
+        # - or raise IgnoreRequest: process_exception() methods of
+        #   installed downloader middleware will be called
+        return None
+
+    def process_response(self, request, response, spider):
+        # Called with the response returned from the downloader.
+
+        # Must either;
+        # - return a Response object
+        # - return a Request object
+        # - or raise IgnoreRequest
+        return response
+
+    def process_exception(self, request, exception, spider):
+        # Called when a download handler or a process_request()
+        # (from other downloader middleware) raises an exception.
+
+        # Must either:
+        # - return None: continue processing this exception
+        # - return a Response object: stops process_exception() chain
+        # - return a Request object: stops process_exception() chain
+        pass
+
+    def spider_opened(self, spider):
+        spider.logger.info('Spider opened: %s' % spider.name)

tests/sample_data/read_only_templates/project/module/pipelines.py.tmpl

@@ -0,0 +1,13 @@
+# Define your item pipelines here
+#
+# Don't forget to add your pipeline to the ITEM_PIPELINES setting
+# See: https://docs.scrapy.org/en/latest/topics/item-pipeline.html
+
+
+# useful for handling different item types with a single interface
+from itemadapter import ItemAdapter
+
+
+class ${ProjectName}Pipeline:
+    def process_item(self, item, spider):
+        return item

tests/sample_data/read_only_templates/project/module/settings.py.tmpl

@@ -0,0 +1,88 @@
+# Scrapy settings for $project_name project
+#
+# For simplicity, this file contains only settings considered important or
+# commonly used. You can find more settings consulting the documentation:
+#
+#     https://docs.scrapy.org/en/latest/topics/settings.html
+#     https://docs.scrapy.org/en/latest/topics/downloader-middleware.html
+#     https://docs.scrapy.org/en/latest/topics/spider-middleware.html
+
+BOT_NAME = '$project_name'
+
+SPIDER_MODULES = ['$project_name.spiders']
+NEWSPIDER_MODULE = '$project_name.spiders'
+
+
+# Crawl responsibly by identifying yourself (and your website) on the user-agent
+#USER_AGENT = '$project_name (+http://www.yourdomain.com)'
+
+# Obey robots.txt rules
+ROBOTSTXT_OBEY = True
+
+# Configure maximum concurrent requests performed by Scrapy (default: 16)
+#CONCURRENT_REQUESTS = 32
+
+# Configure a delay for requests for the same website (default: 0)
+# See https://docs.scrapy.org/en/latest/topics/settings.html#download-delay
+# See also autothrottle settings and docs
+#DOWNLOAD_DELAY = 3
+# The download delay setting will honor only one of:
+#CONCURRENT_REQUESTS_PER_DOMAIN = 16
+#CONCURRENT_REQUESTS_PER_IP = 16
+
+# Disable cookies (enabled by default)
+#COOKIES_ENABLED = False
+
+# Disable Telnet Console (enabled by default)
+#TELNETCONSOLE_ENABLED = False
+
+# Override the default request headers:
+#DEFAULT_REQUEST_HEADERS = {
+#   'Accept': 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8',
+#   'Accept-Language': 'en',
+#}
+
+# Enable or disable spider middlewares
+# See https://docs.scrapy.org/en/latest/topics/spider-middleware.html
+#SPIDER_MIDDLEWARES = {
+#    '$project_name.middlewares.${ProjectName}SpiderMiddleware': 543,
+#}
+
+# Enable or disable downloader middlewares
+# See https://docs.scrapy.org/en/latest/topics/downloader-middleware.html
+#DOWNLOADER_MIDDLEWARES = {
+#    '$project_name.middlewares.${ProjectName}DownloaderMiddleware': 543,
+#}
+
+# Enable or disable extensions
+# See https://docs.scrapy.org/en/latest/topics/extensions.html
+#EXTENSIONS = {
+#    'scrapy.extensions.telnet.TelnetConsole': None,
+#}
+
+# Configure item pipelines
+# See https://docs.scrapy.org/en/latest/topics/item-pipeline.html
+#ITEM_PIPELINES = {
+#    '$project_name.pipelines.${ProjectName}Pipeline': 300,
+#}
+
+# Enable and configure the AutoThrottle extension (disabled by default)
+# See https://docs.scrapy.org/en/latest/topics/autothrottle.html
+#AUTOTHROTTLE_ENABLED = True
+# The initial download delay
+#AUTOTHROTTLE_START_DELAY = 5
+# The maximum download delay to be set in case of high latencies
+#AUTOTHROTTLE_MAX_DELAY = 60
+# The average number of requests Scrapy should be sending in parallel to
+# each remote server
+#AUTOTHROTTLE_TARGET_CONCURRENCY = 1.0
+# Enable showing throttling stats for every response received:
+#AUTOTHROTTLE_DEBUG = False
+
+# Enable and configure HTTP caching (disabled by default)
+# See https://docs.scrapy.org/en/latest/topics/downloader-middleware.html#httpcache-middleware-settings
+#HTTPCACHE_ENABLED = True
+#HTTPCACHE_EXPIRATION_SECS = 0
+#HTTPCACHE_DIR = 'httpcache'
+#HTTPCACHE_IGNORE_HTTP_CODES = []
+#HTTPCACHE_STORAGE = 'scrapy.extensions.httpcache.FilesystemCacheStorage'

tests/sample_data/read_only_templates/project/module/spiders/__init__.py

@@ -0,0 +1,4 @@
+# This package will contain the spiders of your Scrapy project
+#
+# Please refer to the documentation for information on how to create and manage
+# your spiders.

tests/sample_data/read_only_templates/project/scrapy.cfg

@@ -0,0 +1,11 @@
+# Automatically created by: scrapy startproject
+#
+# For more information about the [deploy] section see:
+# https://scrapyd.readthedocs.io/en/latest/deploy.html
+
+[settings]
+default = ${project_name}.settings
+
+[deploy]
+#url = http://localhost:6800/
+project = ${project_name}

tests/sample_data/read_only_templates/spiders/basic.tmpl

@@ -0,0 +1,10 @@
+import scrapy
+
+
+class $classname(scrapy.Spider):
+    name = '$name'
+    allowed_domains = ['$domain']
+    start_urls = ['http://$domain/']
+
+    def parse(self, response):
+        pass

tests/sample_data/read_only_templates/spiders/crawl.tmpl

@@ -0,0 +1,20 @@
+import scrapy
+from scrapy.linkextractors import LinkExtractor
+from scrapy.spiders import CrawlSpider, Rule
+
+
+class $classname(CrawlSpider):
+    name = '$name'
+    allowed_domains = ['$domain']
+    start_urls = ['http://$domain/']
+
+    rules = (
+        Rule(LinkExtractor(allow=r'Items/'), callback='parse_item', follow=True),
+    )
+
+    def parse_item(self, response):
+        item = {}
+        #item['domain_id'] = response.xpath('//input[@id="sid"]/@value').get()
+        #item['name'] = response.xpath('//div[@id="name"]').get()
+        #item['description'] = response.xpath('//div[@id="description"]').get()
+        return item

tests/sample_data/read_only_templates/spiders/csvfeed.tmpl

@@ -0,0 +1,20 @@
+from scrapy.spiders import CSVFeedSpider
+
+
+class $classname(CSVFeedSpider):
+    name = '$name'
+    allowed_domains = ['$domain']
+    start_urls = ['http://$domain/feed.csv']
+    # headers = ['id', 'name', 'description', 'image_link']
+    # delimiter = '\t'
+
+    # Do any adaptations you need here
+    #def adapt_response(self, response):
+    #    return response
+
+    def parse_row(self, response, row):
+        i = {}
+        #i['url'] = row['url']
+        #i['name'] = row['name']
+        #i['description'] = row['description']
+        return i

tests/sample_data/read_only_templates/spiders/xmlfeed.tmpl

@@ -0,0 +1,16 @@
+from scrapy.spiders import XMLFeedSpider
+
+
+class $classname(XMLFeedSpider):
+    name = '$name'
+    allowed_domains = ['$domain']
+    start_urls = ['http://$domain/feed.xml']
+    iterator = 'iternodes' # you can change this; see the docs
+    itertag = 'item' # change it accordingly
+
+    def parse_node(self, response, selector):
+        item = {}
+        #item['url'] = selector.select('url').get()
+        #item['name'] = selector.select('name').get()
+        #item['description'] = selector.select('description').get()
+        return item

tests/test_commands.py

@@ -6,7 +6,9 @@ import subprocess
 import sys
 import tempfile
 from contextlib import contextmanager
+from itertools import chain
 from os.path import exists, join, abspath
+from pathlib import Path
 from shutil import rmtree, copytree
 from tempfile import mkdtemp
 from threading import Timer
@@ -15,6 +17,7 @@ from twisted.trial import unittest
 
 import scrapy
 from scrapy.commands import ScrapyCommand
+from scrapy.commands.startproject import IGNORE
 from scrapy.settings import Settings
 from scrapy.utils.python import to_unicode
 from scrapy.utils.test import get_testenv
@@ -119,8 +122,34 @@ class StartprojectTest(ProjectTest):
         self.assertEqual(2, self.call('startproject', self.project_name, project_dir, 'another_params'))
 
 
+def get_permissions_dict(path, renamings=None, ignore=None):
+    renamings = renamings or tuple()
+    permissions_dict = {
+        '.': os.stat(path).st_mode,
+    }
+    for root, dirs, files in os.walk(path):
+        nodes = list(chain(dirs, files))
+        if ignore:
+            ignored_names = ignore(root, nodes)
+            nodes = [node for node in nodes
+                        if node not in ignored_names]
+        for node in nodes:
+            absolute_path = os.path.join(root, node)
+            relative_path = os.path.relpath(absolute_path, path)
+            for search_string, replacement in renamings:
+                relative_path = relative_path.replace(
+                    search_string,
+                    replacement
+                )
+            permissions = os.stat(absolute_path).st_mode
+            permissions_dict[relative_path] = permissions
+    return permissions_dict
+
+
 class StartprojectTemplatesTest(ProjectTest):
 
+    maxDiff = None
+
     def setUp(self):
         super(StartprojectTemplatesTest, self).setUp()
         self.tmpl = join(self.temp_path, 'templates')
@@ -139,6 +168,141 @@ class StartprojectTemplatesTest(ProjectTest):
         self.assertIn(self.tmpl_proj, out)
         assert exists(join(self.proj_path, 'root_template'))
 
+    def test_startproject_permissions_from_writable(self):
+        """Check that generated files have the right permissions when the
+        template folder has the same permissions as in the project, i.e.
+        everything is writable."""
+        scrapy_path = scrapy.__path__[0]
+        templates_dir = os.path.join(scrapy_path, 'templates', 'project')
+        project_name = 'startproject1'
+        renamings = (
+            ('module', project_name),
+            ('.tmpl', ''),
+        )
+        expected_permissions = get_permissions_dict(
+            templates_dir,
+            renamings,
+            IGNORE,
+        )
+
+        destination = mkdtemp()
+        process = subprocess.Popen(
+            (
+                sys.executable,
+                '-m',
+                'scrapy.cmdline',
+                'startproject',
+                project_name,
+            ),
+            cwd=destination,
+            env=self.env,
+        )
+        process.wait()
+
+        project_dir = os.path.join(destination, project_name)
+        actual_permissions = get_permissions_dict(project_dir)
+
+        self.assertEqual(actual_permissions, expected_permissions)
+
+    def test_startproject_permissions_from_read_only(self):
+        """Check that generated files have the right permissions when the
+        template folder has been made read-only, which is something that some
+        systems do.
+
+        See https://github.com/scrapy/scrapy/pull/4604
+        """
+        scrapy_path = scrapy.__path__[0]
+        templates_dir = os.path.join(scrapy_path, 'templates', 'project')
+        project_name = 'startproject2'
+        renamings = (
+            ('module', project_name),
+            ('.tmpl', ''),
+        )
+        expected_permissions = get_permissions_dict(
+            templates_dir,
+            renamings,
+            IGNORE,
+        )
+
+        tests_path = os.path.dirname(__file__)
+        read_only_templates_dir = os.path.join(
+            tests_path, 'sample_data', 'read_only_templates'
+        )
+        destination = mkdtemp()
+        process = subprocess.Popen(
+            (
+                sys.executable,
+                '-m',
+                'scrapy.cmdline',
+                'startproject',
+                project_name,
+                '--set',
+                'TEMPLATES_DIR={}'.format(read_only_templates_dir),
+            ),
+            cwd=destination,
+            env=self.env,
+        )
+        process.wait()
+
+        project_dir = os.path.join(destination, project_name)
+        actual_permissions = get_permissions_dict(project_dir)
+
+        self.assertEqual(actual_permissions, expected_permissions)
+
+    def test_startproject_permissions_unchanged_in_destination(self):
+        """Check that pre-existing folders and files in the destination folder
+        do not see their permissions modified."""
+        scrapy_path = scrapy.__path__[0]
+        templates_dir = os.path.join(scrapy_path, 'templates', 'project')
+        project_name = 'startproject3'
+        renamings = (
+            ('module', project_name),
+            ('.tmpl', ''),
+        )
+        expected_permissions = get_permissions_dict(
+            templates_dir,
+            renamings,
+            IGNORE,
+        )
+
+        destination = mkdtemp()
+        project_dir = os.path.join(destination, project_name)
+
+        existing_nodes = {
+            oct(permissions)[2:] + extension: permissions
+            for extension in ('', '.d')
+            for permissions in (
+                0o444, 0o555, 0o644, 0o666, 0o755, 0o777,
+            )
+        }
+        os.mkdir(project_dir)
+        project_dir_path = Path(project_dir)
+        for node, permissions in existing_nodes.items():
+            path = project_dir_path / node
+            if node.endswith('.d'):
+                path.mkdir(mode=permissions)
+            else:
+                path.touch(mode=permissions)
+            expected_permissions[node] = path.stat().st_mode
+
+        process = subprocess.Popen(
+            (
+                sys.executable,
+                '-m',
+                'scrapy.cmdline',
+                'startproject',
+                project_name,
+                '.',
+            ),
+            cwd=project_dir,
+            env=self.env,
+        )
+        process.wait()
+
+        actual_permissions = get_permissions_dict(project_dir)
+
+        self.assertEqual(actual_permissions, expected_permissions)
+
 
 class CommandTest(ProjectTest):