Crash creating Autotimer that only matches when description contains a certain string.

[BrianG61UK]

I think what's happening is that the string entered is supposed to be a regular expression.
However, I wanted to match "(S5" which is an invalid regular expression.
It seems to accept it if I input "\(S5" instead so that too points to a regular expression being required here.

Crash logs:
65467
65466

Allowing a regular expression for this feature is a great idea, however I think ideally an invalid regular expression should not cause a crash. But I'm not sure how I would want it dealt with.

Later: Yes, I now realize "\(S5" won't do what I want.

[Huevos]

Please explain post an example of the string you are trying to match and the contents of the description field.

And I agree that bad user input should not bring the house down.

[BrianG61UK]

The program in question has a description field something like this:

Blah blah blah blah ... blah blah (S1 E7/22).

Where the part at the end indicates, in this case, season 1, episode 7 out of 22.

I was hoping to just record the most recent season 5 if it gets shown again at some point. They seem to be working through from the beginning again at the moment.

I've now put this in the string to match "\(S[5-9]" (without the quotes) which I think will match season 5 through season 9 to catch season 5 and some possible new seasons they might make in the future.

[birdman]

Allowing a regular expression for this feature is a great idea,
I suspect it isn't - particularly if there is no indication that a regular expression is what is expected.

I'm not aware of regular expressions ever being used (but will have a look).

You can use filters to further trim the original matches.

[birdman]

I'm not aware of regular expressions ever being used (but will have a look).No sign of regexes being used.
The code does a search of the EPGcache, and all it does is use strncasecmp() or memcmp().

(There is some poor coding there though, as having set up an enum for the types of a query you then have things like if (querytype == 1)!).

[BrianG61UK]

Okay. I was beginning to realize that regular expressions are too complicated for most users anyway.

Then I think the crash is caused by something in


https://github.com/oe-alliance/enigma2-plugins/blob/master/autotimer/src/AutoTimerComponent.py

that's trying to use regular expressions (on line 149) for unknown reasons.

-

Has anyone been able to reproduce this crash?
If not, maybe I messed up something in my build.

[Huevos]

No sign of regexes being used.
The code does a search of the EPGcache, and all it does is use strncasecmp() or memcmp().

(There is some poor coding there though, as having set up an enum for the types of a query you then have things like if (querytype == 1)!).


Yes, regular expressions are used and there is no fail safe if the user enters junk.


def setInclude(self, include):
if include:
self._include = (
[re_compile(x) for x in include[0]],
[re_compile(x) for x in include[1]],
[re_compile(x) for x in include[2]],
include[3]
)
else:
self._include = ([], [], [], [])

[Huevos]

@Brian

Can you print include?


def setInclude(self, include):
print("DEBUG include", include)
if include:
self._include

[BrianG61UK]

print added.
New logs:
65469
65470

[Huevos]

print added.
New logs:
65469
65470


Python 3.10.4 (tags/v3.10.4:9d38120, Mar 23 2022, 23:13:41) [MSC v.1929 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>>
>>> from re import compile as re_compile
>>>
>>> str = "Blah blah blah blah ... blah blah (S1 E7/22)."
>>>
>>> include = ([], [], ['\(S[5-9]'], [])
>>>
>>> out = ([], [], [re_compile(x) for x in include[2]], [])
>>>
>>> for include in out[2]:
... if include.search(str):
... print("Found")
... else:
... print("Not found")
...
Not found
>>>
>>>
>>> str = "Blah blah blah blah ... blah blah (S6 E7/22)."
>>>
>>> include = ([], [], ['\(S[5-9]'], [])
>>>
>>> out = ([], [], [re_compile(x) for x in include[2]], [])
>>>
>>> for include in out[2]:
... if include.search(str):
... print("Found")
... else:
... print("Not found")
...
...
Found
>>>
So, what is not working for you when you add the escape?

[BrianG61UK]

I don't know if it works or not with a valid regular expression (with the bracket escaped).
I was initially concentrating on reporting the crash, which seems to occur when the string isn't a valid regular expression.

If you look at the crash in either log you see:

DEBUG include ([], [], ['(S[5-9]'], [])

Just before the crash, with an unescaped '('.

Is there something you don't understand about what I'm reporting?
Does it seem to work fine for you, and you can't reproduce it on an official OpenViX image?

[birdman]

Yes, regular expressions are used and there is no fail safe if the user enters junk.Ah, sorry. I missed those. Seems to have been there for 15 years. I was playing around in this area a few years back and don't recall noticing that regexes were in use.

So two bugs here.


The code needs to do something (and not crash) if the regex is invalid (Needs to be done at setting time*).
The user needs to be told this is a regex


* possibly in editFilterCallback() (in AutoTimerEditor.py)

[BrianG61UK]

So, what is not working for you when you add the escape?

Just realized, you're probably referring to:

Later: Yes, I now realize "\(S5" won't do what I want.

I was thinking the 5 means repeat 5 times. But I don't think it does, and it would probably work.

[Huevos]

Just realized, you're probably referring to:


I was thinking the 5 means repeat 5 times. But I don't think it does, and it would probably work.

5x of the "S" would be:

"\(S{5}

But, yes, I already agreed this should not crash. There should be a test of the values on save.

[birdman]

And, in one of those oddities that seems to occur so often (although it's just that of it didn't you wouldn't notice anything...) I've now had reason to use this.

the cricket highlights are being sent out as "Today at the Test" (day 1) and "Today at the Ashes" (days 2 to 5).

So I'm matching "Today at the" then want to include if Test or Ashes is there. But includes must all be present for a match (excludes fail instantly on a match). So the solution is to have once include that is "Test|Ashes".

Which may be why it it a regex.

PS: I think a fix is still required?

EDIT: So I'll have a look and make one up....

[birdman]

I've got the test in place, but am having issues trying to get a MessageBox up about any error (as I'm in a ConfigXXX routine at the time).

Even when this is in place I'll need to look at how OpenWebIF sets new ATs as well.

[birdman]

I've now got any errors being reported.

However, there are two files changed.

config.py (in enimga2)
AutoTimerEditor.py (in enigma2-plugins)


Is that enigma2-plugins git only for Vix? If not, I'll have to modify AutoTimerEditor.py to revert to the old behaviour if the addition to config.py isn't available.

[Huevos]

What edition to config.py? We are not going to hack that for a broken plugin.

enigma-plugins is an open repo for everyone.

[birdman]

What edition to config.py? We are not going to hack that for a broken plugin.It adds a ConfigRegex entry, which is a ConfigText object but with a check that it is a valid regex in setValue().

I could try adding it all in AutoTimerEditor.py. My first attempt failed, but I seem to recall having similar issues in EPGTranslator, so could look at what I did there.

The plugin is only "broken" to the extent that it contains a bug. Fixing bugs is surely what we should be doing?

[Huevos]

So what produces the warning, config,py or the plugin?

[birdman]

So what produces the warning, config,py or the plugin?Neither. StartEnigma2 complains about an incorrect number of args in a callback. It's related to whether "self" is an arg in an object call. It didn't seem to be an issue in EPGTranslator in the end, but there was no callback involved there.

Anyway, it's now all in AutoTimerEditor.py and looks much better. It now adds a ConfigRegex class there instead (which is an extension of ConfigText).

[Huevos]

it's now all in AutoTimerEditor.py and looks much better. It now adds a ConfigRegex class there instead (which is an extension of ConfigText).Ok, that sounds like a good idea.

Can I see the changes please?

[birdman]

Can I see the changes please?Yes. But (I also think) I've worked out how to handle the OpenWebIF interface too, so will get that working first (later tonight??).

This is the patch for the enigma2 GUI:


--- AT-Regex/AutoTimerEditor.py 2023-06-20 15:18:34.251932420 +0100
+++ Enigma-Packages/enigma2-plugins/autotimer/src/AutoTimerEditor.py 2022-09-03 01:17:12.291969535 +0100
@@ -40,11 +40,6 @@
# Default Record Directory
from Tools import Directories

-# So we can check for a valid regex
-#
-from re import compile as re_compile
-from sys import version_info as pv_info
-
# Tags
try:
from Screens.TagEditor import TagEditor
@@ -1104,47 +1099,6 @@
def retval(self):
return self.returnVal

-# Extend the ConfigText class so we can check that we have a valid
-# regular expression before saving it.
-#
-class ConfigRegex(ConfigText):
- def __init__(self, **kwargs):
- if "err_timeout" in kwargs:
- self.err_timeout = kwargs["err_timeout"]
- del kwargs["err_timeout"]
- else:
- self.err_timeout = 5
- if "rpt_session" in kwargs:
- self.rpt_session = kwargs["rpt_session"]
- del kwargs["rpt_session"]
- else:
- self.rpt_session = None
- ConfigText.__init__(self, **kwargs)
-
- def setValue(self, val):
- try:
- dnc = re_compile(val)
- except Exception as ex:
- import traceback
- estr = traceback.format_exception_only(ex)
- errm = (_("The filter must be a valid regular expression.\n") +
- _("%s is not valid.") % val +
- "\n\n" + estr[0] + "\n" +
- _("See: ") +
- "https://docs.python.org/%d.%d/library/re.html" % (pv_info.major, pv_info.minor)
- )
- if self.rpt_session:
- self.rpt_session.open(
- MessageBox,
- errm,
- type=MessageBox.TYPE_ERROR,
- timeout=self.err_timeout
- )
- else:
- print("[AutoTimerEditor::ConfigRegex]", errm)
- return
- ConfigText.setValue(self, val)
-

class AutoTimerFilterEditor(Screen, ConfigListScreen):
"""Edit AutoTimer Filter"""
@@ -1333,7 +1287,7 @@
if self.typeSelection.value == "day":
entry = getConfigListEntry(text, NoSave(ConfigSelection(choices=weekdays)))
else:
- entry = getConfigListEntry(text, NoSave(ConfigRegex(rpt_session=self.session,fixed_ size=False)))
+ entry = getConfigListEntry(text, NoSave(ConfigText(fixed_size=False)))

list.insert(pos, entry)
self["config"].setList(list)

[Huevos]

Patch looks reversed.

[birdman]

Patch looks reversed.So it is, but it was just to show the changes.

[birdman]

This is the full patch (the correct way round), including the changes to AutoTimerResource.py so that AT settings from OpenWebIF get checked too (and any error is reported to the Web page).
I'll submit a PR if there are no adverse comments....


--- my-gits/enigma2-plugins/autotimer/src/AutoTimerEditor.py 2022-11-02 12:43:17.325607470 +0000
+++ AT-Regex/AutoTimerEditor.py 2023-06-21 00:51:11.698200471 +0100
@@ -40,6 +40,11 @@
# Default Record Directory
from Tools import Directories

+# So we can check for a valid regex
+#
+from re import compile as re_compile
+from sys import version_info as pv_info
+
# Tags
try:
from Screens.TagEditor import TagEditor
@@ -1099,6 +1104,59 @@
def retval(self):
return self.returnVal

+# Check that a string/list of strings is/are valid regular expressions.
+# Returns the error message on an invalid regex.
+# Returns an empty string if all is OK
+#
+def CheckValidRegex(str_arg):
+ if not isinstance(str_arg, (list, tuple)):
+ str_arg = (str_arg,) # Make a tuple from a single string
+ try:
+ for val in str_arg:
+ dnc = re_compile(val)
+ except Exception as ex:
+ import traceback
+ estr = traceback.format_exception_only(ex)
+ errm = (_("The filter must be a valid regular expression.\n") +
+ _("%s is not valid.") % val +
+ "\n\n" + estr[0] + "\n" +
+ _("See: ") +
+ "https://docs.python.org/%d.%d/library/re.html" % (pv_info.major, pv_info.minor)
+ )
+ return errm
+ return ""
+
+# Extend the ConfigText class so we can check that we have a valid
+# regular expression before saving it.
+#
+class ConfigRegex(ConfigText):
+ def __init__(self, **kwargs):
+ if "err_timeout" in kwargs:
+ self.err_timeout = kwargs["err_timeout"]
+ del kwargs["err_timeout"]
+ else:
+ self.err_timeout = 5
+ if "rpt_session" in kwargs:
+ self.rpt_session = kwargs["rpt_session"]
+ del kwargs["rpt_session"]
+ else:
+ self.rpt_session = None
+ ConfigText.__init__(self, **kwargs)
+
+ def setValue(self, val):
+ errm = CheckValidRegex(val)
+ if errm != "":
+ if self.rpt_session:
+ self.rpt_session.open(
+ MessageBox,
+ errm,
+ type=MessageBox.TYPE_ERROR,
+ timeout=self.err_timeout
+ )
+ else:
+ print("[AutoTimerEditor::ConfigRegex]", errm)
+ return
+ ConfigText.setValue(self, val)

class AutoTimerFilterEditor(Screen, ConfigListScreen):
"""Edit AutoTimer Filter"""
@@ -1287,7 +1345,7 @@
if self.typeSelection.value == "day":
entry = getConfigListEntry(text, NoSave(ConfigSelection(choices=weekdays)))
else:
- entry = getConfigListEntry(text, NoSave(ConfigText(fixed_size=False)))
+ entry = getConfigListEntry(text, NoSave(ConfigRegex(rpt_session=self.session,fixed_ size=False)))

list.insert(pos, entry)
self["config"].setList(list)
--- my-gits/enigma2-plugins/autotimer/src/AutoTimerResource.py 2022-11-02 12:43:17.325607470 +0000
+++ AT-Regex/AutoTimerResource.py 2023-06-21 00:49:39.892327324 +0100
@@ -16,6 +16,8 @@

from .AutoTimerSettings import getAutoTimerSettingsDefinitions

+from .AutoTimerEditor import CheckValidRegex
+
API_VERSION = "1.7"


@@ -410,6 +412,10 @@
description.remove('')
while '' in dayofweek:
dayofweek.remove('')
+# These must be valid regexes
+ errm = CheckValidRegex(title + shortdescription + description)
+ if errm != "":
+ return self.returnResult(req, False, errm)
timer.include = (title, shortdescription, description, dayofweek)

# Excludes
@@ -431,6 +437,10 @@
description.remove('')
while '' in dayofweek:
dayofweek.remove('')
+# These must be valid regexes
+ errm = CheckValidRegex(title + shortdescription + description)
+ if errm != "":
+ return self.returnResult(req, False, errm)
timer.exclude = (title, shortdescription, description, dayofweek)

tags = getA("tag")

[birdman]

Although I might change CheckValidRegex to be CheckREList and always send it a list, rather than getting the function to check what it has been given.
And simplify the keyword extraction for ConfigRegex, by just putting it in the parameter list.

[birdman]

I've submitted a PR (with the changes mentioned in the previous post).

https://github.com/oe-alliance/enigma2-plugins/pull/604

[birdman]

I've submitted a PR (with the changes mentioned in the previous post).And have now received an email saying it failed a quality test as:


Add 1 missing arguments; 'format_exception_only' expects 2 positional arguments.
Bug
However, Since Python 3.10 it doesn't required 2 args and if I did give it a second one it would ignore the first one (which is the relevant one) for "backward compatibility". (The 1-arg version does work on Vix 6.4.xxx).

So I reckon the auto-test is wrong. Since I'm assuming all users of this will be by Python >= 3.10.

EDIT: Although I can confirm it is an issue on Python3.9.

EDIT2: I now have a version that works on 3.9 and 3.11 (using the old 3.9 syntax). Is it worth resubmitting?

[birdman]

New PR with a simpler way to get the exception error text, which works everywhere.

https://github.com/oe-alliance/enigma2-plugins/pull/605