Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions bottle.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"""

import sys
from pathlib import Path

__author__ = 'Marcel Hellkamp'
__version__ = '0.14-dev'
Expand Down Expand Up @@ -2306,10 +2307,16 @@ def load_config(self, filename, **options):
other sections.

:param filename: The path of a config file, or a list of paths.
Can be a string or a :class:`pathlib.Path` object.
:param options: All keyword parameters are passed to the underlying
:class:`python:configparser.ConfigParser` constructor call.

"""
if isinstance(filename, Path):
filename = str(filename)
elif isinstance(filename, (list, tuple)):
filename = [str(f) if isinstance(f, Path) else f for f in filename]

options.setdefault('allow_no_value', True)
options.setdefault('interpolation', configparser.ExtendedInterpolation())
conf = configparser.ConfigParser(**options)
Expand Down Expand Up @@ -2746,8 +2753,9 @@ def static_file(filename, root,
that can be sent back to the client.

:param filename: Name or path of the file to send, relative to ``root``.
Can be a string or a :class:`pathlib.Path` object.
:param root: Root path for file lookups. Should be an absolute directory
path.
path. Can be a string or a :class:`pathlib.Path` object.
:param mimetype: Provide the content-type header (default: guess from
file extension)
:param download: If True, ask the browser to open a `Save as...` dialog
Expand All @@ -2773,6 +2781,11 @@ def static_file(filename, root,
check or continue partial downloads) are also handled automatically.
"""

if isinstance(root, Path):
root = str(root)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can skip the type check and just call str(var) or Path(var) to get what you need. This may even be cheaper than the type check :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @defnull, thank you for your review!

I have a concern about just calling str(var) on it. If someone passes an invalid type here, like None, the value will become a string literal "None", and it may not fail here, but fail eventually later in the codebase somewhere else. So I think it's better to just handle the expected type here, to reduce the risk of this kind of error.

if isinstance(filename, Path):
filename = str(filename)

root = os.path.join(os.path.abspath(root), '')
filename = os.path.abspath(os.path.join(root, filename.strip('/\\')))
headers = headers.copy() if headers else {}
Expand Down Expand Up @@ -3976,7 +3989,10 @@ def __init__(self,
self.name = name
self.source = source.read() if hasattr(source, 'read') else source
self.filename = source.filename if hasattr(source, 'filename') else None
self.lookup = [os.path.abspath(x) for x in lookup] if lookup else []
if lookup:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, just call str on each value unconditionally. The check for an empty lookup variable could also be skipped, as a list comprehension over an empty iterator results in an empty list. This is not a hot path, so a little bit of overhead is fine, readability is more important.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, the default value of lookup is None, because of the common pitfall of Python's mutable types as default parameters. So we have to check if lookup is a falsy value here.

self.lookup = [os.path.abspath(str(x) if isinstance(x, Path) else x) for x in lookup]
else:
self.lookup = []
self.encoding = encoding
self.settings = self.settings.copy() # Copy from class variable
self.settings.update(settings) # Apply
Expand All @@ -3999,6 +4015,8 @@ def search(cls, name, lookup=None):
raise depr(0, 12, "Use of absolute path for template name.",
"Refer to templates with names or paths relative to the lookup path.")

lookup = [str(path) if isinstance(path, Path) else path for path in lookup]

for spath in lookup:
spath = os.path.abspath(spath) + os.sep
fname = os.path.abspath(os.path.join(spath, name))
Expand Down
6 changes: 6 additions & 0 deletions test/test_config.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import os
import tempfile
import unittest
from pathlib import Path
from bottle import ConfigDict


Expand Down Expand Up @@ -198,3 +199,8 @@ def test_load_config(self):
'namespace.section.default': 'otherDefault',
'namespace.section.sub.namespace.key': 'test2',
'port': '8080'}, c)

# Test with pathlib.Path object
c2 = ConfigDict()
c2.load_config(Path(self.config_file.name))
self.assertDictEqual(c, c2)
4 changes: 4 additions & 0 deletions test/test_sendfile.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import sys
import unittest
from pathlib import Path
from bottle import static_file, request, response, parse_date, parse_range_header, Bottle, tob
import bottle
import wsgiref.util
Expand Down Expand Up @@ -57,6 +58,9 @@ def test_valid(self):
out = static_file(basename, root=root)
self.assertEqual(open(__file__,'rb').read(), out.body.read())

out_path = static_file(Path(basename), root=Path(root))
self.assertEqual(open(__file__,'rb').read(), out_path.body.read())

def test_invalid(self):
""" SendFile: Invalid requests"""
self.assertEqual(404, static_file('not/a/file', root=root).status_code)
Expand Down
4 changes: 4 additions & 0 deletions test/test_stpl.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from bottle import SimpleTemplate, TemplateError, view, template, touni, tob, html_quote
import re, os
import traceback
from pathlib import Path
from .tools import chdir


Expand All @@ -24,6 +25,9 @@ def test_file(self):
with chdir(__file__):
t = SimpleTemplate(name='./views/stpl_simple.tpl', lookup=['.'])
self.assertRenders(t, 'start var end\n', var='var')
with chdir(__file__):
t = SimpleTemplate(name='./views/stpl_simple.tpl', lookup=[Path('.')])
self.assertRenders(t, 'start var end\n', var='var')

def test_name(self):
with chdir(__file__):
Expand Down