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

Object.prototype conflict with sharp module! #3292

Closed
Nevro opened this issue Jul 13, 2022 · 3 comments
Closed

Object.prototype conflict with sharp module! #3292

Nevro opened this issue Jul 13, 2022 · 3 comments

Comments

@Nevro
Copy link

Nevro commented Jul 13, 2022

Sample code:

import Sharp from 'sharp';

Object.prototype.testProtoFunc = function () {
	//code
	return this;
};

const { data, info } = await Sharp('./image.png').raw().toBuffer({
	resolveWithObject: true
});

Output:

/home/user/node/node_modules/sharp/lib/output.js:1187
        sharp.pipeline(this.options, (err, data, info) => {
              ^

TypeError: A string was expected
    at /home/user/node/node_modules/sharp/lib/output.js:1187:15
    at new Promise (<anonymous>)
    at Sharp._pipeline (/home/user/node/node_modules/sharp/lib/output.js:1186:14)
    at Sharp.toBuffer (/home/user/node/node_modules/sharp/lib/output.js:146:15)
    at file:///home/user/node/src/base16.js:8:60
    at ModuleJob.run (node:internal/modules/esm/module_job:198:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:409:24)
    at async loadESM (node:internal/process/esm_loader:85:5)
    at async handleMainPromise (node:internal/modules/run_main:61:12)

Node.js v18.5.0
sharp 0.30.7

This is definitely not expected behavior for module!

@lovell
Copy link
Owner

lovell commented Jul 13, 2022

The problem occurs here when parsing an Object containing EXIF metadata:

sharp/src/pipeline.cc

Lines 1496 to 1500 in e40a881

Napi::Array mdStrKeys = mdStrs.GetPropertyNames();
for (unsigned int i = 0; i < mdStrKeys.Length(); i++) {
std::string k = sharp::AttrAsStr(mdStrKeys, i);
baton->withMetadataStrs.insert(std::make_pair(k, sharp::AttrAsStr(mdStrs, k)));
}

The existing logic uses GetPropertyNames but doesn't filter these to ensure they are "owned" by the object, thus will also try to use any properties inherited from the Object prototype.

Commit c295f06 wraps this logic in a HasOwnProperty check. This will be in v0.31.0, thanks for reporting.

@lovell lovell added this to the v0.31.0 milestone Jul 13, 2022
@Nevro Nevro closed this as completed Jul 29, 2022
@lovell
Copy link
Owner

lovell commented Jul 30, 2022

Re-opening until the fix is published (to allow discoverability for others who might run into the same problem).

@lovell lovell reopened this Jul 30, 2022
@lovell
Copy link
Owner

lovell commented Sep 5, 2022

v0.31.0 now available, thanks for reporting.

@lovell lovell closed this as completed Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants