jurialmunkey / plugin.video.themoviedb.helper

GNU General Public License v3.0
203 stars 96 forks source link

player file passing quotes inside plot and not literal case? #276

Closed 123Venom closed 4 years ago

123Venom commented 4 years ago

I'm having some breaks take place in my player file when passing plot from TMDBHelper. For example The Office S07E17 has "Goldenface" in the plot and when passed those quotes are inside the quotes of the entire plot and it breaks. Is there a way to get that passed as literal case \" ?

jurialmunkey commented 4 years ago

Output from TMDb API request shows quotes are already escaped:

Michael screens his action film Threat Level Midnight to the office after eleven years of writing, shooting, re-shooting, and editing. The film features Michael as Agent Michael Scarn, Dwight as Scarn's sidekick, and Jim as archnemesis \"Goldenface,\" as well as several people from Michael's past.

Are you sure that this isn't url decoding issue on the player addon's end?

123Venom commented 4 years ago

the plot is inside a meta str that when the function calls to the addon this is used...

params = dict(parse_qsl(argv[2].replace('?',''))) meta = params.get('meta')

I could not find a special suffix that would keep that literal for \". I've run into a few issues with others and _url was the cure but could find one for this condition.

Maybe instead of this %22plot%22%3A+%22{plot}%22% I use this %22plot%22%3A+%27{plot}%27%

Was worth a shot but the above failed on me as well. When I used json.load(meta) on that str with %22 I get this error TypeError -> 'NoneType' object is not callable

When I use %27 I get this error ValueError -> No JSON object could be decoded

This only happens with quoted items in the plot like the "Goldenface" example.

jurialmunkey commented 4 years ago

Have you tried {plot_escaped} ?

jurialmunkey commented 4 years ago

Ah just spotted that you're using json.loads. That's your problem - even if the " is escaped it will still throw an error because r'"' = '\"' (in a string a raw double quote is the same as an escaped double quote). You need to double escape for loads

Here's a quick test you can do in a python console which will demonstrate:

from json import loads
test1 = '{"key": "a long value for this key"}'
test2 = '{"key": "a long \"value\" for this key"}'
test3 = '{"key": "a long \\"value\\" for this key"}'
loads(test1)
loads(test2)
loads(test3)

First test will give you a dict: {'key': 'a long value for this key'}

Second test will error even though you escaped the doublequotes: json.decoder.JSONDecodeError: Expecting ',' delimiter: line 1 column 18 (char 17)

Third test will give you a dict: {'key': 'a long "value" for this key'}

Option is to use {plot_escaped} and then use urllib's unquote function on the string after it has been run through loads.

123Venom commented 4 years ago

Option is to use {plot_escaped} and then use urllib's unquote function on the string after it has been run through loads<

Sounds like such a great idea and I jumped all over it. I had tried _escpaed on it before but didn't think unquote would leave that literal so went ahead and tested it just now. Note I removed allot of other params from full meta dict to avoid bloat so we can only look at the plot item inside meta

{plot_escaped}, logged before json.loads(meta) As received from TMDBHelper, untouched...see code snip below logged before and after "plot": "Michael%20screens%20his%20action%20film%20Threat%20Level%20Midnight%20to%20the%20office%20after%20eleven%20years%20of%20writing%2C%20shooting%2C%20re-shooting%2C%20and%20editing.%20The%20film%20features%20Michael%20as%20Agent%20Michael%20Scarn%2C%20Dwight%20as%20Scarn%27s%20sidekick%2C%20and%20Jim%20as%20archnemesis%20%22Goldenface%2C%22%20as%20well%20as%20several%20people%20from%20Michael%27s%20past.",

logged after json.loads(meta) and unquote(meta_data) log_utils.log('meta = %s' % str(meta), __name__, log_utils.LOGDEBUG) if not isinstance(meta, dict): meta_data = json.loads(meta) else: meta_data = meta meta_data = unquote(meta_data) log_utils.log('meta_data = %s' % str(meta_data), __name__, log_utils.LOGDEBUG)

"plot": "Michael screens his action film Threat Level Midnight to the office after eleven years of writing, shooting, re-shooting, and editing. The film features Michael as Agent Michael Scarn, Dwight as Scarn's sidekick, and Jim as archnemesis "Goldenface," as well as several people from Michael's past.",

still appears to remove the literal and log now shows new error From func name: \resources\lib\modules\sources.py.get_season_info() Line # :1842 msg : AttributeError -> 'dict' object has no attribute 'split' Line 1842 = meta_data = unquote(meta_data) from above but what is unquote looking to split that fails? Is it a lack of literal again because it's seeing quotes inside quotes and the split point is hosed?

jurialmunkey commented 4 years ago

You can only unquote strings. Loads creates a dict. Either get the string and unquote it or iterate over the dict and unquote (depending on how you want to approach it).

e.g. if you only want to do this with specific key/value pairs do this:

meta_data = json.loads(meta)
if meta_data.get('plot'):
    meta_data['plot'] = unquote(meta_data.get('plot'))

or if you want to convert the whole dict do this:

meta_data = json.loads(meta)
for k, v in meta_data.copy().items():
    if isinstance(v, str):
        meta_data[k] = unquote(v)

EDIT: Another option I just thought of is you could replace the %22 with the literal before loads. Thinking about it, this might actually be the simplest approach.

meta_data = json.loads(unquote(meta.replace('%22', '\\"')))
123Venom commented 4 years ago

That last suggestion seems to have done the trick, no more log errors. I can't thank you enough for your time and patience!!!

123Venom commented 4 years ago

Here's a killer. That change now works great on a Windows or Shield setup. fireTV is throwing this error

msg : ValueError -> Expecting , delimiter: line 1 column 345 (char 344)

And I can't duplicate it here but the above sure looks to me like fireTV still blowing a gasket on the quotes because that's about the position

jurialmunkey commented 4 years ago

Here's a killer. That change now works great on a Windows or Shield setup. fireTV is throwing this error

msg : ValueError -> Expecting , delimiter: line 1 column 345 (char 344)

And I can't duplicate it here but the above sure looks to me like fireTV still blowing a gasket on the quotes because that's about the position

Ah, what a pain!

Okay let's scrap that approach entirely and instead I'll encode it via json.dumps() on my end.

Latest commit adds _meta keys which will run the values through json.dumps(). Because it is running through dumps, enclosing doublequotes will be added to the string - e.g. This is the \"plot\" of the movie becomes "This is the \\"plot\\" of the movie"

Which means you shouldn't add %22 on the value side of the key value pair in the metastring - e.g. %22plot%22%3A%22{plot_url}%22 becomes %22plot%22%3A{plot_meta_url}

_meta key is added for every encoding combo e.g. {plot_meta} {plot_meta_+} {plot_meta_url} {plot_meta_escaped} etc.

It will run dumps() before doing any url percent encoding - I'm pretty sure that's the right way around since I'd expect parse_qsl to happen before json.loads() on the other end.

Let me know how it goes.

123Venom commented 4 years ago

jurial I'm sooo sorry to say this....but...I literally have been attacking this issue for this user all night. Then we discovered the new player file I modded for {plot_escaped} he didn't get...why? idk and I could beat myself up all night over it. I sent him a new module to log the heck out of it tonight and the first thing I see is the meta passed is not escaped...what? That's how we noted he's got the wrong player file all along. I even tested here with just plain {plot} and bingo I now throw his same errors. I sent him the player file he should have received and retested..all seemed good now. I feel soooo bad bothering you with what seemed like a miscommunication after you gave that meta_data = json.loads(unquote(meta.replace('%22', '\\"'))) because with the player file using plot_escaped all was actually good.

jurialmunkey commented 4 years ago

Haha no worries! It happens to us all.

It was a simple piece of code to add the _meta versions of the keys. It doesn't appear to add much overhead to process them and at least now there's the option if anyone else needs to use them.

123Venom commented 4 years ago

Well I'm glad for that. After this discovery I felt so bad that I wasted your time down the final stretch when the user and I were right there on the fix, just a miscommunication on him having the {plot_escaped} change I made to the player file. I hope another user gets use of that _meta key you added. I now want to experiment with it but after an all night affair, not in a good way, I'm a bit spent for now. Thanks again jurial...top notch as always!!!