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

Wrong paths on Windows #42

Closed
mikeschepers4 opened this issue Nov 2, 2022 · 34 comments · Fixed by #45 or #84
Closed

Wrong paths on Windows #42

mikeschepers4 opened this issue Nov 2, 2022 · 34 comments · Fixed by #45 or #84
Labels
bug Something isn't working

Comments

@mikeschepers4
Copy link

mikeschepers4 commented Nov 2, 2022

Hello, i was trying to import the module inside my project but after importing the module i always seem to be getting the following error.
image

This error happens in my own project but also when i setup a completely empty nuxt project with nothing in it.

// https://v3.nuxtjs.org/api/configuration/nuxt.config
export default defineNuxtConfig({
  modules: ["nuxt-security"],
});
{
  "private": true,
  "scripts": {
    "build": "nuxt build",
    "dev": "nuxt dev",
    "generate": "nuxt generate",
    "preview": "nuxt preview",
    "postinstall": "nuxt prepare"
  },
  "devDependencies": {
    "nuxt": "3.0.0-rc.12"
  },
  "dependencies": {
    "nuxt-security": "^0.5.0"
  }
}
@Baroshem
Copy link
Owner

Baroshem commented Nov 2, 2022

Hey @mikeschepers4

Are you using Static Site Generation or SSR?

@Baroshem
Copy link
Owner

Baroshem commented Nov 2, 2022

This is really strange, I have just tested it out on a same project and it works (the only difference is that I am using yarn I believe)

image

Package.json:

{
  "private": true,
  "scripts": {
    "build": "nuxt build",
    "dev": "nuxt dev",
    "generate": "nuxt generate",
    "preview": "nuxt preview"
  },
  "devDependencies": {
    "nuxt": "3.0.0-rc.12"
  },
  "dependencies": {
    "nuxt-security": "^0.5.0"
  }
}

Nuxt.config.ts

export default defineNuxtConfig({
    modules: ['nuxt-security']
})

@mikeschepers4
Copy link
Author

It seems to happen both in ssr and static site generation. I tried to make a stackblitz but there it doesnt happen.

I think it might have something to do with the operating system since my error complained about a bad character sequence. (the error is in my screenshot above). Could windows have something to do with it?

@mikeschepers4
Copy link
Author

image
these are the lines that are causing the issue on my system

@mikeschepers4
Copy link
Author

mikeschepers4 commented Nov 2, 2022

adding .replace(/\\/g, '/')after every resolve call seems to fix the build. i will let you know if everything works as i expected

Edit: This seems to work, i can see the CSP headers and other security elements added to my headers. Im pretty sure that anyone using this on windows will get the same errors

@Baroshem
Copy link
Owner

Baroshem commented Nov 2, 2022

I see, quite strange. I will ask around in the nuxt community. From what you say, this bug can be caused by nuxt itself.

@danielroe
Copy link
Contributor

I would recommend using pathe instead of path which normalises windows paths for you.

@mikeschepers4
Copy link
Author

mikeschepers4 commented Nov 2, 2022

I think the problem lies in this file here the resolve methods dont seem to normalize the paths correctly for windows at least.

The output seems to be like this: C:\rovict\nuxt-app\node_modules\nuxt-security\dist\runtime\server\middleware\requestSizeLimiter

Whilst you need this output on windows: C:/rovict/nuxt-app/node_modules/nuxt-security/dist/runtime/server/middleware/requestSizeLimiter

@danielroe
Copy link
Contributor

Yes, exactly. It can be resolved in this project by using pathe instead of path. We can probably also normalise in addServerHandler etc. in kit.

@Baroshem
Copy link
Owner

Baroshem commented Nov 2, 2022

@danielroe so should I update the module itself to use pathe instead of path or will this be something included in the next version of kit?

@danielroe
Copy link
Contributor

danielroe commented Nov 2, 2022

You should use pathe in any case.

@Baroshem
Copy link
Owner

Baroshem commented Nov 2, 2022

Ok, so I will replace this for the next version. Thanks for the help!

@Baroshem
Copy link
Owner

Baroshem commented Nov 2, 2022

@mikeschepers4 I will release the new version in the upcoming days with this fix and few other smaller functionalities.

@mikeschepers4
Copy link
Author

Amazing! I'll keep an eye on it. Thanks for the quick response!

@Baroshem Baroshem mentioned this issue Nov 4, 2022
6 tasks
@Baroshem
Copy link
Owner

Baroshem commented Nov 4, 2022

Hey @mikeschepers4

I have released a new version with the fix for that. Could you please check if it works right now? :)

@mikeschepers4
Copy link
Author

mikeschepers4 commented Nov 4, 2022

@Baroshem Just updated the module to check. there still seems to be an issue. I'll look into this a bit deeper. (tested with a newly created nuxt app)
image

@Baroshem
Copy link
Owner

Baroshem commented Nov 4, 2022

@mikeschepers4

I see, maybe then changing the path to pathe is not enough. Let me know please after you do some more testing. Unfortunately I do not have a Windows device so I cannot really test it out (almost like blind coding). I can also release a quick patch version to make it work once we find the cause.

What if you wrap all the paths to the middlewares with normalize? Or just add this:

const runtimeDir = normalize(fileURLToPath(new URL('./runtime', import.meta.url)))

@Baroshem Baroshem reopened this Nov 4, 2022
@Baroshem
Copy link
Owner

Baroshem commented Nov 4, 2022

Or for each middleware replace handler: resolve(runtimeDir, 'server/middleware/rateLimiter') with handler: normalize(resolve(runtimeDir, 'server/middleware/rateLimiter'))

@Baroshem
Copy link
Owner

Baroshem commented Nov 7, 2022

Any news @mikeschepers4 ? 🙂

@mikeschepers4
Copy link
Author

Sorry ran out of time to test it last week. will test today :)

@mikeschepers4
Copy link
Author

Ok just tested but im getting the same error message with your suggestion. the path does seem to be correct if i log it however. will look into it a bit deeper

@mikeschepers4
Copy link
Author

Ive not been able to find a solution. I think it may have something to do with the way nitro is referencing the given paths. ill look further once i have some time again

@Baroshem
Copy link
Owner

Baroshem commented Nov 7, 2022

@danielroe Could you take a look at that? Maybe this is an issue in the nitro?

@Baroshem Baroshem added the bug Something isn't working label Nov 10, 2022
@Baroshem
Copy link
Owner

hey @mikeschepers4

Have you managed to test it out more?

@mikeschepers4
Copy link
Author

Sorry no. I think a potential fix would be to make use of relative paths instead of absolute paths. But i dont know how easy that would be to integrate in this solution.

Or Nitro needs to make a fix to handle absolute paths correctly on windows machines.

I sadly dont have a lot of time at the moment to work that out myself

@Baroshem
Copy link
Owner

@danielroe @pi0

I managed to test it out on an old Windows laptop and the issue is appearing when I want to push Nitro plugin from module:

config.plugins.push(normalize(fileURLToPath(new URL('./runtime/nitro', import.meta.url))))

Other URL;s are working correctly. Is it caused by this? https://github.com/nuxt/framework/pull/8626/files

I have updated the package to work with 3.0.0-rc.13 version but the issue is still there

@danielroe
Copy link
Contributor

In dev mode this likely needs to be a file URL not a path. Fancy testing that? If so, we can likely fix in Nuxt itself.

@Baroshem
Copy link
Owner

Related to nuxt/nuxt#15249

@Baroshem
Copy link
Owner

I tested it by sending even a simple path to the plugins array, it still shouts that it is not correct.

@Baroshem
Copy link
Owner

Baroshem commented Nov 11, 2022

I believe this is an issue on the nitro side as other handlers are working correctly, only the one that is being pushed to nitro plugins array is failing.

For all users facing the problem. When you set hidePoweredBy to false in your nuxt.config.ts your project will work fine and you can use it as a workaround now until a proper fix will be published. It will disable one of the middlewares (that is not as crucial as other middlewares) and will make your app work corectly on Windows.

CC @maxime-ducoroy @mikeschepers4 @luabraggion

@mikeschepers4
Copy link
Author

Can confirm that your workaround is working. thanks :)

@Baroshem Baroshem changed the title Error building/serving the nuxt project after importing the nuxt-security module Wrong parhs on Windows Nov 20, 2022
@Baroshem Baroshem changed the title Wrong parhs on Windows Wrong paths on Windows Nov 20, 2022
@weihongyu12
Copy link

Related to nuxt-modules/html-validator#232

@Baroshem
Copy link
Owner

Baroshem commented Dec 6, 2022

Guys,

Daniel is already working on fixing this issue on the nitro side. I will make sure to update nuxt to a newer version once it is released. For now, make sure to use this workaround -> #42 (comment)

@Baroshem
Copy link
Owner

Hey folks,

I have tested solution from @pi0 and it seems to be working now. Please test it out in your environment and let me know if there are any issues. Sorry that it tooked so long and hope that you will be still using the module :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants