WICG / construct-stylesheets

API for constructing CSS stylesheet objects
Other
138 stars 25 forks source link

Defined location/href of Constructed Stylesheets #95

Open manucorporat opened 5 years ago

manucorporat commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

rakina commented 5 years ago

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 commented 5 years ago

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

new CSSStyleSheet({
  baseURL,
});

that would work perfect for us!

rakina commented 5 years ago

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

new CSSStyleSheet({
  baseURL,
});

that would work perfect for us!

Yep. cc @mfreed7 @chrishtr

css-meeting-bot commented 4 years ago

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

The full IRC log of that discussion <stantonm> topic: Defined location/href of Constructed Stylesheets
<astearns> github: https://github.com/WICG/construct-stylesheets/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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

tabatkins commented 4 years ago

Right, thanks.

domenic commented 4 years ago

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 commented 4 years ago

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