Bumping vendored uriparser - #51123

Thanks for opening a pull request!

This pull request has been automatically converted to a draft because its title doesn't match Arrow's required format.

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

After updating the title, you can mark the pull request as ready for review.

See also:

• Other pull requests
• Contribution Guidelines - Contributing Overview

Thanks @imtherealnaska . Actually, this is a lot of additional vendored code and perhaps it would be better to make it a mandatory external dependency... What do you think @rok @kou ?

Thanks for opening this @imtherealnaska !
I don't have a strong opinion. Do we need the update?

uriparser has had security fixes, so yes.

Sorry @rok , I meant to ping @raulcd :)

@pitrou Raul is away for some more time I think.

@pitrou Raul is away for some more time I think.

Until Monday 7th I think.

Thanks @imtherealnaska . Actually, this is a lot of additional vendored code and perhaps it would be better to make it a mandatory external dependency... What do you think @rok @kou ?

Yes , but I saw this done some time ago #5865 . If you would like to have an external dependency, I can work on that.

@imtherealnaska we better wait for @kou to reply so we can decide on the direction. And thanks for being flexible regarding the approach!

@imtherealnaska we better wait for @kou to reply so we can decide on the direction. And thanks for being flexible regarding the approach!

Sure. Happy to contribute

Actually, this is a lot of additional vendored code and perhaps it would be better to make it a mandatory external dependency...

+1

Let's do it.

Yes, I am back on the 7th (aka today) :)
👍 on moving this to a mandatory external dependency. @imtherealnaska have you started working on this, is there an issue tracking it?

Issue should probably be the one to bump it:

• [C++] Bump vendored uriparser #51007

Linking as it wasn't linked on the PR.

Yes, I am back on the 7th (aka today) :) 👍 on moving this to a mandatory external dependency. @imtherealnaska have you started working on this, is there an issue tracking it?

Hi, Yes I am working on this. I have a working branch for this .

But one question, what should be the minimum version of uriparser to support . Currently we are vendoring 0.9.3 , so should that be the required minimum version or should I raise it to 1.0.0 ?

Currently we are vendoring 0.9.3 , so should that be the required minimum version or should I raise it to 1.0.0 ?

I'd just bump to latest (1.0.2) as it contains several security fixes.

Could you accept 0.9.6 or later? Ubuntu 22.04 ships 0.9.6: https://packages.ubuntu.com/jammy/liburiparser-dev

Since this was a different approach I have opened a new PR . #51244