Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify the rules/requirements around the base href tag in web index.html #154620

Open
DanTup opened this issue Sep 4, 2024 · 14 comments · May be fixed by flutter/website#11163
Open

Clarify the rules/requirements around the base href tag in web index.html #154620

DanTup opened this issue Sep 4, 2024 · 14 comments · May be fixed by flutter/website#11163
Labels
P2 Important issues not at the top of the work list platform-web Web applications specifically team-web Owned by Web platform team tool Affects the "flutter" command-line tool. See also t: labels. triaged-web Triaged by Web platform team

Comments

@DanTup
Copy link
Contributor

DanTup commented Sep 4, 2024

I was trying to fix an issue that occurs when using DevTools through a proxy that runs multiple backend hosts through a single hostname, using paths to separate them. For example the address http://localhost:1234/ becomes http://localhost:8888/proxy/1234.

Since the proxy rewrites the request for /proxy/1234 back to /, the backend web server does not know that the path (according to the users browser) is /proxy/1234 and therefore cannot provide a correct absolute base href. However, providing a relative base href of . would be correct (or if the request was for a nested route, something like ../).

In my testing, this is working fine in DevTools, however the relative base href is always "." (because the DevTools pages are aways at the root right now) so I don't know whether it works generally.

However, there is code and comments in this repo suggesting Flutter requires the base href to always begin and end with a slash:

However I can't find info about why this is required (or more importantly, if it is, whether it could be fixed). On the surface, it feels like an unnecessary limitation (at least for the actual base href - I can understand the argument to build being more strict if it wants to avoid mishaps).

Is it possible to clarify these rules somewhere (and confirm whether it really is necessary)?

@darshankawar darshankawar added in triage Presently being triaged by the triage team tool Affects the "flutter" command-line tool. See also t: labels. platform-web Web applications specifically team-tool Owned by Flutter Tool team and removed in triage Presently being triaged by the triage team labels Sep 5, 2024
@DanTup
Copy link
Contributor Author

DanTup commented Sep 5, 2024

I added a relative base href of ./foo/bar/../../ (which actually just resolve to the current folder) and served DevTools app at a subfolder using a small proxy script. Everything appears to still work correctly:

relative_base_href.mp4

I don't know if this necessarily means there are no bugs for any kinds of routing, but on the surface it seems to me that relative base hrefs are working fine. They solve an issue for DevTools with some web IDEs that proxy by rewriting paths (flutter/devtools#8067) but I'm a little nervous about going ahead with this without some further confirmation that this is ok.

@andrewkolos andrewkolos added team-web Owned by Web platform team and removed team-tool Owned by Flutter Tool team labels Sep 10, 2024
@harryterkelsen harryterkelsen added the assigned for triage issue is assigned to a domain expert for further triage label Sep 12, 2024
@mdebbar
Copy link
Contributor

mdebbar commented Sep 12, 2024

Does the app use PathUrlStrategy or the default HashUrlStrategy? (if the app has a url that contains # then it's probably the latter).

PathUrlStrategy:

With this strategy, if the Flutter app has routes, then it's important for the base href to be absolute. The reason is when we see a path that looks like this: /proxy/1234/page/abc, we need to know which part of this path is the server part (/proxy/1234) and which is the client part (/page/abc). That distinction is important for 2 reasons:

  1. To load all files and assets correctly from /proxy/1234/... and not from /proxy/1234/page/abc/....
  2. For Flutter to replace the path correctly when you navigate to /page/xyz.

HashUrlStrategy:

With this strategy you shouldn't have any of the issues mentioned above. The solution is to simply remove the <base> element from your index.html file (or you can also try setting href to "" or "." which have the same behavior IIRC).

@mdebbar mdebbar added waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds and removed assigned for triage issue is assigned to a domain expert for further triage labels Sep 12, 2024
@mdebbar mdebbar removed their assignment Sep 12, 2024
@DanTup
Copy link
Contributor Author

DanTup commented Sep 12, 2024

Does the app use PathUrlStrategy or the default HashUrlStrategy? (if the app has a url that contains # then it's probably the latter).

It uses PathUrlStrategy - you can see the URLs in the video above, they're updating fine as I move around.

With this strategy, if the Flutter app has routes, then it's important for the base href to be absolute.

It's not clear to me why this is, and that's what this issue is about. It appears to be working correctly, and it's not clear to me why (if it did need to be absolute), Flutter can't just resolve it in the same way that the browser does. Flutter isn't involved in the loading of the <script tag for the compiled Dart source code, that's handled by the browser and it does it just fine - but some of the code mentioned above suggests Flutter doesn't support relative base hrefs but there's no explanation about why (and in my testing, it's working).

So, I guess the request is initially for an explanation of why this can't work and/or where it doesn't work, so I can reproduce (and perhaps contribute a fix for) the issue.

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Sep 12, 2024
@DanTup
Copy link
Contributor Author

DanTup commented Sep 12, 2024

And just to be clear, I am talking about the base href being computed correctly by the server here. If the backend is serving up the contents of index.html at multiple levels (like / and /foo/ and /foo/bar), then it is the backends responsibility to correctly adjust the base href (for example returning ., ../, ../) accordingly, which it would already need to do for the Dart script to be loaded correctly by the browser if it existed in the root. A relative base href should be resolved relative to the URL that made for that HTML page content.

@mdebbar
Copy link
Contributor

mdebbar commented Sep 17, 2024

I'm afraid your example may seem to be working because the app routes are only one level deep (i.e. /performance, /cpu-profiler, etc).

  1. Does the relative base href continue to work if the app's paths were deeper? (e.g. /profiling/cpu)
  2. Does it work if the you add a / to the end? (i.e. /performance/)
  3. Does it work in the above cases when you refresh the page?

Please keep an eye on the network tab in the above cases and keep note of the URLs that the files are loaded from.

If my understanding is correct (BIG IF), then you'll see files loaded from /proxy/1234/profiling/index.html, etc.

@DanTup
Copy link
Contributor Author

DanTup commented Sep 17, 2024

Those URLs don't work for DevTools, because its routing is not set up for them. I'll try to make a smaller sample I can test these things with - though if you know of any existing sample app that is already set up with routing like this that I would test with, please let me know!

@mdebbar
Copy link
Contributor

mdebbar commented Sep 17, 2024

In order to repro the cases I mentioned, you can try the following sample:

import 'package:flutter/material.dart';
import 'package:flutter_web_plugins/url_strategy.dart';

void main() {
  usePathUrlStrategy();
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp();

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      debugShowCheckedModeBanner: false,
      initialRoute: '/home',
      routes: {
        // This is fine with a relative base href.
        '/home': (_) => HomePage(destination: '/profile/'),
        // This isn't fine because it has a trailing slash, adding one level of
        // nesting in the path.
        '/profile/': (_) => HomePage(destination: '/deep/path'),
        // This is also not fine because it adds nesting in the path.
        '/deep/path': (_) => HomePage(destination: '/home'),
      },
    );
  }
}

class HomePage extends StatelessWidget {
  const HomePage({required this.destination});

  final String destination;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: TextButton(
          onPressed: () {
            Navigator.of(context).pushNamed(destination);
          },
          child: Text('Next: `$destination`'),
        ),
      ),
    );
  }
}

You will also need to apply this patch in flutter/flutter to loosen the base href requirements in order to test this: #155320

Steps:

  1. Navigate to each of the routes (i.e. /home, /profile/, /deep/path).
  2. Refresh the page on each of the paths.
  3. Keep an eye on the Network tab to see what location the files are being downloaded from (e.g. flutter.js, etc).

The above doesn't have the /proxy/1234 prefix because that requires a custom server that strips out the prefix on each request. But the repro I provided should clarify the problem, and the same problem happens when you add the /proxy/1234 prefix.

@DanTup
Copy link
Contributor Author

DanTup commented Sep 18, 2024

@mdebbar thank you for the sample - although I'm unable to reproduce any issue with it. I pushed a copy of it here:

https://github.com/DanTup/repros_flutter_154620

I extracted the routes to lib/routes.dart and added a bunch of additional combinations.

I added a small web server script that serves the app up at /proxy/1234 but additionally serves index.html for anything in lib/routes.dart with a computed relative base href.

Even without your patch to Flutter, I can navigate the app fine, both if loaded at the default URL, and also if I hard-refresh so that the initial page is loaded from one of the nested routes.

relative.base.href.mp4

image

I can't think of any cases this repro doesn't cover - do you know if I've overlooked something?

@mdebbar
Copy link
Contributor

mdebbar commented Sep 18, 2024

Even without your patch to Flutter, I can navigate the app fine, both if loaded at the default URL, and also if I hard-refresh so that the initial page is loaded from one of the nested routes.

This tells me you are using an absolute path as your base href. I can't confirm since you didn't include web/index.html in your repro repo :)

If you try a relative base href in your index.html, Flutter should yell at you! I thought that was the core of your issue, or am I misunderstanding something?

My point is:

  1. If you use an absolute base href (i.e. starting with /) then Flutter will gladly accept it, and everything will work fine (as long as your base href is the correct one).
  2. If you use a relative base href (i.e. what you are asking for in this issue), then Flutter will reject it and ask for an absolute path. The reason we do this is because a relative base href doesn't work well with the PathUrlStrategy (as explained in the comments above).

@DanTup
Copy link
Contributor Author

DanTup commented Sep 18, 2024

This tells me you are using an absolute path as your base href. I can't confirm since you didn't include web/index.html in your repro repo :)

You just need to run the command in the readme (flutter create), the file is just the default, but the web server replaces the value with a relative path. You should be able to run the whole thing locally using the instructions in the readme, but here's a screenshot showing relative base hrefs for both the root and a nested (hard refreshed) page:

image

image

@DanTup
Copy link
Contributor Author

DanTup commented Sep 18, 2024

If you try a relative base href in your index.html, Flutter should yell at you! I thought that was the core of your issue, or am I misunderstanding something?

I'm rewriting the base href to relative in the web server. I don't think some of the code in your PR actually runs for this case, but what I'm trying to determine/clarify is whether this is allowed. There are places in Flutter that say it's not allowed, but it's not clear if they only apply to things like the template (which is modified during the build step), or whether they intended to also cover just runtime where the index.html file is being generated by something that knows how to handle it.

So I'm not asking about what can go in web/index.html so that flutter build works. I'm saying, if I run flutter build and produce an app, and then I host it somewhere that rewrites the base href to be relative (and does this correctly, taking into account the path of the incoming request to the web server), is this supported? (I know it works from the repro above, but I would like it to be supported, because I want to land this change in DevTools and not fear that it might break in future).

@mdebbar
Copy link
Contributor

mdebbar commented Sep 18, 2024

Oh my bad, I misunderstood what you were doing. I get it now. You are setting the base href on the fly when you serve index.html.

is this supported?

Server:

Relative base href is NOT supported with flutter run because the server there is basic and doesn't do base href rewriting on the fly, etc. So the restriction was made based on the fact that the index.html file is static. Same restriction applies if you use --base-href with flutter build because you would be making a static base href as far as we are concerned.

If you use your own server instead of flutter run, you can do whatever you want with base href. I suggest you remove <base href="*"> from web/index.html to make it clear that you aren't supplying a base href when building the app.

Client:

Our client side code should work just like any other web app when it comes to base href. We use it in flutter_bootstrap.js to load other files from the correct path. We also use it in the PathUrlStrategy to figure out how to update the URL when the app pushes or replaces a route.

On the client, relative vs absolute base href doesn't make a difference to us because the browser converts it to a full URL (try document.baseURI in Chrome DevTools console).

@DanTup
Copy link
Contributor Author

DanTup commented Sep 18, 2024

Oh my bad, I misunderstood what you were doing. I get it now. You are setting the base href on the fly when you serve index.html.

Yes, that's it! Sorry for any confusion 🙂

Relative base href is NOT supported with flutter run

Understood - this doesn't affect us here - flutter run is always from the root and the rewriting to relative base href would only occur on release builds being served through the DevTools server (which does the rewriting).

Our client side code should work just like any other web app when it comes to base href.

Great, this is what I hoped (and seemed to be the case in my testing), but the comments about it not being supported in some places made me worry whether this was a policy and could be applied to client/runtime code in future. Is it possible we could make this explicitly clear somewhere (perhaps the text at the bottom of https://docs.flutter.dev/ui/navigation/url-strategies#hosting-a-flutter-app-at-a-non-root-location) to indicate that relative base hrefs work in client apps but the servers must provide them correctly, taking into account the URL a page was loaded at? (I'm happy to open a PR for this and we can agree on the specific wording)?

Thanks for your help (and patience)!

@mdebbar
Copy link
Contributor

mdebbar commented Sep 18, 2024

Is it possible we could make this explicitly clear somewhere

Yes, we should!

(I'm happy to open a PR for this and we can agree on the specific wording)?

That would be amazing! Thank you!

@mdebbar mdebbar added P2 Important issues not at the top of the work list triaged-web Triaged by Web platform team labels Sep 18, 2024
DanTup added a commit to DanTup/flutter-website that referenced this issue Sep 19, 2024
…untime if appropriately formed

There are some places where a relative `base href` is not supported (and these are documented/validated), but there is nothing explicitly calling out that they are supported in the client app. This adds a note to solidify this so they can be used (for example in the DevTools server) without the worry of them being undocumented.

Fixes flutter/flutter#154620
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Important issues not at the top of the work list platform-web Web applications specifically team-web Owned by Web platform team tool Affects the "flutter" command-line tool. See also t: labels. triaged-web Triaged by Web platform team
Projects
None yet
5 participants