From 17dcab8f83f38ccea761e3c778666646d152b9a6 Mon Sep 17 00:00:00 2001 From: Robert Ma Date: Tue, 6 Oct 2020 17:12:32 -0400 Subject: [PATCH 1/2] [wptserve] Make .headers work with generated tests Honour *.headers (including *.sub.headers and __dir__{.sub}.headers) when serving tests automatically generated from *.js files. The documentation doesn't mention this exception and reasonable people would assume *.headers work the same way as with static files. Add a functional test, too. Fixes https://crbug.com/1135559. Drive-by: some stylistic changes and docstrings. --- tools/serve/serve.py | 10 ++- tools/wptserve/tests/functional/base.py | 26 ++++--- .../tests/functional/test_handlers.py | 21 +++-- tools/wptserve/wptserve/handlers.py | 78 ++++++++++++------- 4 files changed, 88 insertions(+), 47 deletions(-) diff --git a/tools/serve/serve.py b/tools/serve/serve.py index 8243acc5eb8ec2..b1b9e931317f4e 100644 --- a/tools/serve/serve.py +++ b/tools/serve/serve.py @@ -73,7 +73,9 @@ def __call__(self, request, response): self.handler(request, response) def handle_request(self, request, response): - for header_name, header_value in self.headers: + headers = self.headers + handlers.load_headers( + request, self._get_filesystem_path(request)) + for header_name, header_value in headers: response.headers.set(header_name, header_value) self.check_exposure(request) @@ -111,13 +113,17 @@ def _get_path(self, path, resource_path): path = replace_end(path, src, dest) return path + def _get_filesystem_path(self, request): + """Get the path of the underlying resource file on disk.""" + return self._get_path(filesystem_path(self.base_path, request, self.url_base), False) + def _get_metadata(self, request): """Get an iterator over script metadata based on // META comments in the associated js file. :param request: The Request being processed. """ - path = self._get_path(filesystem_path(self.base_path, request, self.url_base), False) + path = self._get_filesystem_path(request) try: with open(path, "rb") as f: for key, value in read_script_metadata(f, js_meta_re): diff --git a/tools/wptserve/tests/functional/base.py b/tools/wptserve/tests/functional/base.py index 8b34eb7cd0cdfb..d3ffff61762601 100644 --- a/tools/wptserve/tests/functional/base.py +++ b/tools/wptserve/tests/functional/base.py @@ -85,6 +85,7 @@ def assert_multiple_headers(self, resp, name, values): else: assert resp.info()[name] == ", ".join(values) + @pytest.mark.skipif(not wptserve.utils.http2_compatible(), reason="h2 server only works in python 2.7.15") class TestUsingH2Server: def setup_method(self, test_method): @@ -114,36 +115,39 @@ class TestWrapperHandlerUsingServer(TestUsingServer): the html file. This class extends the TestUsingServer and do some some extra work: it tries to generate the dummy .js file in setUp and remove it in tearDown.''' - dummy_js_files = {} + dummy_files = {} - def gen_js_file(self, filename, empty=True, content=b''): - self.remove_js_file(filename) + def gen_file(self, filename, empty=True, content=b''): + self.remove_file(filename) with open(filename, 'wb') as fp: if not empty: fp.write(content) - def remove_js_file(self, filename): + def remove_file(self, filename): if os.path.exists(filename): os.remove(filename) def setUp(self): super(TestWrapperHandlerUsingServer, self).setUp() - for filename, content in self.dummy_js_files.items(): + for filename, content in self.dummy_files.items(): filepath = os.path.join(doc_root, filename) if content == '': - self.gen_js_file(filepath) + self.gen_file(filepath) else: - self.gen_js_file(filepath, False, content) + self.gen_file(filepath, False, content) - def run_wrapper_test(self, req_file, header_data, wrapper_handler): + def run_wrapper_test(self, req_file, content_type, wrapper_handler, + headers=None): route = ('GET', req_file, wrapper_handler()) self.server.router.register(*route) resp = self.request(route[1]) self.assertEqual(200, resp.getcode()) - self.assertEqual(header_data, resp.info()['Content-Type']) + self.assertEqual(content_type, resp.info()['Content-Type']) + for key, val in headers or []: + self.assertEqual(val, resp.info()[key]) with open(os.path.join(doc_root, req_file), 'rb') as fp: self.assertEqual(fp.read(), resp.read()) @@ -151,6 +155,6 @@ def run_wrapper_test(self, req_file, header_data, wrapper_handler): def tearDown(self): super(TestWrapperHandlerUsingServer, self).tearDown() - for filename, _ in self.dummy_js_files.items(): + for filename, _ in self.dummy_files.items(): filepath = os.path.join(doc_root, filename) - self.remove_js_file(filepath) + self.remove_file(filepath) diff --git a/tools/wptserve/tests/functional/test_handlers.py b/tools/wptserve/tests/functional/test_handlers.py index 965a214f166cfc..7866e641e6aa2d 100644 --- a/tools/wptserve/tests/functional/test_handlers.py +++ b/tools/wptserve/tests/functional/test_handlers.py @@ -13,6 +13,7 @@ from serve import serve + class TestFileHandler(TestUsingServer): def test_GET(self): resp = self.request("/document.txt") @@ -388,8 +389,8 @@ def test_requesting_multiple_resources(self): class TestWorkersHandler(TestWrapperHandlerUsingServer): - dummy_js_files = {'foo.worker.js': b'', - 'foo.any.js': b''} + dummy_files = {'foo.worker.js': b'', + 'foo.any.js': b''} def test_any_worker_html(self): self.run_wrapper_test('foo.any.worker.html', @@ -401,7 +402,7 @@ def test_worker_html(self): class TestWindowHandler(TestWrapperHandlerUsingServer): - dummy_js_files = {'foo.window.js': b''} + dummy_files = {'foo.window.js': b''} def test_window_html(self): self.run_wrapper_test('foo.window.html', @@ -409,15 +410,19 @@ def test_window_html(self): class TestAnyHtmlHandler(TestWrapperHandlerUsingServer): - dummy_js_files = {'foo.any.js': b''} + dummy_files = {'foo.any.js': b'', + 'foo.any.js.headers': b'X-Foo: 1', + '__dir__.headers': b'X-Bar: 2'} def test_any_html(self): self.run_wrapper_test('foo.any.html', - 'text/html', serve.AnyHtmlHandler) + 'text/html', + serve.AnyHtmlHandler, + headers=[('X-Foo', '1'), ('X-Bar', '2')]) class TestSharedWorkersHandler(TestWrapperHandlerUsingServer): - dummy_js_files = {'foo.any.js': b'// META: global=sharedworker\n'} + dummy_files = {'foo.any.js': b'// META: global=sharedworker\n'} def test_any_sharedworkers_html(self): self.run_wrapper_test('foo.any.sharedworker.html', @@ -425,7 +430,7 @@ def test_any_sharedworkers_html(self): class TestServiceWorkersHandler(TestWrapperHandlerUsingServer): - dummy_js_files = {'foo.any.js': b'// META: global=serviceworker\n'} + dummy_files = {'foo.any.js': b'// META: global=serviceworker\n'} def test_serviceworker_html(self): self.run_wrapper_test('foo.any.serviceworker.html', @@ -433,7 +438,7 @@ def test_serviceworker_html(self): class TestAnyWorkerHandler(TestWrapperHandlerUsingServer): - dummy_js_files = {'bar.any.js': b''} + dummy_files = {'bar.any.js': b''} def test_any_work_js(self): self.run_wrapper_test('bar.any.worker.js', 'text/javascript', diff --git a/tools/wptserve/wptserve/handlers.py b/tools/wptserve/wptserve/handlers.py index 6677ab394b7a31..4ce37abaafdec8 100644 --- a/tools/wptserve/wptserve/handlers.py +++ b/tools/wptserve/wptserve/handlers.py @@ -32,7 +32,6 @@ def guess_content_type(path): return "application/octet-stream" - def filesystem_path(base_path, request, url_base="/"): if base_path is None: base_path = request.doc_root @@ -53,6 +52,7 @@ def filesystem_path(base_path, request, url_base="/"): return new_path + class DirectoryHandler(object): def __init__(self, base_path=None, url_base="/"): self.base_path = base_path @@ -121,6 +121,7 @@ def list_items(self, base_path, path): {"link": link, "name": escape(item), "class": class_, "headers": dot_headers_markup}) + def parse_qs(qs): """Parse a query string given as a string argument (data of type application/x-www-form-urlencoded). Data are returned as a dictionary. The @@ -140,7 +141,12 @@ def parse_qs(qs): rv[name].append(value) return dict(rv) + def wrap_pipeline(path, request, response): + """Applies pipelines to a response. + + Pipelines are specified in the filename (.sub.) or the query param (?pipe). + """ query = parse_qs(request.url_parts.query) pipe_string = "" @@ -161,6 +167,36 @@ def wrap_pipeline(path, request, response): return response +def load_headers(request, path): + """Loads headers from files for a given path. + + Attempt to load both the neighbouring __dir__{.sub}.headers and + PATH{.sub}.headers (apply template substituition if needed); results are + concatenated in this order. + """ + def _load(request, path): + headers_path = path + ".sub.headers" + if os.path.exists(headers_path): + use_sub = True + else: + headers_path = path + ".headers" + use_sub = False + + try: + with open(headers_path, "rb") as headers_file: + data = headers_file.read() + except IOError: + return [] + else: + if use_sub: + data = template(request, data, escape_type="none") + return [tuple(item.strip() for item in line.split(b":", 1)) + for line in data.splitlines() if line] + + return (_load(request, os.path.join(os.path.dirname(path), "__dir__")) + + _load(request, path)) + + class FileHandler(object): def __init__(self, base_path=None, url_base="/"): self.base_path = base_path @@ -197,33 +233,13 @@ def __call__(self, request, response): raise HTTPException(404) def get_headers(self, request, path): - rv = (self.load_headers(request, os.path.join(os.path.dirname(path), "__dir__")) + - self.load_headers(request, path)) + rv = load_headers(request, path) if not any(key.lower() == b"content-type" for (key, _) in rv): rv.insert(0, (b"Content-Type", guess_content_type(path).encode("ascii"))) return rv - def load_headers(self, request, path): - headers_path = path + ".sub.headers" - if os.path.exists(headers_path): - use_sub = True - else: - headers_path = path + ".headers" - use_sub = False - - try: - with open(headers_path, "rb") as headers_file: - data = headers_file.read() - except IOError: - return [] - else: - if use_sub: - data = template(request, data, escape_type="none") - return [tuple(item.strip() for item in line.split(b":", 1)) - for line in data.splitlines() if line] - def get_data(self, response, path, byte_ranges): """Return either the handle to a file, or a string containing the content of a chunk of the file, if we have a range request.""" @@ -312,7 +328,6 @@ def func(request, response, environ, path): self._set_path_and_load_file(request, response, func) - def frame_handler(self, request): """ This creates a FunctionHandler with one or more of the handling functions. @@ -340,8 +355,10 @@ def _main(req, resp): return handler return self._set_path_and_load_file(request, None, func) + python_script_handler = PythonScriptHandler() + class FunctionHandler(object): def __init__(self, func): self.func = func @@ -370,10 +387,11 @@ def __call__(self, request, response): wrap_pipeline('', request, response) -#The generic name here is so that this can be used as a decorator +# The generic name here is so that this can be used as a decorator def handler(func): return FunctionHandler(func) + class JsonHandler(object): def __init__(self, func): self.func = func @@ -395,9 +413,11 @@ def handle_request(self, request, response): response.headers.set("Content-Length", length) return value + def json_handler(func): return JsonHandler(func) + class AsIsHandler(object): def __init__(self, base_path=None, url_base="/"): self.base_path = base_path @@ -414,8 +434,10 @@ def __call__(self, request, response): except IOError: raise HTTPException(404) + as_is_handler = AsIsHandler() + class BasicAuthHandler(object): def __init__(self, handler, user, password): """ @@ -442,8 +464,10 @@ def __call__(self, request, response): return response return self.handler(request, response) + basic_auth_handler = BasicAuthHandler(file_handler, None, None) + class ErrorHandler(object): def __init__(self, status): self.status = status @@ -454,7 +478,7 @@ def __call__(self, request, response): class StringHandler(object): def __init__(self, data, content_type, **headers): - """Hander that reads a file from a path and substitutes some fixed data + """Handler that returns a fixed data string and headers :param data: String to use :param content_type: Content type header to server the response with @@ -478,7 +502,9 @@ def __call__(self, request, response): class StaticHandler(StringHandler): def __init__(self, path, format_args, content_type, **headers): - """Hander that reads a file from a path and substitutes some fixed data + """Handler that reads a file from a path and substitutes some fixed data + + Note that *.headers files have no effect in this handler. :param path: Path to the template file to use :param format_args: Dictionary of values to substitute into the template file From d344c2783cf8dfc34dc4a36e51f21dbf86936bc4 Mon Sep 17 00:00:00 2001 From: Robert Ma Date: Wed, 7 Oct 2020 17:25:32 -0400 Subject: [PATCH 2/2] Fix docstrings Co-authored-by: Stephen McGruer --- tools/wptserve/wptserve/handlers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/wptserve/wptserve/handlers.py b/tools/wptserve/wptserve/handlers.py index 4ce37abaafdec8..d1e9614c031279 100644 --- a/tools/wptserve/wptserve/handlers.py +++ b/tools/wptserve/wptserve/handlers.py @@ -170,9 +170,9 @@ def wrap_pipeline(path, request, response): def load_headers(request, path): """Loads headers from files for a given path. - Attempt to load both the neighbouring __dir__{.sub}.headers and - PATH{.sub}.headers (apply template substituition if needed); results are - concatenated in this order. + Attempts to load both the neighbouring __dir__{.sub}.headers and + PATH{.sub}.headers (applying template substitution if needed); results are + concatenated in that order. """ def _load(request, path): headers_path = path + ".sub.headers"