raisely / cli

Raisely CLI for local development
Other
8 stars 6 forks source link

Ensure filenames sent do not include absolute paths (Windows) #20

Closed tmns closed 2 years ago

tmns commented 2 years ago

What This PR ensures that filenames sent to the Raisely api via buildStyles don't also include the files' absolute paths when on Windows.

Why It was observed that when running raisely start on Windows and making changes to files, the filenames uploaded via buildStyles include the files' absolute paths on the User's system. This happens due to our use of glob to create an array of all our files, which outputs paths forward-slash-normalized (relevant Github issue).

This in turn causes the first replace call both here and here to not replace anything, as they rely on stylesDir, which still contains the platform specific path (i.e., with back slashes for Windows). So, for example, instead of sending a value like 0_core/1_variables.scss, we would send 'C:/Users/username/raisely/stylesheets/0_core/1_variables.scss'.

This issue then manifests itself when syncing locally / running raisely update. As the filenames received from campaign.data.config.css.files will all actually be absolute paths, resulting in an error like the following when trying to create the directory for the given filename:

× ENOENT: no such file or directory, mkdir 'C:\Users\tmns\raisely\stylesheets\campaign\C:\Users\tmns\raisely\stylesheets\0_core'

How The proposed fix is to also run replace on the full path, swapping all potential forward slashes with back slashes, before trying to remove the path as normal:

const fileName = path.replace(`${fullPath.replace(/\\/g, '/')}/`, "");

@chrisjensen also suggested using path.normalize; however, if we do that, I think we then need to use path.sep in places where we're currently using / (for example in sync.js). I see that we actually did do that before but then that change was reverted; so, I'm not too keen on adding that back.. but someone with more context could probably make the right call here!

Another potential challenge with path.normalize is that a user would again face update issues when switching between Windows / non-Windows machines, as one machine would push with a platform-specific path and then the other would try to parse it with its own path.sep. For example, if on Windows 0_core\1_variables.scss was pushed, on Mac / Linux instead of the folder 0_core with the file 1_variables.scss being created, a file with the name 0_core\1_variables.scss would be created. Having said all that, I imagine this is also an edge case.

Testing I've tested the changes on Win11 and Mac. However, folks with more knowledge of the code / cli should definitely test it as well, as it's entirely possible I'm not covering all cases.

To reproduce the error on Windows:

  1. Run raisely init and pull down all relevant files.
  2. Run raisely start and make a change to a stylesheet.
  3. Run raisely update; at this point you should get an error similar to that shown above.