diff --git a/pkg/dds/lib/src/devtools/handler.dart b/pkg/dds/lib/src/devtools/handler.dart index ef95f811c4c..32a058fc3f8 100644 --- a/pkg/dds/lib/src/devtools/handler.dart +++ b/pkg/dds/lib/src/devtools/handler.dart @@ -224,6 +224,15 @@ Future _serveStaticFile( if (baseHref != null) { assert(baseHref.startsWith('/')); assert(baseHref.endsWith('/')); + + // Always use a relative base href to support going through proxies that + // rewrite paths. For example if the server thinks we are hosting at + // `http://localhost/devtools/` but a frontend proxy means the client app + // is at `http://localhost/proxy/1234/devtools/` then we need the base href + // to effectively be `/proxy/1234/devtools/`, however we have no knowledge + // of `/proxy/1234` because it was stripped by the proxy on its way to us. + baseHref = computeRelativeBaseHref(baseHref, request.requestedUri); + // Replace the base href to match where the app is being served from. final baseHrefPattern = RegExp(r''); contents = contents.replaceFirst( @@ -233,3 +242,16 @@ Future _serveStaticFile( } return Response.ok(contents, headers: headers); } + +/// Computes a relative "base href" that can be used in place of +/// [absoluteBaseHref] for a request served at [requestUri]. +String computeRelativeBaseHref(String absoluteBaseHref, Uri requestUri) { + // path.relative will always treat `from` as if it's a directory, but for + // URIs that's not correct. If the request is /foo/bar then `.` is `/foo` and + // not `/foo/bar`. To handle this, trim the last segment if the request does + // not end with a slash. + final requestFolderPath = requestUri.path.endsWith('/') + ? requestUri.path + : path.posix.dirname(requestUri.path); + return path.posix.relative(absoluteBaseHref, from: requestFolderPath); +} diff --git a/pkg/dds/test/devtools_server/base_href_test.dart b/pkg/dds/test/devtools_server/base_href_test.dart new file mode 100644 index 00000000000..c5bb6b44164 --- /dev/null +++ b/pkg/dds/test/devtools_server/base_href_test.dart @@ -0,0 +1,54 @@ +// Copyright 2024 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:dds/src/devtools/handler.dart'; +import 'package:test/test.dart'; + +void main() { + /// A set of test cases to verify base hrefs for. + /// + /// The key is a suffix appended to / or /devtools/ depending on how DevTools + /// is hosted. + /// The value is the expected base href (which should always resolve back to + /// the root of where DevTools is being hosted). + final testBaseHrefs = { + '': '.', + 'inspector': '.', + 'inspector/': '..', + 'inspector/foo': '..', + 'inspector/foo/': '../..', + 'inspector/foo/bar': '../..', + 'inspector/foo/bar/baz': '../../..', + }; + + for (final MapEntry(key: suffix, value: expectedBaseHref) + in testBaseHrefs.entries) { + test('computes correct base href for /$suffix with devtools at root', + () async { + final actual = computeRelativeBaseHref( + '/', + Uri.parse('http://localhost/$suffix'), + ); + expect(actual, expectedBaseHref); + }); + + test('computes correct base href for /$suffix with devtools at /devtools/', + () async { + final actual = computeRelativeBaseHref( + '/devtools/', + Uri.parse('http://localhost/devtools/$suffix'), + ); + expect(actual, expectedBaseHref); + }); + + test('computes correct base href for /$suffix in a devtools extension', + () async { + final actual = computeRelativeBaseHref( + '/devtools/devtools_extension/foo/', + Uri.parse('http://localhost/devtools/devtools_extension/foo/$suffix'), + ); + expect(actual, expectedBaseHref); + }); + } +} diff --git a/pkg/dds/test/devtools_server/devtools_server_path_strategy_dds_test.dart b/pkg/dds/test/devtools_server/devtools_server_path_strategy_dds_test.dart index ac48d027517..2b61572de11 100644 --- a/pkg/dds/test/devtools_server/devtools_server_path_strategy_dds_test.dart +++ b/pkg/dds/test/devtools_server/devtools_server_path_strategy_dds_test.dart @@ -18,15 +18,21 @@ void main() { 'Future main() => Future.delayed(const Duration(minutes: 10));'; final tempDir = Directory.systemTemp.createTempSync('devtools_server.'); final devToolsBannerRegex = - RegExp(r'DevTools[\w\s]+at: (https?:.*\/devtools)'); + RegExp(r'DevTools[\w\s]+at: (https?:.*\/devtools/)'); + final baseHrefRegex = RegExp('(); proc.stderr .transform(utf8.decoder) @@ -51,24 +57,50 @@ void main() { } }, ); + devToolsUrl = Uri.parse(await completer.future); + }); - final devToolsUrl = Uri.parse('${await completer.future}/'); - final httpClient = HttpClient(); - late HttpClientResponse resp; - try { - final req = await httpClient.get(devToolsUrl.host, devToolsUrl.port, - '${devToolsUrl.path}/inspector'); - resp = await req.close(); + tearDownAll(() { + httpClient.close(force: true); + process?.kill(); + }); + + test('correct content for /token/devtools/', () async { + final uri = devToolsUrl; + final req = await httpClient.getUrl(uri); + final resp = await req.close(); + expect(resp.statusCode, 200); + final bodyContent = await resp.transform(utf8.decoder).join(); + expect(bodyContent, contains('Dart DevTools')); + }, timeout: const Timeout.factor(10)); + + /// A set of test cases to verify base hrefs for. + /// + /// The key is a suffix to go after /devtools/ in the URI. + /// The value is the expected base href (which should always resolve back to + /// `/devtools/` or in the case of an extension, the base of the extension). + final testBaseHrefs = { + '': '.', + 'inspector': '.', + // We can't test devtools_extensions here without having one set up, but + // their paths are also tested in `base_href_test.dart`. + }; + + for (final MapEntry(key: suffix, value: expectedBaseHref) + in testBaseHrefs.entries) { + test('with correct base href for /token/devtools/$suffix', () async { + final uri = Uri.parse('$devToolsUrl$suffix'); + final req = await httpClient.getUrl(uri); + final resp = await req.close(); expect(resp.statusCode, 200); final bodyContent = await resp.transform(utf8.decoder).join(); - expect(bodyContent, contains('Dart DevTools')); - final expectedBaseHref = htmlEscape.convert(devToolsUrl.path); - expect(bodyContent, contains('')); - } finally { - httpClient.close(); - } - } finally { - proc.kill(); + expect(bodyContent, contains(' map!['event'] == 'server.started', ))!; final host = startedEvent['params']['host']; final port = startedEvent['params']['port']; + devToolsUrl = Uri(scheme: 'http', host: host, port: port); + }); - final req = await httpClient.get(host, port, '/inspector'); - resp = await req.close(); + tearDownAll(() { + httpClient.close(force: true); + server?.kill(); + }); + + test('correct content for /inspector', () async { + final req = await httpClient.getUrl(devToolsUrl.resolve('/inspector')); + final resp = await req.close(); expect(resp.statusCode, 200); final bodyContent = await resp.transform(utf8.decoder).join(); expect(bodyContent, contains('Dart DevTools')); - final expectedBaseHref = htmlEscape.convert('/'); - expect(bodyContent, contains('')); - } finally { - httpClient.close(); - server.kill(); - } - }, timeout: const Timeout.factor(10)); - - test('serves 404 contents for requests that are not pages', () async { - final server = await DevToolsServerDriver.create(); - final httpClient = HttpClient(); - late HttpClientResponse resp; - try { - final startedEvent = (await server.stdout.firstWhere( - (map) => map!['event'] == 'server.started', - ))!; - final host = startedEvent['params']['host']; - final port = startedEvent['params']['port']; + }, timeout: const Timeout.factor(10)); + test('serves 404 for requests that are not pages', () async { // The index page is only served up for extension-less requests. - final req = await httpClient.get(host, port, '/inspector.html'); - resp = await req.close(); + final req = + await httpClient.getUrl(devToolsUrl.resolve('/inspector.html')); + final resp = await req.close(); expect(resp.statusCode, 404); - } finally { - httpClient.close(); await resp.drain(); - server.kill(); + }, timeout: const Timeout.factor(10)); + + /// A set of test cases to verify base hrefs for. + /// + /// The key is a suffix to go after /devtools/ in the URI. + /// The value is the expected base href (which should always resolve back to + /// `/devtools/` or in the case of an extension, the base of the extension). + final testBaseHrefs = { + '': '.', + 'inspector': '.', + // TODO(dantup): Is there a way we could verify extension URLs here? + // 'devtools_extensions/foo/': '.', + // 'devtools_extensions/foo/bar': '.', + // 'devtools_extensions/foo/bar/': '..', + // 'devtools_extensions/foo/bar/baz': '../..', + }; + + for (final MapEntry(key: suffix, value: expectedBaseHref) + in testBaseHrefs.entries) { + test('with correct base href for /$suffix', () async { + final req = await httpClient.getUrl(devToolsUrl.resolve('/inspector')); + final resp = await req.close(); + expect(resp.statusCode, 200); + final bodyContent = await resp.transform(utf8.decoder).join(); + + // Extract the base href so if the test failures, we get a simpler error + // than just the entire content. + final actualBaseHref = baseHrefRegex.firstMatch(bodyContent)!.group(1); + expect(actualBaseHref, htmlEscape.convert(expectedBaseHref)); + }, timeout: const Timeout.factor(10)); } }, timeout: const Timeout.factor(10)); }