[dds/devtools] Switch to always using a relative base href in DevTool…

…s pages

This avoids the server needing to know the exact URL path being used on the client, so proxies that rewrite the path (such as code-server) can still work.

See flutter/devtools#8067 (comment)

Change-Id: I5b131d84232c63f5495ccbb533da727cce514de5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/376682
Reviewed-by: Kenzie Davisson <[email protected]>
Commit-Queue: Ben Konyi <[email protected]>
Reviewed-by: Ben Konyi <[email protected]>

pkg/dds/lib/src/devtools/handler.dart

@@ -224,6 +224,15 @@ Future<Response> _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'<base href="\/"\s?\/?>');
     contents = contents.replaceFirst(
@@ -233,3 +242,16 @@ Future<Response> _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);
+}

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);
+    });
+  }
+}

pkg/dds/test/devtools_server/devtools_server_path_strategy_dds_test.dart

@@ -18,15 +18,21 @@ void main() {
       'Future<void> 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('<base href="([^"]+)"');
 
-  test('serves index.html contents for /token/devtools/inspector', () async {
-    final testFile = File(path.join(tempDir.path, 'foo.dart'));
-    testFile.writeAsStringSync(testScriptContents);
+  group('serves index.html', () {
+    Process? process;
+    late Uri devToolsUrl;
+    final httpClient = HttpClient();
+
+    setUpAll(() async {
+      final testFile = File(path.join(tempDir.path, 'foo.dart'));
+      testFile.writeAsStringSync(testScriptContents);
+
+      final proc = process = await Process.start(
+          Platform.resolvedExecutable, ['--observe=0', testFile.path]);
 
-    final proc = await Process.start(
-        Platform.resolvedExecutable, ['--observe=0', testFile.path]);
-    try {
       final completer = Completer<String>();
       proc.stderr
           .transform(utf8.decoder)
@@ -51,24 +57,50 @@ void main() {
           }
         },
       );
+      devToolsUrl = Uri.parse(await completer.future);
+    });
+
+    tearDownAll(() {
+      httpClient.close(force: true);
+      process?.kill();
+    });
 
-      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();
+    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('<base href="$expectedBaseHref">'));
-      } finally {
-        httpClient.close();
-      }
-    } finally {
-      proc.kill();
+        expect(bodyContent, contains('<base href="'));
+
+        // 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));
 }

pkg/dds/test/devtools_server/devtools_server_path_strategy_test.dart

@@ -13,6 +13,7 @@ late final DevToolsServerTestController testController;
 
 void main() {
   testController = DevToolsServerTestController();
+  final baseHrefRegex = RegExp('<base href="([^"]+)"');
 
   setUp(() async {
     await testController.setUp();
@@ -22,49 +23,71 @@ void main() {
     await testController.tearDown();
   });
 
-  test('serves index.html contents for /inspector', () async {
-    final server = await DevToolsServerDriver.create();
+  group('serves index.html', () {
+    DevToolsServerDriver? server;
+    late Uri devToolsUrl;
     final httpClient = HttpClient();
-    late HttpClientResponse resp;
-    try {
-      final startedEvent = (await server.stdout.firstWhere(
+
+    setUpAll(() async {
+      server = await DevToolsServerDriver.create();
+      final startedEvent = (await server!.stdout.firstWhere(
         (map) => map!['event'] == 'server.started',
       ))!;
       final host = startedEvent['params']['host'];
       final port = startedEvent['params']['port'];
+      devToolsUrl = Uri(scheme: 'http', host: host, port: port);
+    });
+
+    tearDownAll(() {
+      httpClient.close(force: true);
+      server?.kill();
+    });
 
-      final req = await httpClient.get(host, port, '/inspector');
-      resp = await req.close();
+    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('<base href="$expectedBaseHref">'));
-    } 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));
 }