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] Request for bumping up urllib3 version in previous opensearch-py functions #516

Closed
AkshathRaghav opened this issue Sep 29, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@AkshathRaghav
Copy link

Is your feature request related to a problem?

An issue was discovered in urllib3 before 1.26.5: when provided with a URL containing many @ characters in the authority component, the authority regular expression exhibits catastrophic backtracking, causing a denial of service if a URL were passed as a parameter or redirected to via an HTTP redirect. This request is for opensearch-benchmark.

What solution would you like?

Bumping up the urllib3 version from >=1.21.1 to >= 1.26.5 in previous releases of opensearch-py specifically 2.2.0 and 1.0.0, which are used by opensearch-benchmark versions 1.1 and 1.0.

What alternatives have you considered?

I could hardcode it in the setup.py in opensearch-benchmarks so that it installs the correct version after it installs opensearch-py
https://github.com/opensearch-project/opensearch-benchmark/blob/5a99b0770ab3a0df4145e76f3cacb95dd8118073/setup.py#L61

Do you have any additional context?

Not really. This is just a request, and the issue can be handled easily if denied.

@AkshathRaghav AkshathRaghav added enhancement New feature or request untriaged Need triage labels Sep 29, 2023
@saimedhi saimedhi removed the untriaged Need triage label Oct 2, 2023
@wbeckler
Copy link
Contributor

wbeckler commented Oct 3, 2023

If #518 goes through, can't you bump the version of opensearch-py you're depending on in opensearch-benchmark?

@saimedhi
Copy link
Collaborator

saimedhi commented Oct 3, 2023

@saimedhi
Copy link
Collaborator

saimedhi commented Oct 3, 2023

Hello @AkshathRaghav and @IanHoang, I've merged the changes to the main branch. Now, we have a few more steps to complete:

  1. Similar changes should be merged into other branches needed by opensearch-benchmarks.
    Please note: Check the noxfile.py in the corresponding branches to confirm the Python versions it uses and ensure
    they are supported by urllib3 1.26.5.

  2. Release patch versions for the corresponding branches.

@wbeckler
Copy link
Contributor

wbeckler commented Oct 3, 2023

  1. Similar changes should be merged into other branches needed by opensearch-benchmarks.
    Please note: Check the noxfile.py in the corresponding branches to confirm the Python versions it uses and ensure
    they are supported by urllib3 1.26.5.

I assume you mean changing old versions of opensearch-py? I would propose that's not necessary since dependent projects can bump the version they depend on.

  1. Release patch versions for the corresponding branches.

I assume you mean branches of opensearch-py?

@IanHoang
Copy link

IanHoang commented Oct 3, 2023

@wbeckler @saimedhi To clarify, based on releasing guide and since OSB is using opensearch-py 2.2.0, we should cherry-pick the commit (#518) from main onto 2.X and put out a PR with patch and backport labels?

@saimedhi
Copy link
Collaborator

saimedhi commented Oct 3, 2023

Hello @IanHoang, after a thoughtful discussion with wbeckler, we've decided to release a new opensearch-py version that incorporates the changes from PR #518. Following the release, please consider updating opensearch-benchmark to use this latest opensearch-py version. We believe this approach is the most effective, and we kindly request your comment on the issue, expressing your desire to release opensearch-py 2.3.2.

@IanHoang
Copy link

IanHoang commented Oct 3, 2023

Thanks @saimedhi and @wbeckler!

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

No branches or pull requests

4 participants