Fix wiz.bin DB crashes when a description is longer than 32KB

seansd-zz commented 6 years ago

Don't write to Show desc data via bytes when the length is greater than 32KB, instead opting to use the desc string. I am creating the PR to create the discussion. . .I'm pretty sure there should be something done on the read side as well around line 477, b/c it's calling readShort to get the size and I'm not sure that's good either, but not sure how to fix that. . .

googlebot commented 6 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

seansd-zz commented 6 years ago

I signed the CLA, sean.sd@gmail.com

Narflex commented 6 years ago

I'm totally baffled as to how this wasn't fixed before...I have various fixes in other places for handling large strings and I thought the original reason was because of large descriptions in the Show objects on imported MediaFiles...but anyways, clearly this part wasn't done correctly. The best way to fix this I think would be to just get rid of those byte arrays it's using and instead always read/write the description as a UTF. It'll then be backwards compatible and support adding of any new objects that have a description larger than 32K.

seansd-zz commented 6 years ago

We were wondering on the forums why the byte arrays were used at all. . .so good ok. I can try to address this in the PR as well but how should i tackle it? Just use descBytes only IFF there's no descStr? Or just get rid of descBytes completely?

seansd-zz commented 6 years ago

Line 472 is what concerns me. . . as I said, but assuming I can just use descStr always then I don't think you even need to do a readShort / readFully but not sure where descStr would come from then

seansd-zz commented 6 years ago

Also here's the link to the forums where we were discussing the issue: https://forums.sagetv.com/forums/showthread.php?t=65692

Narflex commented 6 years ago

I'm suggesting getting rid of descBytes completely and also episodeNameBytes as well while you're at it. They were there originally as a memory usage optimization since Java uses 16 bit chars...and most chars used for this data are within 8 bits....so these cut down on memory usage a bunch. Then just use read/writeUTF with the descStr and episodeNameStr vars and that should be good.

seansd-zz commented 6 years ago

:+1:

seansd-zz commented 6 years ago

@Narflex have a look, changes done, I left the the deprecated bytes properties for now just b/c I was worried of other unforseen side effects, but if you want me to remove them i can

seansd-zz commented 6 years ago

OK i'm going to submit a new PR and close this one so that I can get it cleaner and avoid formatting changes

google / sagetv