Skip to content

Fixing regex bypass issue #4336

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
Merged
merged 5 commits into from
Oct 24, 2025
Merged

Fixing regex bypass issue #4336

merged 5 commits into from
Oct 24, 2025

Conversation

@zane-neo
Copy link
Collaborator

@zane-neo zane-neo commented Oct 22, 2025

Description

Fix connector regex bypass issue

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@zane-neo
Fixing regex bypass issue
922075c 
Signed-off-by: zane-neo <zaniu@amazon.com>
@zane-neo zane-neo had a problem deploying to ml-commons-cicd-env October 22, 2025 03:27 — with GitHub Actions Failure
@zane-neo zane-neo had a problem deploying to ml-commons-cicd-env October 22, 2025 03:27 — with GitHub Actions Failure
@zane-neo
fix failure ITs
6289562 
Signed-off-by: zane-neo <zaniu@amazon.com>
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env October 22, 2025 04:56 — with GitHub Actions Inactive
@zane-neo zane-neo had a problem deploying to ml-commons-cicd-env October 22, 2025 04:56 — with GitHub Actions Failure
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env October 22, 2025 04:56 — with GitHub Actions Inactive
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env October 22, 2025 06:50 — with GitHub Actions Inactive
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env October 22, 2025 06:50 — with GitHub Actions Inactive
@codecov
Copy link

codecov bot commented Oct 22, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 78.75000% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.13%. Comparing base (eec7179) to head (fe85a6e).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ommon/httpclient/MLValidatableAsyncHttpClient.java 67.44% 11 Missing and 3 partials ⚠️
...engine/algorithms/remote/AwsConnectorExecutor.java 91.66% 1 Missing ⚠️
...e/algorithms/remote/HttpJsonConnectorExecutor.java 90.90% 0 Missing and 1 partial ⚠️
...ine/algorithms/remote/RemoteConnectorExecutor.java 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #4336      +/-   ##
============================================
- Coverage     80.13%   80.13%   -0.01%     
- Complexity    10171    10175       +4     
============================================
  Files           854      855       +1     
  Lines         44219    44239      +20     
  Branches       5113     5113              
============================================
+ Hits          35437    35449      +12     
- Misses         6631     6641      +10     
+ Partials       2151     2149       -2
Flag Coverage Δ
ml-commons 80.13% <78.75%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zane-neo zane-neo had a problem deploying to ml-commons-cicd-env October 22, 2025 07:55 — with GitHub Actions Failure
Hailong-am
Hailong-am reviewed Oct 22, 2025
View reviewed changes
common/src/main/java/org/opensearch/ml/common/httpclient/MLHttpClientFactory.java Outdated
Duration connectionTimeout,
Duration readTimeout,
int maxConnections,
AtomicBoolean connectorPrivateIpEnabled
Copy link
Contributor

@Hailong-am Hailong-am Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use bool here? what's the reason to use atomicBoolean

Copy link
Collaborator Author

@zane-neo zane-neo Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree that boolean is better, I tried to change all the occurrence of this connectorPrivateIpEnabled to boolean all the way back to where it's defined, but there's more UTs need to be changed. But changing to boolean is correct direction, let me try again this change.

@zane-neo zane-neo had a problem deploying to ml-commons-cicd-env October 23, 2025 03:01 — with GitHub Actions Failure
@zane-neo zane-neo had a problem deploying to ml-commons-cicd-env October 23, 2025 03:23 — with GitHub Actions Failure
@zane-neo zane-neo had a problem deploying to ml-commons-cicd-env October 23, 2025 03:23 — with GitHub Actions Failure
@zane-neo
fix dependency version conflict issue
56fe122 
Signed-off-by: zane-neo <zaniu@amazon.com>
@zane-neo zane-neo had a problem deploying to ml-commons-cicd-env October 23, 2025 03:52 — with GitHub Actions Failure
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env October 23, 2025 03:52 — with GitHub Actions Inactive
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env October 23, 2025 03:52 — with GitHub Actions Inactive
@zane-neo zane-neo had a problem deploying to ml-commons-cicd-env October 24, 2025 00:49 — with GitHub Actions Failure
@zane-neo zane-neo had a problem deploying to ml-commons-cicd-env October 24, 2025 00:49 — with GitHub Actions Failure
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env October 24, 2025 00:49 — with GitHub Actions Inactive
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env October 24, 2025 03:12 — with GitHub Actions Inactive
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env October 24, 2025 03:12 — with GitHub Actions Inactive
@zane-neo zane-neo temporarily deployed to ml-commons-cicd-env October 24, 2025 03:12 — with GitHub Actions Inactive
@zane-neo zane-neo had a problem deploying to ml-commons-cicd-env October 24, 2025 04:18 — with GitHub Actions Failure
@zane-neo zane-neo merged commit c4ec014 into opensearch-project:main Oct 24, 2025
15 of 21 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 24, 2025
@zane-neo @github-actions
Fixing regex bypass issue (#4336)
2b3c08b 
* Fixing regex bypass issue

Signed-off-by: zane-neo <zaniu@amazon.com>

* fix failure ITs

Signed-off-by: zane-neo <zaniu@amazon.com>

* Change connector private ip enabled to boolean type

Signed-off-by: zane-neo <zaniu@amazon.com>

* fix dependency version conflict issue

Signed-off-by: zane-neo <zaniu@amazon.com>

---------

Signed-off-by: zane-neo <zaniu@amazon.com>
(cherry picked from commit c4ec014)
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Oct 24, 2025
Merged
zane-neo added a commit that referenced this pull request Oct 24, 2025
@opensearch-trigger-bot @zane-neo
Fixing regex bypass issue (#4336) (#4344)
0f57545 
* Fixing regex bypass issue



* fix failure ITs



* Change connector private ip enabled to boolean type



* fix dependency version conflict issue



---------


(cherry picked from commit c4ec014)

Signed-off-by: zane-neo <zaniu@amazon.com>
Co-authored-by: zane-neo <zaniu@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ylwu-amzn ylwu-amzn ylwu-amzn left review comments

@xinyual xinyual xinyual approved these changes

@b4sjoo b4sjoo b4sjoo approved these changes

@dhrubo-os dhrubo-os Awaiting requested review from dhrubo-os dhrubo-os is a code owner

@mingshl mingshl Awaiting requested review from mingshl mingshl is a code owner

@jngz-es jngz-es Awaiting requested review from jngz-es jngz-es is a code owner

@model-collapse model-collapse Awaiting requested review from model-collapse model-collapse is a code owner

@rbhavna rbhavna Awaiting requested review from rbhavna rbhavna is a code owner

@Zhangxunmt Zhangxunmt Awaiting requested review from Zhangxunmt Zhangxunmt is a code owner

@austintlee austintlee Awaiting requested review from austintlee austintlee is a code owner

@HenryL27 HenryL27 Awaiting requested review from HenryL27 HenryL27 is a code owner

@sam-herman sam-herman Awaiting requested review from sam-herman sam-herman is a code owner

@pyek-bot pyek-bot Awaiting requested review from pyek-bot pyek-bot is a code owner

+2 more reviewers

@Hailong-am Hailong-am Hailong-am left review comments

@rithin-pullela-aws rithin-pullela-aws rithin-pullela-aws approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

backport 3.3

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@zane-neo @ylwu-amzn @xinyual @Hailong-am @b4sjoo @rithin-pullela-aws