slimserver icon indicating copy to clipboard operation
slimserver copied to clipboard

__requestRE should match words provided to Slim::Control::Request::subscribe

Open earlchew opened this issue 4 years ago • 1 comments

Presently, the documentation for Slim::Control::Request::subscribe reads:

 Optionally, the subscribe routine accepts a filter, which limits calls to the
 subscriber callback to those requests matching the filter. The filter is
 in the form of an array ref containing arrays refs (one per dispatch level)
 containing lists of desirable commands (easier to code than explain, see
 examples below).

The implementation of __requestRE simply splices the words together:

$regexp .= ',' if $i++;
$regexp .= (scalar @$names > 1) ? '(?:' . join('|', @$names) . ')' : $names->[0];

The result is unsurprising if each word is not a substring of another word. For example, rescan is not a substring of client.

The result is surprising when a word happens to be a substring of another word. For example play is a substring of display and playlist.

A couple of ways suggest themselves to address this ambiguity:

  1. Do nothing.
  • Clients are required to be prepared for imprecision in the filter, and each callback must check the received notification for specific matches as required.
  1. Change __requestRE to bracket each word with \b. For example, '\\b' . $names->[0] . '\\b'.
  • Clients that presently call Slim::Control::Request::subscribe using regexps, instead of plain words, might break.
  1. Sanction the use of regexps by clients using Slim::Control::Request::subscribe. For example, Slim::Control::Request::subscribe(\&callback, [ [ "\\bplay\\b" ] ]).
  • Clients must know to apply word boundaries, or know the set of commands to know if words are potentially substrings of other words, or be prepared to check the received notification for specific patterns.
  1. Provide a new method (eg Slim::Control::Request::subscribeNotification) with clearer semantics, and do not modify the ambiguous behaviour in Slim::Control::Request::subscribe.
  • Clients can migrate to Slim::Control::Request::subscribeNotification as required.

What do you think is the preferred approach?

earlchew avatar Jun 21 '21 14:06 earlchew

I somehow missed this one. I'd prefer suggestion #2 - if you wanted to provide a patch. It's not high priority for me, though.

mherger avatar Apr 12 '22 05:04 mherger