node-pcap / node_pcap

libpcap bindings for node
MIT License
928 stars 253 forks source link

Updated to node 12.x #254

Closed jonahharris closed 4 years ago

terbo commented 4 years ago

This has been working flawlessly in my project over the last week.

What are the possibilities of creating a specific repository with these changes merged in case the maintainer is unresponsive?

roccomuso commented 4 years ago

I'd like some test from another mantainer before merging

bnonni commented 4 years ago

@terbo forgive me for being a n00b here, but how can I implement this branch into my project? Should I reference the url under dependencies in my package.json file?

"pcap":  https://github.com/jonahharris/node_pcap/tree/with-v12-fixes

Or should I clone the branch, build, and reference that file path in my package.json?

jonahharris commented 4 years ago

Thanks @terbo — I’ve been using it successfully on my AMQP dissector as well. Unfortunately, no response from the node-socketwatcher maintainer for those related changes, so I don’t know what that’s going to look like insofar as keeping the fork of the underlying module.

terbo commented 4 years ago

@bnonni - that was my concern, with the node LTS version bump, how I would reference this update cleanly.

bnonni commented 4 years ago

@terbo - I got cha. I ended up cloning the node_pcap repo into my app folder, building and referencing that folder in my package.json file like so:

"pcap": "./node_pcap",

This works. It builds, and my app runs. For reference, I'm building a simple pcap electron app. I can easily reference pcap in my .js files, and wire up a GUI to "Start" and "Stop" pcap sessions this way. I'm so excited that this worked! @roccomuso I hope all testing goes well, so this can be merged bc it solves all my issues!

@jonahharris great work man!

mildsunrise commented 4 years ago

Just in case it's useful to anyone, I've started a fork → pcap3. It contains these fixes, drops the socketwatch dependency, and a bunch of other fixes.

TincaTibo commented 4 years ago

Thanks all of you, I used @mildsunrise fork to upgrade two weeks ago. And it proves working really well! I've captured and analysed hundreds of gigabytes without a noticable issue. And it requires less CPU than before :)

roccomuso commented 4 years ago

@mildsunrise would love to bring those changes here

mildsunrise commented 4 years ago

Great, feel free to do so :) If you want I can try to separate them into PRs, but there will be conflicts...

The only API breaking change is the radiotap decoder, which was rewritten (https://github.com/mildsunrise/node_pcap3/commit/175fa9b4d8f38a6640da8381d0e6703751fc0ce3, https://github.com/mildsunrise/node_pcap3/commit/b85f8d0ca2ab0058590823f7b69d27efe2d7fedd) to correct many issues

roccomuso commented 4 years ago

@mildsunrise yeah would be nice to document that

mildsunrise commented 4 years ago

All changes in my fork are now incorporated into v3 of pcap, so the fork is currently deprecated.