Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prefix filter occasionally insufficient #36

Closed
commonquail opened this issue Feb 18, 2015 · 6 comments
Closed

Prefix filter occasionally insufficient #36

commonquail opened this issue Feb 18, 2015 · 6 comments
Assignees
@gmdfalk
Labels
enhancement

Comments

@commonquail
Copy link

Blocklist filtering is based on a prefix. This is typically fine but recently I've noticed a few situations where the desired filter is not a prefix. A substring search trivially solves this but is also potentially much more damaging, so that may not be a desired change; if it is, I imagine I can submit a PR.
@gmdfalk
Copy link
Owner

gmdfalk commented Feb 19, 2015

Hi commonquail,

substring searches are indeed a little volatile which is why i decided on prefixes.
A possible middle ground would be to introduce an option that lets people choose whether song matching should be prefix or substring based.
Before changing anything though, can you give me one or two examples of situations where the desired filter would not be a prefix?

Regards

@commonquail
Copy link
Author

The localized Filtr is the sinner. The exact format escapes my memory but something close to

FRESH LIGE NU Filtr
KLIK & LYT Filtr
SPIL NU Filtr
HITS Filtr

I do not remember if anything came after "Filtr".

By all accounts it's a minor issue.

@gmdfalk
Copy link
Owner

gmdfalk commented Feb 22, 2015

I imagine you've changed your code to do a substring search. Let me know if you experience any unexpected behaviour with that.
If you don't, i might as well merge the pull request. Otherwise, i'll introduce the option to enable substring search in addition to the prefix search.
That way, if the option is enabled and a prefix search found no match, the additional substring search kicks in.

@gmdfalk gmdfalk self-assigned this Feb 22, 2015
@commonquail
Copy link
Author

I actually don't listen to a lot of music so I might not be a good test subject. I haven't had any issues but I'm simply unlikely to have any.

commonquail pushed a commit to commonquail/blockify that referenced this issue Feb 24, 2015
[gmdfalk#36] Replace prefix search with substring search
9504844 
In a few rare situations the best blocklist filter string is not a
prefix of the song. A substring search will handle these anomalies
correctly, with the caveat that it is inevitably much less conservative.

At least four instances of a pattern similar to the following have been
observed:

    FRESH LIGE NU Filtr
    KLIK & LYT Filtr

Here, "Filtr" is the desired filter string but isn't a prefix. The real
issue here is that the prefix filter is trivially bypassed in theory.
While I have only one example of this, the pattern is straight-forward.

An alternative could be to optionally try either a substring search or a
suffix search after a failed prefix search, letting users opt in to the
more dangerous behaviour. Personally I have had no issues with
substrings but my music consumption simply isn't big enough for my
experience to have much weight.

Note that I don't remember if "Filtr" is actually a suffix in the
examples above. That is, I have forgotten if anything comes after it.
gmdfalk pushed a commit that referenced this issue Mar 15, 2015
Update to 1.8.0
2d2a3fe 
Fixed 3 issues: #36,#38, #39
New options:
substring_search (boolean, set to True if you want to be able to block
"Bloodhound Gang - Ivan Eth Nioj" with "Ivan". Risk for fals e
positives)
pacmd_muted_value (string, the localized value for "yes", needed to
parse the pacmd list-sink-inputs output correctly)
@gmdfalk
Copy link
Owner

gmdfalk commented Mar 15, 2015

Fixed in 1.8.0 by adding a new option to the [general] section of blockify.ini:
substring_search
Set to True to enable it.

@gmdfalk gmdfalk closed this as completed Mar 15, 2015
@commonquail
Copy link
Author

Thanks!

Further justification for posterity: my list now has these additional entries:

RUNNING HITS fra Filtr – ALTID KUN HITS I TOPFORM
FRESH LIGE NU fra Filtr – ALTID FØRST MED NY MUSIK

Given that the blocklist does not distinguish between artist and track, neither prefix nor suffix matching adequately addresses this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gmdfalk gmdfalk

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@gmdfalk @commonquail