Opened 2 years ago

Closed 2 years ago

Last modified 16 months ago

#13056 closed Bug Report - General (fixed)

database error in AddRecordSchedule returns 'ok'

Reported by: Jeff Dwork <jeffdwork47@…> Owned by: Bill Meek
Priority: minor Milestone: 29.0
Component: MythTV - Services API - Backend Version: 0.28.1
Severity: medium Keywords:
Cc: Ticket locked: no

Description

A POST to AddRecordSchedule? with null StartTime? causes a database error but returns 'ok' to poster:

snip from post:

 &StartTime=&EndTime=&

snip from log:

#012Driver error was [2/1048]:#012QMYSQL3: Unable to execute statement#012Database error was:#012Column 'starttime' cannot be null
Jun 14 23:12:25 vcr2 mythbackend: mythbackend[1185]: I HttpServer51 httprequest.cpp:368 (SendResponse) HTTPRequest::SendResponse(xml/html) () :200 OK -> 127.0.0.1: 8

Attachments (2)

sample.log (2.6 KB) - added by Bill Meek 2 years ago.
backend.log (4.8 KB) - added by Jeff Dwork <jeffdwork47@…> 2 years ago.
log of complete transaction

Download all attachments as: .zip

Change History (15)

Changed 2 years ago by Bill Meek

Attachment: sample.log added

comment:1 Changed 2 years ago by Bill Meek

Status: newinfoneeded_new

Please do: mythbackend --setverbose http and then attach the output from the POST. It should look similar to the 1st attachment. Should be easy to test (for me) but I'm getting a 500 error and proper logging of the empty times. The tests in libs/libmythtv/recordingrule.cpp haven't been touched since 2013.

comment:2 Changed 2 years ago by Roger Siddons

For search rules , eg PowerSearch?, RecordingRule::IsValid? doesn't validate the times (because they're N/A) but the Save still tries to write them to the Db.

comment:3 Changed 2 years ago by Bill Meek

Roger, I see it, thanks.

Jeff, in addition to the log requested above, please add the actual response from the backend (not just the 200/OK.)

When I tested as a Title Search, I see the DB Error, but the response is -1 (or actually: {"uint": "4294967295"}.)

Changed 2 years ago by Jeff Dwork <jeffdwork47@…>

Attachment: backend.log added

log of complete transaction

comment:4 Changed 2 years ago by Jeff Dwork <jeffdwork47@…>

I've attached the section of the log showing the complete transaction.

comment:5 in reply to:  2 Changed 2 years ago by gigem

Replying to rsiddons:

For search rules , eg PowerSearch?, RecordingRule::IsValid? doesn't validate the times (because they're N/A) but the Save still tries to write them to the Db.

The schema should probably allow NULLs is such cases. That's something we generally haven't done, though. I also wonder how far such a change would ripple.

comment:6 Changed 2 years ago by Bill Meek

Status: infoneeded_newassigned

Just looking at rules created from the frontend (e.g. Power Search, Record One) and they include a valid start/end/chanid/station in the DB.

Would removing the !isSearch test here: https://code.mythtv.org/cgit/mythtv/tree/mythtv/libs/libmythtv/recordingrule.cpp#n940 be valid?

comment:7 Changed 2 years ago by gigem

I assume you mean leaving the start/end, duration, chanid and station checks, then? I'd prefer trying the allow NULL solution first before requiring the user to provide valid values to irrelevant columns.

comment:8 in reply to:  7 Changed 2 years ago by Bill Meek

Replying to gigem:

I assume you mean leaving the start/end, duration, chanid and station checks, then? I'd prefer trying the allow NULL solution first before requiring the user to provide valid values to irrelevant columns.

Yes, I meant leaving the tests, making them always required.

If I allow NULLs, then wouldn't that be a "master only" solution because it would be a schema change?

ALTER TABLE record MODIFY COLUMN startdate DATE DEFAULT NULL
ALTER TABLE record MODIFY COLUMN enddate DATE DEFAULT NULL
ALTER TABLE record MODIFY COLUMN starttime TIME DEFAULT NULL
ALTER TABLE record MODIFY COLUMN endtime TIME DEFAULT NULL

And, did you want to convert all k(OneRecord,AllRecord,DailyRecord, WeeklyRecord) existing entries to NULL for the 4 columns above?

UPDATE record SET starttime=NULL, endtime=NULL WHERE search IN (2, 4, 5, 6)
UPDATE record SET startdate=NULL, enddate=NULL WHERE searchtype IN (2, 4, 5, 6)

comment:9 Changed 2 years ago by gigem

The conversion of old rules probably isn't necessary. I don't think it would hurt anything, but I don't think it would help anything either.

comment:10 Changed 2 years ago by Bill Meek

Milestone: unknown29.0

comment:11 Changed 2 years ago by Bill Meek <bmeek@…>

Resolution: fixed
Status: assignedclosed

In a29633136dcffff10b74d877fcac2cf14bc863d5/mythtv:

Services API: Add/UpdateRecordSchedule?, prevent DB Errors when timestamps aren't passed

Because they aren't required for SearchTypes? other than None and Manual,
the start/end date/time, weren't being validated. If StartTime? and EndTime?
prarmeters weren't passed, then when an INSERT/UPDATE was done, the NULL
fields caused a DB Error and the API returned -1 ({"uint": "4294967295"}.)

This fix allows NULLs in the start/end date/time in the record table.

Fixes #13056

comment:12 Changed 2 years ago by Jeff Dwork <jeffdwork47@…>

I agree that allowing NULLS in these fields is a good thing and will prevent this error, but I think there is still a bug. A database error is not being returned to the initiator. This fix prevents a particular database error, but there might be others in the future.

comment:13 Changed 16 months ago by Bill Meek

Owner: changed from Bill Meek to Bill Meek
Note: See TracTickets for help on using tickets.