Comment 14 for bug 1659124

Created attachment 34698
Updated patch proposal including a warning message for characters that aren't allowed

(In reply to Mark Thomas from comment #10)
> Thanks for the updated patch. I like the overall design. Some detail
> comments:

No problem.

> - I think a different name is required. We might want to override other
> restrictions in the future. Maybe requestTargetAllow

That makes sense.

> - The docs need to state which characters are valid in the allowed list

Agreed.

> - What to do if some other invalid character is placed on the allowed list.
> Log a warning?

I thought about that but since there isn't any logging there at the moment I let it go. I think it's a good idea to log a warning though, so I'll add that.

> - I'm still undecided on whether this should be per connector configuration

That would be nice, but I haven't dug into the code enough to be able to quickly provide a patch for it.

> We also need to decide which versions to add this to. I currently thinking:
> - 7.0.x - yes
> - 8.0.x - yes

+1

> - 8.5.x - maybe

I'd vote yes on adding the option to 8.5.x because the stable version is already out and the behavior has changed. We obviously don't want to continue allowing broken clients to work, but I don't think we can change this behavior in a stable version, as evidenced by the users list complaints :)

> - 9.0.x - no

+1

I also noticed that the property being parsed was including the quotes, so I changed the commented out example accordingly.