Skip to content

fix(util): guard _datetime_from_weaviate_str against empty string - #2053

Merged
g-despot merged 5 commits into
weaviate:mainfrom
devteamaegis:fix/indexerror-empty-datetime-str
Sep 8, 2026
Merged

fix(util): guard _datetime_from_weaviate_str against empty string#2053
g-despot merged 5 commits into
weaviate:mainfrom
devteamaegis:fix/indexerror-empty-datetime-str

Conversation

@devteamaegis

Copy link
Copy Markdown
Contributor

What's broken

_datetime_from_weaviate_str in weaviate/util.py evaluates string[-1] on line 754 without checking whether string is empty. Python's negative indexing on an empty string raises IndexError. The protobuf wire-format default for a string field is "", so any gRPC query response containing an object with an unset date property crashes the client during deserialization in base_executor.py.

Traceback: IndexError: string index out of range at weaviate/util.py line 754.

Why it happens

The function assumes the input is a non-empty datetime string and immediately indexes from the end, which is invalid for the empty-string protobuf default.

Fix

Added a two-line guard at the top of _datetime_from_weaviate_str: if string is empty, return datetime.min. This matches the existing behaviour for year-zero dates and avoids any semantic change for valid inputs.

Test

Added ("", datetime.min) as a parametrized case in the existing test_datetime_from_weaviate_str test in test/test_util.py. All 40 tests pass.

Fixes #2052

Empty string is the protobuf wire-format default for an unset string
field. Calling string[-1] on "" raises IndexError, crashing gRPC
deserialization for any object with an unset date property. Return
datetime.min for empty input, matching the existing behaviour for
year-zero dates.

Fixes weaviate#2052

@orca-security-eu orca-security-eu Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed SAST high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

@weaviate-git-bot

Copy link
Copy Markdown

To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge.

beep boop - the Weaviate bot 👋🤖

PS:
Are you already a member of the Weaviate Forum?

Comment thread weaviate/util.py Outdated

def _datetime_from_weaviate_str(string: str) -> datetime.datetime:
if not string:
return datetime.datetime.min

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wouldn't "None" be a better return here?

If we have properties I would expect None if it is unset in Weaviate

@devteamaegis
devteamaegis force-pushed the fix/indexerror-empty-datetime-str branch from bcfe5e9 to 6987d13 Compare June 11, 2026 19:46
Per review (@dirkkul): an unset date property should surface as None,
not a datetime.min sentinel. Empty-string input now returns None;
year-0 out-of-range handling is unchanged. Callers propagate the value
as a property (typed Any / Optional[List[Any]]) so None flows through.
@devteamaegis

Copy link
Copy Markdown
Contributor Author

Good call @dirkkul — changed it to return None for an unset/empty datetime instead of datetime.min. That matches the "unset property should be None" expectation, and it flows through safely: both callers in base_executor.py propagate the value as a property (Any / Optional[List[Any]]), neither does date math on it. Updated the return type to Optional[datetime.datetime] and the test case to expect None. Left the year-0 → datetime.min path untouched since that's separate existing behavior with its own warning.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The implementation/test behavior for empty strings (None) conflicts with the PR description’s stated behavior (datetime.min), so the intended semantics need to be made consistent.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens datetime deserialization in weaviate/util.py to prevent an IndexError when Weaviate returns an unset date property as the protobuf default empty string (""), and extends the existing unit test coverage for this edge case.

Changes:

  • Add an early guard in _datetime_from_weaviate_str for empty strings.
  • Adjust _datetime_from_weaviate_str’s return annotation and behavior to allow an “empty value” outcome.
  • Extend test_datetime_from_weaviate_str with an empty-string parameterized case and update expected type accordingly.
File summaries
File Description
weaviate/util.py Adds an empty-string guard and changes the helper’s return contract to allow a sentinel for unset date values.
test/test_util.py Adds a test case for "" and updates the expected type to match the new behavior.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread weaviate/util.py
Comment on lines +753 to +755
def _datetime_from_weaviate_str(string: str) -> Optional[datetime.datetime]:
if not string:
return None
@g-despot

g-despot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Hi @devteamaegis! 😄
Could you please write a comment that you agree to our CLA so we can continue with this PR?

@devteamaegis

Copy link
Copy Markdown
Contributor Author

Hi @g-despot! I have read the Weaviate Contributor License Agreement and I hereby agree to its terms. Also pushed a small ruff format fix so the linter check passes — should be good to continue now. Thanks!

…ently

An unset date property arrives as null_value and never reaches this path, so an
empty string can only be a malformed value from the server. Emit Con006 rather
than dropping it quietly, and cover the scalar and array paths against a mock
gRPC response.
@g-despot
g-despot merged commit f336b6d into weaviate:main Sep 8, 2026
247 of 249 checks passed
@g-despot

g-despot commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Thanks a lot for fixing the issue and kudos on your first Weaviate contribution! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: IndexError in _datetime_from_weaviate_str when date property is unset (empty string)

5 participants