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

Defined location/href of Constructed Stylesheets #95

Open
manucorporat opened this issue May 26, 2019 · 17 comments
Open

Defined location/href of Constructed Stylesheets #95

manucorporat opened this issue May 26, 2019 · 17 comments
Labels
needs resolution Needs consensus/resolution before shipping

Comments

@manucorporat
Copy link

manucorporat commented May 26, 2019

Constructed Stylesheets use the same location (base path) as the document, this makes it impossible to use relative links inside, url(), or @import() when deploying reusable components.

This is an important problem to solve when using CSS that could be imported from a CDN or any non-determined path.

Let's say a web component that uses CSSStylesheet is imported from the following url:
https://unpkg.com/my-super-css-component/dist/index.js

And it contains some styles that will loaded using Constructable Stylesheets and adopted by some shadow-root:

.style {
  background: url('./background.png')
}

We would like the user-agent to load:
https://unpkg.com/my-super-css-component/dist/background.png

but instead it will load:
https://mydomain.com/path/to/page/background.png

Today, there is not a good solution for this problem, since:

  1. Making the path absolute url('/my-super-css-component/dist/background.png') would still load the resource from the wrong origin.

  2. It's not always possible to hard code the full deployment URL, since it's likely to change or being deployment in mutable places (different paths and origins).

Proposal

When a CSSStylesheet is instantiated, it takes an optional href, that can be defined by JS to simulate the stylesheet has an path and origin different than the document, just like it was loaded using <link rel="stylesheet" href>:

const stylesheet = new CSSStylesheet({
   href: new URL('./cmp.css', import.meta.url)
});

shadowRoot.adoptedStyleSheets = [stylesheet];

This way the any relative paths, including @import and url() would resolve based in the CSSStylesheet's specified href.

@rakina
Copy link
Member

rakina commented May 28, 2019

Thanks for filing this. I think the addition makes sense, however I think we should be fine with just defining a base URL (so we don't need the './cmp.css' part in your example)?

(also, cc-ing @tabatkins @bzbarsky @domenic)

@bzbarsky
Copy link

My questions here revolve around spoofing and security risks. Presumably we'd want to only allow an href value that could be an href of a sheet loaded by this document (e.g. no file:// URL stylesheets attached to http:// documents)? What other restrictions might we need? What referrer is sent with the stylesheet subresource loads? Would this allow spoofing a not-same-site referrer?

If we just set the base URL, but keep the sheet URL as the document URL and ensure it's the thing that's used in all sorts of security checks, referrer headers, etc, etc, I think most of those issues don't arise.

@bzbarsky
Copy link

e.g. no file:// URL stylesheets attached to http:// documents

To expand on this, right now if you attach a file:// stylesheet, e.g. as a user sheet, to an http:// document, that file:// sheet can link to other file:// resources, for obvious reasons. So we would want to prevent being able to spoof a sheet being file://.

@manucorporat
Copy link
Author

Thanks for filing this. I think the addition makes sense, however I think we should be fine with just defining a base URL (so we don't need the './cmp.css' part in your example)?

yep, we don't need the "filename" as part of the full url resolution, i guess it could be named location, baseURL, or something else. I found it was easier to understand the problematic by making an analogy with `.

Presumably we'd want to only allow an href value that could be an href of a sheet loaded by this document (e.g. no file:// URL stylesheets attached to http:// documents)? What other restrictions might we need? What referrer is sent with the stylesheet subresource loads? Would this allow spoofing a not-same-site referrer?

yes, we should prevent file:// and follow CSP if specified, with respect the referrer i believe it should be the document's one. I think the least risky implementation is use the "location" to transform relative paths within the stylesheet into full urls, so:

const stylesheet = new CSSStyleSheet({
   location: 'https://unpkg.com/my-collection/dist/assets'
});
stylesheet.replace(`
body {
   background: url('./image.png')
}
`);

would be equivalent to:

const stylesheet = new CSSStyleSheet();
stylesheet.replace(`
body {
   background: url('https://unpkg.com/my-collection/dist/assets')
}
`);

But the referrer should still be the document's location.

@manucorporat
Copy link
Author

manucorporat commented May 28, 2019

In order to workaround this issue, I am building a PostCSS transform that injects certain comments at specific locations within the CSS text:

-  background: url('./image.png')
+  background: url(/*!baseURL*/'./image.png')

-  background: url('/image.png')
+  background: url(/*!origin*/'/image.png')

This way, at runtime, we can perfom a quick find-replace over the stylesheet text before parsing them with replace().

cssText = cssText
  .replace(/\/\*baseURL\*\//g, resourcesURL)
  .replace(/\/\*origin\*\//g, resourcesOrigin);

const stylesheet = new CSSStylesheet();
stylesheet.replace(cssText);

I believe this transform of relative paths to absolute URLs could be done internally by CSSStyleSheet without exposing any security risk

@bzbarsky
Copy link

I think the least risky implementation is use the "location" to transform relative paths within the stylesheet into full urls

That's exactly what the stylesheet base URL is used for, yes. At least in Gecko.

@manucorporat
Copy link
Author

@bzbarsky thanks cool! so exactly that for Constructable Stylesheets I guess :)

@rakina
Copy link
Member

rakina commented May 29, 2019

If we just set the base URL, but keep the sheet URL as the document URL and ensure it's the thing that's used in all sorts of security checks, referrer headers, etc, etc, I think most of those issues don't arise.

Yes, after reading stylesheet referrer and relative URL specs, it seems like it would be better to just leave the stylesheet's location as it was (the document's base URL, in the case of constructed stylesheets), and just change the relative url resolving to check for a constructed stylesheet's baseURL (new attribute?) if defined.

@manucorporat
Copy link
Author

@rakina would that option be passed down in the constructor?

new CSSStyleSheet({
  baseURL,
});

that would work perfect for us!

@rakina
Copy link
Member

rakina commented Jul 10, 2019

@rakina would that option be passed down in the constructor?

new CSSStyleSheet({
  baseURL,
});

that would work perfect for us!

Yep. cc @mfreed7 @chrishtr

@rakina rakina added enhancement New feature or request needs resolution Needs consensus/resolution before shipping and removed enhancement New feature or request labels Jan 20, 2020
@css-meeting-bot
Copy link

The CSS Working Group just discussed Defined location/href of Constructed Stylesheets, and agreed to the following:

  • RESOLVED: add base URL constructor argument for sole purpose of resolving relative URLs in stylesheet, and location of the stylesheet remains that of the document
The full IRC log of that discussion <stantonm> topic: Defined location/href of Constructed Stylesheets
<astearns> github: https://github.com//issues/95
<stantonm> heycam: regular non-constructable style sheets have url to resolve other stylesheets
<stantonm> with constructable, no facility to provide url
<stantonm> ... regular urls are resolved against document
<stantonm> ... you might want to be able to specify
<stantonm> TabAtkins: web component wants css to also be relative to some 3rd party url?
<stantonm> ... can't hardcode deployment url, how do you use to it to get relative url
<stantonm> ... when user says background.png, want it to use 3rd party load path - currently uses first party
<stantonm> heycam: read issue as today there's not a good way to do this
<stantonm> TabAtkins: splitting parts of URL, base part still might be hard to obtain
<stantonm> ... url is hardcoded somewhere...just elsewhere than your stylesheet pipeline
<stantonm> hober: probably fine to set base and location as specified
<stantonm> heycam: not speficying url of sheet itself
<stantonm> hober: yes, sheet url is same as document url
<stantonm> RESOLVED: add base URL constructor argument for sole purpose of resolving relative URLs in stylesheet, and location of the stylesheet remains that of the document
<TabAtkins> TabAtkins: And do we need to note the security concerns that bz raised, like with file:// etc?
<TabAtkins> hober: Those are addressed by instead doing this as just a relative-url resolver, and keeping the stylesheet location the same.
<TabAtkins> TabAtkins: Cool.

@nordzilla
Copy link
Contributor

nordzilla commented Mar 2, 2020

How should the baseURL be interpreted? Will the constructor options allow relative paths?

For example, if someone provides:

new CSSStyleSheet({ baseURL: "foo" })

could we resolve this to <document's location> + "/foo"? Or should we keep things simple and only allow full paths?

@tabatkins
Copy link
Contributor

tabatkins commented Mar 3, 2020

I think the right answer it to pass it through new URL(baseURL, null), and use the serialized result as the base URL for everything. (After sanitization, as described earlier in this issue.)

Unless others do think it's useful to resolve it as new URL(baseURL, document.location)?

@bzbarsky
Copy link

bzbarsky commented Mar 4, 2020

Or more likely as new URL(baseURL, document.baseURI) if we go that route.

@tabatkins
Copy link
Contributor

Right, thanks.

@domenic
Copy link
Contributor

domenic commented Mar 4, 2020

The majority of URL-accepting APIs resolve relative to the current settings object's API base URL. (I.e., document.baseURI.) The only exception I'm aware of that requires absolute URLs is the WebSocket constructor, and that because it's not possible to have a relative URL end up with the ws:/wss: scheme.

So, I'd encourage us to be consistent.

@tabatkins
Copy link
Contributor

Cool, I wasn't sure of the consistency. More than happy to be consistent there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs resolution Needs consensus/resolution before shipping
Projects
None yet
Development

No branches or pull requests

7 participants