Inconsistent behaviour in path and fullPath

[dodgex]

As mentioned in #13 (comment), i found that fullPath and path have slightly inconsitent behaviour for cases where exactPath starts with the version.

In such cases, path could return a path with the version contained twice. Once from the String format arguments, and once from it beeing contained in exactPath. fullPath on the other hand check for the version beeing part (startsWith) of the exactPath and in that case not including the version again in the String format call. Although fullPath has a TODO comment questioning the need for that check.

For me using the WebJars locator only indirectly with Spring, only the fullPath seems to be used. And as I do not have deeper knowledge about the API I can't tell if this is by any means can be a problem for other use cases.

Regarding the question in the TODO comment, i would suggest keeping the check (and potentially add it to path). The reason for this is, that I saw cases, where the full path, including the version, has been used in the a thymeleaf template. With this check the locator gracefully handles this kind of "invalid" usage instead of having the version twice. On the other hand, this could cause some false security on the developer side... as it then fails when updating the webjar and not already at the time of adding...

Okay, as of writing and thinking a bit more about this. Maybe the check should be removed, to fail early when adding the webjar instead of only failing after updating it?

What are your thoughts on this?

[jamesward]

Thanks for opening this new issue to discuss this. There probably isn't a good way to do this but I wish we could prohibit the use of a version in the fullPath and path methods as the primary point of using the locator is to not hard-code versions. The only way I can think of to do that is with Regex but version syntax varies wildly so at most we could provide a more helpful error message. Are there other cases I'm not thinking of where using a version there makes sense?

[dodgex]

Well, as you said, there is no (good) reason for the user to provide a version in the incoming parameters, as determining the version is exactly what the WebJars locator is for. We had some trainee wo used webjars but added the path including the version, but in my opinion it is ok, to "fail" in a case like this.

At least in Spring based projects
<link rel="stylesheet" data-th-href="@{/webjars/font-awesome/css/font-awesome.min.css}"/>
becomes <link rel="stylesheet" href="/webjars/font-awesome/4.7.0/css/font-awesome.min.css"/>.

When the template contains the version like <link rel="stylesheet" data-th-href="@{/webjars/font-awesome/4.7.0/css/font-awesome.min.css}"/>.
Without the check it would become <link rel="stylesheet" href="/webjars/font-awesome/4.7.0/4.7.0/css/font-awesome.min.css"/> with the version twice.

This would cause a 404 during development allowing to fix it early. when the locator ignores the 4.7.0 the output would become <link rel="stylesheet" href="/webjars/font-awesome/4.8.0/4.7.0/css/font-awesome.min.css"/> for the next update of the webjar, as the path in the template would no longer start with the version.

In my opinion, the check should be removed to avoid this kind delayed fails.

Assuming the check gets removed, I also would avoid adding any other check for versions in the path. While it might me a strange edge case, but what if the webjar structure is actually somewhat like /mywebjar/1.2.3/3.2.1/*files* where the webjar version 1.2.3, MIGHT contain multiple sub-versions 3.2.1 or anything that might look like a version e.g 4.x to show compat to another lib. This would most likely only apply to custom WebJars but could break with a check for a version. As said, this is an edge case, but on the other hand, I see no benefit in having a check to prohibit using versions. If a version is used, the resulting path will result in 404, that should be prohibiting enough?

[jamesward]

That makes sense. I'm all for enabling better and earlier error messages.