Skip to content
This repository has been archived by the owner on Oct 21, 2020. It is now read-only.

feat: validate contents.zip on Node API #99

Merged
merged 2 commits into from
Apr 17, 2018
Merged

Conversation

koba04
Copy link
Contributor

@koba04 koba04 commented Apr 16, 2018

Fixes #95.

I reused the validation for Browsers, which validates a buffer of contents.zip, so it doesn't depend on any local environments like paths.

@koba04 koba04 requested a review from teppeis April 16, 2018 07:38
@kintone-labs kintone-labs deleted a comment Apr 16, 2018
@kintone-labs kintone-labs deleted a comment Apr 16, 2018
Copy link
Contributor Author

@koba04 koba04 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to test on browsers on windows.

  • Edge on Windows
  • Chrome on Windows

src/zip.js Outdated
function validateManifest(entries, manifestJson, manifestPath) {
const manifestPrefix = path.dirname(manifestPath);
const getEntryKey = filePath =>
path.join(manifestPrefix, filePath).replace(new RegExp(`\\${path.sep}`, 'g'), '/');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔

@kintone-labs kintone-labs deleted a comment Apr 16, 2018
@koba04
Copy link
Contributor Author

koba04 commented Apr 17, 2018

@teppeis
This PR is ready for reviews!
I had to add a logic converting path separators from win style to posix style because the path is used as Map keys of contents.zip entries. The keys are always posix style even on windows.

@teppeis teppeis merged commit 85ef593 into master Apr 17, 2018
@teppeis teppeis deleted the validation-on-node-api branch April 17, 2018 08:49
@teppeis
Copy link
Contributor

teppeis commented Apr 17, 2018

@koba04 Thank you for awesome work for Windows!

@koba04
Copy link
Contributor Author

koba04 commented Apr 17, 2018

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants