Safelist - make is* methods public?

[snak3b1te]

I had the problem to verify complete HTML documents and provide detailed reporting on offending tags. No problem - just write a new Cleaner implementation with the changed behavior I thought. Well, not so much: Safelist cannot be used in that scenario because all test methods like #isSafeTag() and friends (#isSafeAttribute(), #getEnforcedAttributes()) are protected or default.
Cleaner can access them of course, being in the same package.
With the Java-9+ module system however, I cannot just create my own class in that same package as it belongs to a different module.
So, I have to write my own Safelist, too. (Or copy the source to some local package.) Which is kind of sad.
Could we make those test methods public so that Safelist could be used in custom code?

[susJohnny]

The public methods are added instead of modified, #isSafeTag() to be #testSafeTag(), #isSafeAttribute() to be #testSafeAttribute(), #getEnforcedAttributes() to be #enforcedAttributes()

[snak3b1te]

Brilliant! Thanks a lot.
You may want to consider renaming #testValidProtocol() for consistency to something else - it is currently private but could be mistaken for "one of the test methods". However, I don't see a benefit in making it accessible. As it doesn't show in the API doc. it might be pointless to change it, though.

[jhy]

Wouldn't it be better to make the methods only protected? So they can be overridden by extending classes outside of the package. They only make sense in the context of an extending class.

BTW, would @snak3b1te, I would be interested to know what your implementation of isSafeTag is - what extra functionality are you doing? Is it something that would be useful to add to the core jsoup?

[snak3b1te]

@jhy my interest was to create a custom cleaner. The cleaner needs access to the Safelist's methods to check tags to be safe. With protected they are not accessible from outside the package. I had no intention to change the behavior of Safelist, so no point in inheriting or overriding (IIRC cannot change visibility when overriding).
My cleaner differs from Jsoup's in that it can validate / sanitize complete documents (ie <head/> and <body/>) and that I return some XPath-like expression for invalid tags. I recon that this is rather specific for my use case and does not belong into the core.

[snak3b1te]

@jhy wanted to check back on the status of this request (as of version 1.16.1). With the Safelist.is* methods protected I can work around by extending Safelist and leaking access to the methods under a different name (not pretty though). Unfortunately, access to the enforced attributes is default i.e., accessible only from the same package. So, no dice. Still need to copy the class :-(

Another thing that bugs me: All methods to create or modify Safelists are public - only when you want to use it, that is all hidden away.

So, while my use case might be rather specific, it would allow re-use of the Safelist class, if those three (#isTagSafe(), #isAttributeSafe(), #getEnforcedAttributes()) methods would be accessible. It would allow to implement custom checkers and cleaners.

[jhy]

Thanks for the feedback - have made those three public.