Suggestion for plugin enhancement

Description

Suggestion: let plugins register an icon to display on the router console homepage instead of the default jigzaw. Given that the plugin-mechanism is now so inviting with the load-from-file function I think it makes sense. Only small modifications of NavHelper?.java and PluginStarter?.java are required. And of course the introduction of a new property: console-icon.

would you please attach a diff or patch file for easier review, thanks

Patchfiles, slightly more presumptous than the diff files added. The code commented out could be removed I think. It compiles and runs in my local branch anyway. Although I haven't the Bote plugin installed. If that dev could be swayed to include an icon, the hardcoded hack could go as well.

If we introduce a way not do to that, we have to take height for them not having webresources, meaning they would have to refrain from using the property if they don't, or find a convoluted way to get to it.

I'm thinking it is a very rare scenario where the plugin does not have a webresource. Are they not more likely to be applications written to the streaming library without the router as a vehicle? If so I would say it is better to present the icon-opportunity as an extra benefit of webapplications within the router and stay with the default icon for other cases.

Option 3 feels like it would make a pretty mess with the code. I like it when changes make things lighter and clearer. Option 2 is cool - that one was not even in my cognitive function but perhaps overkill. So I would say option 1 is the most likely to make things better not worse.

But does that mean the plugin has to present a type-identification of sorts?

Off the top of my head, zzzot, i2pcontrol, i2phex, jwebcache, jircii, neodatis, and seedless don't have a webapp. Just guessing, not 100% sure. So almost half, and some of the more popular ones. Some offer a web server at a different port (not a webapp under the console), some fork off a different process.

The plugin code just looks for a webapp in the standard place, and loads it if there. See the plugin docs.

Now, if options 2 and 3 are too hard or too complex/clever or a bad idea, I'm fine with option 1. Or even that we check in what you've posted now, and offer some alternate solution later, if we get around to it. But let's decide that knowing that it doesn't work for everybody.

Ah, I think I finally get it. Anything without 'console/webapps' falls straight through (PluginStarter:359) but is still registered at (PluginStarter:423,425) if it has a link prop. (Took me 2 commutes)

But then option 2 suddenly seems like a really good idea. Can I have a go at it or do we have to close the ticket? I can see there are a few things I have to learn on the way.

It's such a trivial feature, but if it is included I think it should allow a web resource to contain the icon as the premier option. I'm thinking that the target group is people coming from web development rather than crypto and then it is a nice perk. On the other hand, a very minor feature mustn't be allowed to clutter down the code.

But two options would be more generous than one I think.

I tried moving the evaluation of the web resource into the block for 'console/webapps' it works just as well there, and then it would be ignored even if present in a config file for a webless plugin.

If I'm not totally wasting your time I'd like to explore both options further. No need to push it.

A more realistic version of above. Stipulates that the encoded file string starts with 3-character identification of file format. Supported types are those recognized by viewtheme.jsp i.e. png, ico, gif and jpg. This is done automatically in the shell script.

And this is the other way of doing it that I can think of. The servlet is unnecessarily chatty because I just lifted it from a NetbeansTemplate?, it would be nicer just extending GenericServlet?. Most of the code in PluginStarter? is now commented out. To be more consistent with the codingstyle a copy of flags.jsp would do the same thing. This is a lot less sneaky than the other method, but also more invasive, adding a new class.

Perhaps something like this? We would still have to name the filetype in the header right? Or is that unnecessary? My browser doesn't seem to care what kind of image binary it's pushing regardless of filetype. flags.jsp is particular about it though, so I did this instead: store the undecoded data in a clob in NavHelper? with filetype prepended to the encoded data. This still removes any concerns about URL-size. Presumably the maximum length of a String is 231 .

By filetype you mean like a Content-Type I assume. Easiest to just require (and assume) png rather than get fancy.

And as I suggested in comment 15, storing in binary as a byte[] in the NavHelper? hashmap is more space-efficient and time-efficient, as it's only decoded from base 64 once. Which you presumably can't do if you are prepending it with a Content-Type.

Yes strings can be long, but I believe our code that reads plugin.config uses DataHelper?.readLine() which has an 8 KB limit, just to prevent trouble.

Reopening as tickets shouldn't be fixed and closed until the code is in trunk.

In your servlet, you will NPE if the binary isn't found. You should either return 404 or the default image, whatever makes more sense or is easier.

And just to throw an alternative in at this late date: inline image data ​https://en.wikipedia.org/wiki/Data_URI_scheme … we'd have to convert from our base64 to standard base64. But then we wouldn't need a servlet at all. Worth thinking about. Not necessarily better.

This version handles two error-conditions. First if NavHelper?.getBinary() returns a nullpointer, second if the property is present in plugin.config but without attached content. Both cases will redirect to docs/themes/console/images/plugin.png. Caches the icon for a day. Exception handling lifted straight from flags.jsp.

3) At PluginStarter? line ~358, you don't need to strip HTML from the icon code (it's not being displayed as-is), just do fullprop = props.getProperty("icon-code")

4) Dies NPE if no "plugin" parameter is supplied (name is null)

Error 500: /Plugins/pluginicon Server Error
java.lang.NullPointerException
at java.util.concurrent.ConcurrentHashMap.hash(ConcurrentHashMap.java:333)
at java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:988)
at net.i2p.router.web.NavHelper.getBinary(NavHelper.java:48)
at net.i2p.router.web.CodedIconRendererServlet.service(CodedIconRendererServlet.java:36)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:848)
at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:684)
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1496)
at net.i2p.servlet.filters.XSSFilter.doFilter(XSSFilter.java:28)
...