kit icon indicating copy to clipboard operation
kit copied to clipboard

Invalid css file name (for url path) generated in catch-all route.

Open eahrold opened this issue 3 years ago • 1 comments

Describe the bug

When a wild card route is used, and the application is build with the adapter-node the resulting .css file has square brackets in the file name which should be escaped.

This is not a problem with serving with node directly, however when proxied through some non-js routers the route is considered invalid, as per various specs. (i.e. I'm serving the node application in a docker container, and have a proxy server intercepting requests and forwarding them to the node server)

and if you pass the route through encodeURI it too escapes the brackets

encodeURI("http://localhost:3000/_app/immutable/assets/[...allRoutes]-b0a89eda.css")

// http://localhost:3000/_app/immutable/assets/%5B...allRoutes%5D-b0a89eda.css

Demo of the URL format when used in Java

Reproduction

https://github.com/eahrold/svelte-kit-invalid-css-path

The problem .css file is here https://github.com/eahrold/svelte-kit-invalid-css-path/tree/main/build/client/_app/immutable/assets

The corresponding .js file works and looks to be processed differently. https://github.com/eahrold/svelte-kit-invalid-css-path/tree/main/build/client/_app/immutable/pages

The browser request doesn't encode the url

invalid-route-used-directly

Logs

No response

System Info

System:
    OS: macOS 12.2
    CPU: (8) arm64 Apple M1
    Memory: 97.22 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.15.1 - ~/.nvm/versions/node/v16.15.1/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v16.15.1/bin/yarn
    npm: 8.11.0 - ~/.nvm/versions/node/v16.15.1/bin/npm
  Browsers:
    Chrome: 104.0.5112.79
    Firefox: 102.0.1
    Safari: 15.3
  npmPackages:
    @sveltejs/adapter-node: next => 1.0.0-next.85 
    @sveltejs/kit: next => 1.0.0-next.403 
    svelte: ^3.44.0 => 3.49.0 
    vite: ^3.0.0 => 3.0.4

Severity

serious, but I can work around it

Additional Information

No response

eahrold avatar Aug 06 '22 18:08 eahrold

This was previously happening with JS files, but was fixed for that case: https://github.com/sveltejs/kit/issues/1974#issuecomment-1023471917

Anyway, this likely isn't generated by SvelteKit, but by Vite. Please file the issue there if you'd like it to be seen by anyone likely to fix it: https://github.com/sveltejs/kit#bug-reporting

benmccann avatar Aug 08 '22 23:08 benmccann

@benmccann did you mean to put the sveltekt bug-reporting link above? Also I couldn't quite suss out the PR for the JS fix, did that happen on the vite side? Can you provide any more pointers to where I should look?

eahrold avatar Aug 12 '22 12:08 eahrold

@eahrold that is the link I mean to share. Did you read what's there? It describes how to report issues to Vite that affect SvelteKit projects. I'm not exactly sure which PR changed the behavior in Vite for JS files

It could also be from Rollup. Understanding whether it's Vite or Rollup responsible for this logic would be a good first step

benmccann avatar Aug 12 '22 16:08 benmccann

I'm also running into this now. With the new routing, all route related files are prepended with + which would need encoding to be valid in a URL. Vite transforms +page.js to _page.js on build, but doesn't seem to do the same with CSS.

My project works fine in dev and preview, but when I deploy to Cloudfront and S3 I get error 404 on all route related CSS files, such as +layout.css, as they still have the + in them. Manually replacing the + with %2b in the URL serves the correct files.

@eahrold Did you find any related issue on the Vite repo? Or know of any workaround?

denovodavid avatar Aug 18 '22 05:08 denovodavid

We've narrowed it down to this function in Vite, which is supplying an invalid filename to Rollup (which has no choice but to use the supplied fileName verbatim). I think we just need to replace all special characters here with underscores to mirror how Rollup handles renaming for JS modules:

const name = basename.slice(0, -extname.length)

Rich-Harris avatar Aug 18 '22 14:08 Rich-Harris

I made a quick workaround for my case using vite's plugin system to override the config sveltekit sets. Add to vite.config plugins after sveltekit. This only fixes + prefixed css assets but could be adapted easily for other cases.


/** @type {import('vite').UserConfig} */
const config = {

  plugins: [
    sveltekit(),

    // <workaround for https://github.com/sveltejs/kit/issues/5843>
    {
      config(config) {
        const original = config.build.rollupOptions.output.assetFileNames;
        config.build.rollupOptions.output.assetFileNames = assetInfo => {
          const match = assetInfo.name.match(/\/\+(.*)\.css$/);
          return match ? original.replace('[name]', match[1]) : original;
        };
        return config;
      }
    },
    // </workaround>

  ]
};

mattosborn avatar Aug 19 '22 03:08 mattosborn

Can this issue be closed or is the bug still relevant?

JanTrichter avatar Aug 25 '22 21:08 JanTrichter

The fix in Vite has not been released yet. We should keep this open until we confirm that the fix works.

Conduitry avatar Aug 25 '22 21:08 Conduitry

I made a quick workaround for my case using vite's plugin system to override the config sveltekit sets. Add to vite.config plugins after sveltekit. This only fixes + prefixed css assets but could be adapted easily for other cases.

/** @type {import('vite').UserConfig} */
const config = {

  plugins: [
    sveltekit(),

    // <workaround for https://github.com/sveltejs/kit/issues/5843>
    {
      config(config) {
        const original = config.build.rollupOptions.output.assetFileNames;
        config.build.rollupOptions.output.assetFileNames = assetInfo => {
          const match = assetInfo.name.match(/\/\+(.*)\.css$/);
          return match ? original.replace('[name]', match[1]) : original;
        };
        return config;
      }
    },
    // </workaround>

  ]
};

Hi @mattosborn, I'm getting below error when trying to run the application locally

error when starting dev server:
TypeError: Cannot read properties of undefined (reading 'assetFileNames')
    at Object.config (file:///home/souravjamwal77/Desktop/projects/truviss-svelte/vite.config.js.timestamp-1661601113233.mjs:8:61)
    at resolveConfig (file:///home/souravjamwal77/Desktop/projects/truviss-svelte/node_modules/vite/dist/node/chunks/dep-0fc8e132.js:62629:33)
    at async createServer (file:///home/souravjamwal77/Desktop/projects/truviss-svelte/node_modules/vite/dist/node/chunks/dep-0fc8e132.js:59183:20)
    at async CAC.<anonymous> (file:///home/souravjamwal77/Desktop/projects/truviss-svelte/node_modules/vite/dist/node/cli.js:699:24)

It's coming from Vite config.

souravjamwal77 avatar Aug 27 '22 11:08 souravjamwal77

@souravjamwal77 the issue with the invalid file names only happens during build and not during development, so the workaround should only be applied during build:

{
  config(config) {
    const original = config.build.rollupOptions.output.assetFileNames;
    config.build.rollupOptions.output.assetFileNames = (assetInfo) => {
    const match = assetInfo.name.match(/\/\+(.*)\.css$/);
    return match ? original.replace('[name]', match[1]) : original;
  };
  return config;
  },
  apply: 'build'
}

stephane-vanraes avatar Aug 29 '22 06:08 stephane-vanraes