Suggested analyzer re channel subscribe

[mgravell]

scenario; application takes user-supplied channels, and they were using subscribe; they expected discreet channels, but a user-entered channel used a wildcard, and SE.Redis dutifully does a PSUBSCRIBE via the default / implicit behaviour ; bad things and possible infosec

I don't think we can change the behaviour at this point without breaking lots of things, however I see a few possible routes forward:

option 1

a global public static bool UseImplicitAutoPattern {get;set;} = true; - if set to true, then the conversion operators and any constructors that do not take a PatternMode would use PatternMode.Literal rather than PatternMode.Auto; example:

- return new RedisChannel(whatever, PatternMode.Auto);
+ return new RedisChannel(whatever, DefaultPatternMode);
+
+ public static bool UseImplicitAutoPattern {
+     get => DefaultPatternMode == PatternMode.Auto;
+     set => DefaultPatternMode = value ? PatternMode.Auto : PatternMode.Literal;
+ }
+ // keep this simple, since accessed more frequently
+ private static PatternMode DefaultPatternMode {get;set;} = PatternMode.Auto;

simple and quick, but not very nuanced - global setting, etc; anyone explicitly using PatternMode.Auto would not be impacted; we could also add some helper creation methods

option 2

we write an analyzer that spots and of the same constructors / operators, excluding any scenarios where the input is a literal that does not contain a glob-like pattern (so: only non-constant expressions, or literals that are glob-like), that adds an info-level analyzer output saying "hey, you might want to specify whether this is meant to be literal or pattern based"

examples:

foo.Subscribe("abc", ...); // no analyzer output; literal and not glob-like
RedisChannel x = "abc"); // no analyzer output; literal and not glob-like

foo.Subscribe("abc*", ...); // add output
RedisChannel x = "abc*"; // add output

foo.Subscribe(someStringChannel, ...); // add output
RedisChannel x = someStringChannel; // add output

foo.Subscribe(some + expression, ...); // add output
RedisChannel x = some + expression; // add output

foo.Publish(literallyAnythingHere, ...); // no analyzer output: publish is never pattern-based
var x = new RedisChannel(literallyAnythingHere, anyPatternModeHere); // they made a choice

option 3

we do option 1 as a quick fix, and follow up with option 2 in due course

option 4

in combination with option 1, we mark the implicit conversion operators as

[Obsolete("You might want to specify a " + nameof(PatternMode), error: false)]

and then later un-obsolete them when/if we ship the same via an analyzer (so: "all of the above")

Thoughts...

---

Also included in this topic: expose the "is pattern based" flag via an instance accessor