[Core] Reject negative values when converting to unsigned by om4rrr · Pull Request #33482 · openvinotoolkit/openvino

om4rrr · 2026-01-06T15:28:29Z

Fix

This PR prevents negative values from being accepted for ov::hint::num_requests.
Previously, passing a signed negative value could end up as a large unsigned number when stored/handled as an unsigned property. The property helper now validates inputs and rejects negative signed values early with a clear exception.

Tests

Added unit tests in src/inference/tests/unit/core.cpp:
- PropertiesValidation.HintNumRequestsRejectsNegativeSigned (negative values are rejected)
- PropertiesValidation.HintNumRequestsAcceptsUnsigned (valid unsigned values are accepted and stored correctly)

Ticket

[Good First Issue][Bug][Core]: negative values are not checked in property with unsigned value type #23194

Copilot

Pull request overview

This PR enhances type safety in ov::Any conversions by adding validation for integral type conversions that could result in data loss or unexpected wraparound. The changes prevent silent errors like converting -1 to 4294967295 when casting from signed to unsigned types.

Adds range checking for signed-to-unsigned and unsigned-to-signed integral conversions
Rejects negative signed values when converting to unsigned types
Validates that values fit within target type's numeric limits before conversion

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/core/include/openvino/core/any.hpp

om4rrr · 2026-01-15T03:01:31Z

Hi @praasz
I aligned the fix with your note about Any::as being a cast-only API.
I reverted the Any-level change and added validation at the ov::hint::num_requests property layer instead, so negative values are rejected instead of wrapping.
I added unit tests in src/inference/tests/unit/core.cpp to cover both the negative case and a valid unsigned value.

mlukasze · 2026-01-15T06:24:01Z

please resolve conflict before we go any further

om4rrr · 2026-01-15T16:09:48Z

Hi @mlukasze
I have updated the PR to address the merge conflict and ensured the tests are passing. When you have time, could you please review the PR?

src/inference/include/openvino/runtime/properties.hpp

olpipi · 2026-01-22T11:50:35Z

I don't have any fundamental objections regarding rejecting negative values for all properties.
Please add tests to check if converting from "-1" as string shows the same behavior as from int.
And please check if binary size is not increased dramatically after adding implementation into .hpp file.

om4rrr · 2026-01-22T19:00:42Z

Hi @olpipi
I added coverage to ensure negative values are rejected consistently for ov::hint::num_requests, including "-1" passed as a string, and verified core.set_property(..., ov::hint::num_requests(...)) rejects both -1 and "-1".

could you please review it?

src/inference/include/openvino/runtime/properties.hpp

src/inference/tests/unit/core.cpp

praasz · 2026-01-23T06:44:40Z

Hi @olpipi I added coverage to ensure negative values are rejected consistently for ov::hint::num_requests, including "-1" passed as a string, and verified core.set_property(..., ov::hint::num_requests(...)) rejects both -1 and "-1".

could you please review it?

I agree with @olpipi that this change is valid.
Just make more detailed tests for different property types and input value type to check corner case of conversion and remove the validation if it is not required to avoid dead code which will be reported by static analysis tool.

src/inference/include/openvino/runtime/properties.hpp

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

om4rrr · 2026-03-06T23:36:17Z

Hi @praasz
Could you please take a look and re-trigger the Jenkins build?
Thank you!

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Copilot · 2026-03-09T06:10:33Z

src/inference/include/openvino/runtime/properties.hpp

+            if constexpr (std::is_integral_v<UType> && std::is_unsigned_v<UType>) {
+                const std::string str_value = value;
+                // Find first non-whitespace character
+                const auto pos = str_value.find_first_not_of(" \t\n\r");
+                if (pos != std::string::npos && str_value[pos] == '-') {
+                    OPENVINO_THROW("Cannot assign negative value ", str_value, " to unsigned property");
+                }


[MEDIUM] The string-to-unsigned path on line 147 checks std::is_integral_v<UType> && std::is_unsigned_v<UType> but does not exclude bool, while the numeric path on line 166 explicitly excludes bool with !std::is_same_v<UType, bool>. Since std::is_unsigned_v<bool> is true, a bool property constructed from a string like "-1" would incorrectly hit the negative check in the string path. Add && !std::is_same_v<UType, bool> to the condition on line 147 for consistency.

olpipi · 2026-03-09T08:26:57Z

build_jenkins

om4rrr · 2026-03-10T07:11:37Z

@olpipi
Could you please take a look at the failing CI checks and handle them?

om4rrr · 2026-03-12T05:31:00Z

Hi @olpipi @praasz
I’m looking forward to getting pr merged once the CI checks are resolved. Please let me know if I should update or fix anything from my side.

mlukasze · 2026-03-12T06:58:20Z

build_jenkins

praasz · 2026-03-17T11:06:04Z

@om4rrr
There are still compilation issue on Windows build.

src\inference\include\openvino/runtime/properties.hpp(172): error C2589: '(': illegal token on right side of '::'
src\inference\include\openvino/runtime/properties.hpp(162): note: This diagnostic occurred in the compiler generated function 'ov::Property<T,mutability_>::Forward<V>::operator U(void)'
src\inference\include\openvino/runtime/properties.hpp(172): note: the template instantiation context (the oldest one first) is
src\inference\include\openvino/runtime/properties.hpp(122): note: while compiling class template 'ov::Property'
src\inference\include\openvino/runtime/properties.hpp(172): error C2760: syntax error: '(' was unexpected here; expected ')'
src\inference\include\openvino/runtime/properties.hpp(162): note: This diagnostic occurred in the compiler generated function 'ov::Property<T,mutability_>::Forward<V>::operator U(void)'
src\inference\include\openvino/runtime/properties.hpp(172): error C3878: syntax error: unexpected token '(' following 'expression'
src\inference\include\openvino/runtime/properties.hpp(162): note: This diagnostic occurred in the compiler generated function 'ov::Property<T,mutability_>::Forward<V>::operator U(void)'
src\inference\include\openvino/runtime/properties.hpp(172): note: error recovery skipped: '('
src\inference\include\openvino/runtime/properties.hpp(162): note: This diagnostic occurred in the compiler generated function 'ov::Property<T,mutability_>::Forward<V>::operator U(void)'
src\inference\include\openvino/runtime/properties.hpp(172): error C2760: syntax error: '>' was unexpected here; expected 'statement'
src\inference\include\openvino/runtime/properties.hpp(162): note: This diagnostic occurred in the compiler generated function 'ov::Property<T,mutability_>::Forward<V>::operator U(void)'
src\inference\include\openvino/runtime/properties.hpp(172): note: error recovery skipped: '>'

om4rrr · 2026-03-17T12:31:46Z

@praasz @olpipi @mlukasze

Could you please re-run the required workflows/Jenkins?

praasz · 2026-03-17T13:07:53Z

build_jenkins

…olkit#33482) ### Fix This PR prevents negative values from being accepted for `ov::hint::num_requests`. Previously, passing a signed negative value could end up as a large unsigned number when stored/handled as an unsigned property. The property helper now validates inputs and rejects negative signed values early with a clear exception. ### Tests - Added unit tests in `src/inference/tests/unit/core.cpp`: - `PropertiesValidation.HintNumRequestsRejectsNegativeSigned` (negative values are rejected) - `PropertiesValidation.HintNumRequestsAcceptsUnsigned` (valid unsigned values are accepted and stored correctly) ### Ticket - openvinotoolkit#23194

Copilot AI review requested due to automatic review settings January 6, 2026 15:28

om4rrr requested a review from a team as a code owner January 6, 2026 15:28

github-actions bot added category: Core OpenVINO Core (aka ngraph) category: CPP API OpenVINO CPP API bindings labels Jan 6, 2026

sys-openvino-ci added the ExternalPR External contributor label Jan 6, 2026

Copilot started reviewing on behalf of om4rrr January 6, 2026 15:29 View session

Copilot AI reviewed Jan 6, 2026

View reviewed changes

src/core/include/openvino/core/any.hpp Outdated Show resolved Hide resolved

src/core/include/openvino/core/any.hpp Outdated Show resolved Hide resolved

praasz requested changes Jan 13, 2026

View reviewed changes

src/core/include/openvino/core/any.hpp Show resolved Hide resolved

praasz assigned olpipi Jan 13, 2026

om4rrr requested a review from a team as a code owner January 15, 2026 02:53

github-actions bot added category: inference OpenVINO Runtime library - Inference and removed category: Core OpenVINO Core (aka ngraph) labels Jan 15, 2026

om4rrr force-pushed the fix-negative-unsigned branch from 5e2523d to 6cd2054 Compare January 15, 2026 15:52

om4rrr requested a review from praasz January 15, 2026 15:57

praasz reviewed Jan 16, 2026

View reviewed changes

src/inference/include/openvino/runtime/properties.hpp Outdated Show resolved Hide resolved

om4rrr requested a review from praasz January 16, 2026 15:08

om4rrr force-pushed the fix-negative-unsigned branch from c131491 to 5e9b295 Compare January 19, 2026 18:49

praasz self-assigned this Jan 20, 2026

praasz reviewed Jan 23, 2026

View reviewed changes

praasz added this to the 2026.1 milestone Jan 23, 2026

mlukasze linked an issue Jan 23, 2026 that may be closed by this pull request

[Good First Issue][Bug][Core]: negative values are not checked in property with unsigned value type #23194

Closed

om4rrr requested a review from praasz January 23, 2026 15:19

olpipi reviewed Feb 4, 2026

View reviewed changes

src/inference/include/openvino/runtime/properties.hpp Outdated Show resolved Hide resolved

om4rrr force-pushed the fix-negative-unsigned branch from 0934d62 to 2f369e6 Compare February 12, 2026 09:37

Copilot AI reviewed Mar 6, 2026

View reviewed changes

mlukasze requested a review from Copilot March 9, 2026 06:05

Copilot started reviewing on behalf of mlukasze March 9, 2026 06:05 View session

Copilot AI reviewed Mar 9, 2026

View reviewed changes

praasz approved these changes Mar 9, 2026

View reviewed changes

olpipi approved these changes Mar 9, 2026

View reviewed changes

mlukasze modified the milestones: 2026.1, 2026.2 Mar 16, 2026

olpipi enabled auto-merge March 17, 2026 11:18

olpipi disabled auto-merge March 17, 2026 11:18

Fix MSVC max macro collision in property range check

4bd8315

om4rrr force-pushed the fix-negative-unsigned branch from 14ab1a3 to 4bd8315 Compare March 17, 2026 12:20

Merge branch 'master' into fix-negative-unsigned

c5f2efa

olpipi enabled auto-merge March 17, 2026 18:34

olpipi added this pull request to the merge queue Mar 17, 2026

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 17, 2026

StefaniaHergane added this pull request to the merge queue Mar 17, 2026

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 17, 2026

mlukasze added this pull request to the merge queue Mar 18, 2026

Merged via the queue into openvinotoolkit:master with commit 54e2018 Mar 18, 2026
210 checks passed

Conversation

om4rrr commented Jan 6, 2026 • edited by praasz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix

Tests

Ticket

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

om4rrr commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlukasze commented Jan 15, 2026

Uh oh!

om4rrr commented Jan 15, 2026

Uh oh!

Uh oh!

olpipi commented Jan 22, 2026

Uh oh!

om4rrr commented Jan 22, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

praasz commented Jan 23, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

om4rrr commented Mar 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

olpipi commented Mar 9, 2026

Uh oh!

om4rrr commented Mar 10, 2026

Uh oh!

om4rrr commented Mar 12, 2026

Uh oh!

mlukasze commented Mar 12, 2026

Uh oh!

praasz commented Mar 17, 2026

Uh oh!

om4rrr commented Mar 17, 2026

Uh oh!

praasz commented Mar 17, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

om4rrr commented Jan 6, 2026 •

edited by praasz

Loading

om4rrr commented Jan 15, 2026 •

edited

Loading