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

feature(dependencies): adding support for protobufjs 7 #244

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

catYalere
Copy link

@catYalere catYalere commented Jun 22, 2023

This PR add support for higher versions of protobufjs.
resolves #240 and closes #237

It has been tested with protobufjs versions from 6.10.1 to 7.2.3 [UPDATE: 7.2.4 on latest commit]
6.10.1
image

7.2.3
image

Also run integration test with kafkajs and sent events to confluent

BTW You can move protobujs dependency to test it also limiting to under 8 because we don't know which breaking changes will come there. Leaving yarn.lock with latest protobufjs version so you can test it

Important to see if it will be better to install protobufjs as a peerDependency

  "peerDependencies":{
    "protobufjs": ">= 6.10.1 < 8"
  },

As a plus

  • Upgrade ts-jest that was misalign with jest version
  • Fix spacing in docker-compose.yml
  • Move dockest to latest version and modify correctly the dockest.ts + gitignore new log file

Extra in case someone wants to run dockest over Mac, it doesn't work because docker-compose config print published port as string instead of number, they have a PR since long ago but hasn't done anything

But you can do this in case of necessity (only Mac)

yarn &&
sed -i ''  's/mergedComposeFiles,/mergedComposeFiles.replaceAll(\/published: "(.*)"\/g, "published: $1"),/g' node_modules/dockest/dist/run/bootstrap/getParsedComposeFile.js &&
yarn test

@Gilad-Gur-Andelman
Copy link

@catYalere Hi, there is a new Prototype Pollution vulnerability in protobufjs:
GHSA-h755-8qp9-cq85

Can you please upgrade protobuf to 7.2.4 as suggested in the vulnerability details?

@catYalere
Copy link
Author

catYalere commented Jul 12, 2023

@Gilad-Gur-Andelman updated and verify that everything work as expected

image

@FlashThePlayer
Copy link

@Nevon any chance to get this merged?
would silence a lot of npm audit screams 😄

@catYalere
Copy link
Author

@Nevon @tulios ?

@catYalere
Copy link
Author

After a month of waiting I'm currently thinking to fork this and start maintenance and also publish npm package under my public account, thoughts?

@iwt-gregorpoloczek
Copy link

@catYalere as much as I like this pragmatic approach, It would be better for you having rights to maintain this repository here, instead of forking it (imho). I don't think you'd want to put the burden of developing this package in the future all onto you?

@catYalere
Copy link
Author

catYalere commented Jul 20, 2023

@iwt-gregorpoloczek agree with that and I also prefer it but the issue is that we don't find the current maintainers, so I can help them, so I currently discard that option 😢

@buccfer-knauf
Copy link

Updates on this? Can someone merge it?

@Fryuni
Copy link

Fryuni commented Jul 28, 2023

I don't see any movement from any maintainer or collaborator since last October, not just the latest commit, I haven't seen even a comment. Seems like this project has been abandoned.

@gavageovanni
Copy link

@Nevon @evanshortiss

<3 could you do the merge please? 🥹🥹🥹🥹🥹🥹🥹

@gavageovanni
Copy link

oI @augustozanetti consegue dar uma mao aqui ? rs

@augustozanetti
Copy link
Contributor

@Nevon @tulios ⬆️

@cyppan
Copy link

cyppan commented Aug 9, 2023

cf tulios/kafkajs#1603 @catYalere you could comment there if you're still up for the task :)

@kaliabadi
Copy link

@Nevon @bifrost @erikengervall @tulios can we please have a getting this merged or update this repo to be archived so it is clear it is no longer maintained please?

@dotkas
Copy link

dotkas commented Dec 7, 2023

@sufiyangorgias
Copy link

sufiyangorgias commented May 3, 2024

Are we merging anytime soon?

#258

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

Successfully merging this pull request may close these issues.

Update npm packages Update dependencies: protobufjs-6.10.1 has vulnerabilities