durs / node-activex

Node.JS Implementaion of ActiveXObject
MIT License
329 stars 62 forks source link

Need info for preparing PR #103

Closed somanuell closed 3 years ago

somanuell commented 3 years ago

Hi durs,

as you may know, winax fails to build against recent versions of Electron, due to link errors with the Microsoft C++ libs

I am currently working on that problem, and will hopefully submit a PR.

I have to change the code in disp.cpp in function DispObject::NodeCreate around line 553

I have a FIX, but obviously want to test it before submitting the PR.

Question: How to test? How do I supply "a dispatch pointer"?

durs commented 3 years ago

Hi, what is "a dispatch pointer"? IDispatch*? Almost all tests use it. What exactly do you need to test?

somanuell commented 3 years ago

I want to test the code below line 546 in disp.cpp. The comment is: // Use supplied dispatch pointer How do I get there?

durs commented 3 years ago

It`is something specific, a pointer is passed in the argument as an 8-byte array, better ask OSjoerdWie But the problem started exactly in this place, due to the fact that I disabled GetBackingStore() - depricate warning; And also did not test this piece of code

somanuell commented 3 years ago

The code was submitted 15 month ago without a way of testing it? How do I contact OSjoerdWie?

durs commented 3 years ago

I have no contacts, I thought I could write through the site. Without testing - it happens too It's just an elementary transformation from Uint8Array (8 or 4 bytes on 32bit systems) to INT_PTR. Сan be implemented with a simple loop, but not very productive, like that: INT_PTR ptr = 0; for (size_t i = 0; i < input->Length(); i++) { ptr |= input->Get(i) << 8 * i; }

somanuell commented 3 years ago

Injecting an arbitrary integer which will be considered as a IDispatch pointer may be viewed as a major security vulnerability. If you don't remember of any use case, you may as well drop the code entirely.

Anyway, I will try to contact the developer.

OSjoerdWie commented 3 years ago

I was trying to connect to a particular running instance of Excel (multiple Excel processes might be open already). For this, I obtained the hWnd of the workbook of interest running in that particular instance. Next the pointer value was obtained via the AccessibleObjectFromWindow function (oleacc.h), which I accessed via an ffi-napi wrap.

The input parameters being the hWnd, OBJID_NATIVEOM, and a ref to the workbook COM object's UUID, the latter specified as: const WorkbookUUIDValues = { data1: 132096, data2: 0, data3: 0, data4: [192, 0, 0, 0, 0, 0, 0, 70] }; The output parameter of this call is supplied to the winax library.

Note that the approach is mostly based on some VBA code that was shared with me more than a decade ago (looks similar to the code shared here, in particular the GetXlApp_ByWBName function)

And yes, I should have added proper test code. Unfortunately, I don't foresee I will be able to still do this in the near future.

somanuell commented 3 years ago

And yes, I should have added proper test code. Unfortunately, I don't foresee I will be able to still do this in the near future.

Ok, how do we proceed?

I see 3 possibilities:

  1. We simply drop the code. But it will piss off any user of that code.
  2. We replace it by the @durs solution, that is: INT_PTR ptr = 0; for (size_t i = 0; i < input->Length(); i++) { ptr |= input->Get(i) << 8 * i; }
  3. We replace it by my version: Local localBuffer; node::Buffer::New(isolate, input->Buffer(), 0, input->Buffer()->ByteLength() ).ToLocal(&localBuffer); void* data = node::Buffer::Data( localBuffer );

    @durs What solution has your preference? Do you want to change the code yourself, or do you prefer a PR?

    durs commented 3 years ago

    made it even easier: void *data = node::Buffer::Data(args[0]);

    and added test "Dispatch pointer as Uint8Array" in test/variant.js

    somanuell commented 3 years ago

    Wonderfull, thanks, closing, winax is now ok for recent electron (with ".\node_modules.bin\electron-rebuild.cmd")