-
-
Notifications
You must be signed in to change notification settings - Fork 377
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
Add support for Asar #378
Add support for Asar #378
Conversation
@dyoshikawa In the pull request message, change |
@Richienb Thanks. Changed my message. |
Co-authored-by: Richie Bendall <[email protected]>
As it turns out, asar doesn't actually have a mime type so it should be set to |
Can we use original mime type name? |
@dyoshikawa Not sure. Try asking that on electron/asar#199 |
https://www.iana.org/assignments/media-types/media-types.xhtml Unregisterd mime types in this library seems to already exist. https://tools.ietf.org/html/rfc6838#section-3.4
So |
🤷♂️ |
These unregistered types are still used elsewhere so there's at least some traction behind them.
Since I don't think this library should create new MIME types. It should be the responsibility of the community around the format to decide on and register their MIME type. Given |
Consistent with existing code, I think we can use |
@Borewit Thanks. |
Co-authored-by: Richie Bendall <[email protected]>
Co-authored-by: Richie Bendall <[email protected]>
core.js
Outdated
if (buffer.length > 16 && checkString('{"files":{', {offset: 16})) { | ||
try { | ||
// Check header_size | ||
buffer.readUInt32LE(4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unlike the comment suggests, there is in fact nothing checked here.
The header size is read, but the result is ignored.
Should the result not be used:
- to check the format
- determine the length of the JSON payload?
If required, note that buffer.readUInt32LE(4)
can be executed safely on any file, as long as there are 4 bytes available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
I think this line should delete because your opinion.
Do you think so, too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go ahead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, sir.
core.js
Outdated
@@ -1355,6 +1355,24 @@ async function _fromTokenizer(tokenizer) { | |||
} | |||
} | |||
} | |||
|
|||
// Check for Asar | |||
if (buffer.length > 16 && checkString('{"files":{', {offset: 16})) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The checkString
will only succeed if the string exist at that location, so no need to check the buffer length yourself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx. I adopted it.
Co-authored-by: Richie Bendall <[email protected]>
Move the test in the "256-byte" group.
Gradually check if format is ASAR/Pickle.
@@ -1367,7 +1385,8 @@ const stream = readableStream => new Promise((resolve, reject) => { | |||
const pass = new stream.PassThrough(); | |||
let outputStream; | |||
if (stream.pipeline) { | |||
outputStream = stream.pipeline(readableStream, pass, () => {}); | |||
outputStream = stream.pipeline(readableStream, pass, () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Borewit
Is it needed?
This diff looks only a new line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Best remove this newline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Borewit Thank you.
I adopted your suggesting code.
@@ -1195,6 +1195,24 @@ async function _fromTokenizer(tokenizer) { | |||
}; | |||
} | |||
|
|||
if (check([0x04, 0x00, 0x00, 0x00]) && buffer.length >= 16) { // Rough & quick check Pickle/ASAR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are all of these comments necessary?
Thank you for everyone! |
Fixes #375
The official page of the file format
https://github.com/electron/asar#format