fix(db): guard string_to_datetime against non-string input#201
Draft
clewdotcc wants to merge 1 commit into
Draft
fix(db): guard string_to_datetime against non-string input#201clewdotcc wants to merge 1 commit into
clewdotcc wants to merge 1 commit into
Conversation
Rekordbox 6 has been observed to store non-string values (e.g. integer
0) in DateTime columns such as DjmdAlbum.created_at and updated_at.
Without a type-guard, the unconditional `value.endswith("Z")` call
crashes with `AttributeError: 'int' object has no attribute 'endswith'`
at ORM result-set materialization time, breaking any query that
touches the affected row (e.g. session.query(DjmdAlbum).all()).
Type-guard the function: return None for non-string input. This is
consistent with DateTime.process_result_value, which already returns
Optional[datetime] when the raw column value is falsy — so downstream
callers of the TypeDecorator already handle a None datetime.
Before:
>>> from pyrekordbox.masterdb.models import string_to_datetime
>>> string_to_datetime(0)
AttributeError: 'int' object has no attribute 'endswith'
After:
>>> string_to_datetime(0)
>>> # None — treated as missing value
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
pyrekordbox.masterdb.models.string_to_datetime(also exposed aspyrekordbox.db6.tables.string_to_datetimevia the back-compat re-export) callsvalue.endswith("Z")without type-guardingvalue. Rekordbox 6 has been observed in the wild to store non-string values (specifically integer0) inDateTimecolumns — for exampleDjmdAlbum.created_at/updated_at.When this happens, the function raises
AttributeError: 'int' object has no attribute 'endswith'at SQLAlchemy result-set materialization time. This is not a recoverable per-row error — a single corrupt row crashes the entire query:session.query(DjmdAlbum).all(),.first(), etc. all fail.Reproduction
Fix
Type-guard the function: return
Nonefor non-string input. This treats malformedDateTimecolumns as "no date" rather than crashing the consuming query.This is consistent with the existing
DateTime.process_result_valuebehavior, which already declares-> Optional[datetime]and returnsNonewhen the raw column value is falsy. The return-type annotation ofstring_to_datetimeis widened fromdatetimetoOptional[datetime]to honestly describe the newNonereturn path; the only in-repo caller (DateTime.process_result_value) already handlesOptional[datetime].Before / After
Behavior on valid string input is unchanged. Behavior on empty string
""is also unchanged (still raisesValueErrorfromdatetime.fromisoformat(""), same as before) — in practice theDateTimeTypeDecorator's truthy guard atprocess_result_valuealready short-circuits this path, so the empty-string semantic is irrelevant in production but is preserved here for strict backward-compat.Tests
Added
test_string_to_datetime_returns_none_for_non_string_inputintests/test_masterdb.py, parametrized over0,None, and a larger integer. The test fails onmaster(raisesAttributeError) and passes with this patch.Scope
Limited to
pyrekordbox/masterdb/models.py. Side-find:pyrekordbox/devicelib_plus/models.pyhas a byte-identicalstring_to_datetimewith the same vulnerability. I kept it out of this PR to keep the review focused, but happy to extend the PR or open a parallel one — whichever you prefer.Backward compatibility
ValueError).int,None, ...): previously crashed, now returnNone.datetimetoOptional[datetime]. The only in-repo caller already handlesOptional[datetime]; external callers that rely on a non-None return for non-string inputs were already encounteringAttributeError, so this is a strict improvement.