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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 2 commits into from
Closed

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

wants to merge 2 commits into from

Conversation

catYalere
Copy link
Contributor

@catYalere catYalere commented Jun 22, 2023 •
edited
Loading

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

@catYalere catYalere mentioned this pull request Jun 22, 2023
Open
@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
Contributor Author

catYalere commented Jul 12, 2023 •
edited
Loading

@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
Contributor Author

@Nevon @tulios ?

@catYalere
Copy link
Contributor 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
Contributor Author

catYalere commented Jul 20, 2023 •
edited
Loading

@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?

@JamesPatrickGill JamesPatrickGill mentioned this pull request Aug 23, 2023
Closed
@dotkas
Copy link

dotkas commented Dec 7, 2023

@sufiyangorgias
Copy link

sufiyangorgias commented May 3, 2024 •
edited
Loading

Are we merging anytime soon?

#258

@kriskw1999 kriskw1999 mentioned this pull request Dec 21, 2024
Merged
@kriskw1999
Copy link
Contributor

I am going to merge the linked one as a follow up

@kriskw1999 kriskw1999 closed this Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update dependencies: protobufjs-6.10.1 has vulnerabilities Update npm packages