search

wilr / grunt-shopify

Grunt plug-in for publishing Shopify theme assets
BSD 3-Clause "New" or "Revised" License
107 stars 32 forks source link

initial attempt to handle shopify whitelist. #36

Closed ctalkington closed 10 years ago

ctalkington commented 10 years ago

this should allow only paths in our base/cwd and allowed by shopify to be uploaded.

ctalkington commented 10 years ago

this should resolve #15 and #35 (for the time being). this needs testing with manually calling upload and delete.

ctalkington commented 10 years ago

in regards to the manual tasks, i think the biggest constraint is that they will need to be within the base root which i think was already required.

ctalkington commented 10 years ago

some examples:

// works from cwd with base 'shop'
grunt shopify:upload:shop/assets/style.css.liquid

// doesn't work from 'shop' with base 'shop'
grunt shopify:upload:assets/style.css.liquid
ctalkington commented 10 years ago

going to make a few more tweaks to this.

ctalkington commented 10 years ago

new commit switches to path.resolve as fs.realpathSync does stat and such on file which is overkill for what we are trying to do. i also added warnings (though annoying when the watch picks up files it shouldn't but atleast it's not making API calls now..).

ctalkington commented 10 years ago

i can switch the warnings to use the notify method but i question if it would create flood of notifications during certain watch instances that pick up invalid files.

ctalkington commented 10 years ago

ok made some changes to make the watch handler check if filepath is in its base before performing any actions that should resolve my caution of using notify so i've made it all use notify. let me know what you think.

wilr commented 10 years ago

Doesn't break my install so will ship it. Cannot get the duplicate messages when changing files now. One thing I did patch was any errors (say incorrect file path) can through as [Object] so converted those to something readable in 5e52fe05bce3263bd1ee8e6404e47fb56cbadfa5

ctalkington commented 10 years ago

yah, i had questioned the detail/errors being an array and if it would get converted or not. hadn't tested the logging bit was just making use of the err.detail over resp.errors in my last PR.

ctalkington commented 10 years ago

i do think the whitelisting and other tweaks will handle quite a few issues.

btw sorry for the flood of PRs. things should quiet down for a bit now :)

ctalkington commented 10 years ago

can we get a version bump by chance? I think minor is fine being its mostly internal changes.