CurrentNode is matched by partial URL match

[anton-kanashkin]

cloudscribe.Web.Navigation v2.1.16

We have a controller named, say, OverviewController, with Index action, and another controller with action Overview. navigation.xml might look like this:

<NavNode key="Home" text="Home" controller="Home" action="Index">
    <Children>
        <NavNode key="OverviewWhatever" text="Overview from WhateverController" controller="Whatever" action="Overview" />
        <NavNode key="OverviewIndex" text="Index from OverviewController" controller="Overview" action="Index" />
    </Children>
</NavNode>

The order here is important.

Now if we would go to /Overview, NavigationViewModel.CurrentNode will return the node with key OverviewWhatever - not OverviewIndex as expected.

This happens because TreeNodeExtensions.FindByUrl() matches URL by partial match:
https://github.com/cloudscribe/cloudscribe.Web.Navigation/blob/master/src/cloudscribe.Web.Navigation/TreeNodeExtensions.cs#L75

Since /Overview is a substring of /Whatever/Overview and WhateverController's action precedes OverviewController's action, wrong action will be matched.

I can make a demo later if it would help you to reproduce the issue.

[joeaudette]

Thanks for the bug report.

This should now be fixed in the latest nuget https://www.nuget.org/packages/cloudscribe.Web.Navigation/2.1.17

[nikolay-isakov]

Hello.
We have found as issue which caused as a result of the fix of issue #71 in cloudscribe.Web.Navigation v2.1.17.

For example, we have an application that uses URLs in the following format to access pages:
/cloudscribe/test/<controller>/<action>

When we go, for example, to /cloudscribe/test/Overview, the CurrentNode has null value, because TreeNodeExtensions.FindByUrl() method does the following
"/cloudscribe/test/Overview".StartsWith("Overview", StringComparison.OrdinalIgnoreCase)
instead of
"/cloudscribe/test/Overview".IndexOf("Overview", StringComparison.OrdinalIgnoreCase) >= 0

[joeaudette]

You have not explained how you are configuring your nodes and you have not explained the extra url segments "/cloudscribe/test", is that because the app is deployed in a virtual directory?

[joeaudette]

Note that we know the default logic for finding the current node doesn't work in every situation, that is why we have the IFindCurrentNode so you can implement and inject your own logic to find it if it is not found by the default logic.

[joeaudette]

If I change it back to use indexof rather than startwith, then I cannot solve the original issue.

However, for your situation if hosted in a virtual directory with extra segments at the beginning of the url I've published an update to the nuget that may work if you implement and inject INodeUrlPrefixProvider. Your implementation could be like this:

public class CustomUrlPrefixProvider : INodeUrlPrefixProvider
{
    public CustomUrlPrefixProvider()
    { }

    public string GetPrefix()
    {
        return "/cloudscribe/test";
    }
}

the prefix will be prepended to the resolved target url and then it will try StartsWith again.

[nikolay-isakov]

Thank you for your answer and for your suggestion. Our application is deployed as a virtual application, not a virtual directory.
Sorry that I didn't designate the problem at once, you can easily reproduce it with the following configuration in "launchSettings.json" in the NavigationDemo.Web project:

"iisSettings": {
    "windowsAuthentication": false,
    "anonymousAuthentication": true,
    "iisExpress": {
        "applicationUrl": "http://localhost:51443/test/cloudscribe",
        "sslPort": 0
    }
}

In such case NavigationViewModel is always null.
I have implement CustomUrlPrefixProvider but it does not work because of this peace of code:

if (!string.IsNullOrWhiteSpace(urlPrefix))
{
    targetUrl = urlPrefix + targetUrl;
    if ((!string.IsNullOrEmpty(targetUrl)) && (targetUrl.StartsWith(urlToMatch, StringComparison.OrdinalIgnoreCase))) { return true; }
}

The reason is that urlHelper.Action returns URL which contains all of the parts, for example: /test/cloudscribe/area51/roswell/
And then as a result of
targetUrl = urlPrefix + targetUrl;
targetUrl has value /test/cloudscribe/test/cloudscribe/area51/roswell/

All of the above is reproduced in the NavigationDemo.Web project. If it helps I can make a simple demo based on the NavigationDemo.Web.

[joeaudette]

@nikolay-isakov ok, following your instruction with launch settings I was able to reproduce the problem and fix it so it can work if you implement the CustomUrlPrefixProvider. I implemented it in the demo like this:

public class CustomUrlPrefixProvider : INodeUrlPrefixProvider
{
    public CustomUrlPrefixProvider()
    { }

    public string GetPrefix()
    {
        return "/test/cloudscribe";
    }
}

and injected it and got it working. The bug in my logic was that the urlprefix needs to be prepended to the url to match not the target url generated by urlhelper.

I've shipped an updated nuget: https://www.nuget.org/packages/cloudscribe.Web.Navigation/2.1.19

Note that I have changed the launchsettings back to how it was without those segments in the demo and I've commented out the line in Startup.cs:

services.AddScoped<cloudscribe.Web.Navigation.INodeUrlPrefixProvider, CustomUrlPrefixProvider>();

but you can test by uncomment that and put the launchsettings back how you suggested.

You could make a better implementation of the CustomUrlPrefixProvider using an appsetting to set the prefix string instead of hard coding it like I did in the demo. That way you can change it you deploy to a different url.

[nikolay-isakov]

I have updated the package and implemented the CustomUrlPrefixProvider like this:

public class CustomUrlPrefixProvider : INodeUrlPrefixProvider
{
    private readonly IHttpContextAccessor _httpContentAccessor;

    public CustomUrlPrefixProvider(IHttpContextAccessor httpContentAccessor)
    {
        _httpContentAccessor = httpContentAccessor;
    }

    public string GetPrefix()
    {
        return _httpContentAccessor.HttpContext.Request.PathBase;
    }
}

I have tested it in the demo for application with empty path and with /test/cloudscribe path. It works fine, maybe it can be used as DefaultUrlPrefixProvider which returns empty string now, what do you think about it?

[joeaudette]

@nikolay-isakov yes, that seems a good solution, I should have thought of that. I've made the change you suggest to DefaultUrlPrefixProvider and published another nuget https://www.nuget.org/packages/cloudscribe.Web.Navigation/2.1.20

[mgbbs]

I'm still seeing this issue occurring in v2.1.20.

I simplified my navigation.xml file to eliminate as many variables as possible:

<NavNode key="Home" controller="Home" action="Index" area="" preservedRouteParameters="" text="Home" iconCssClass="">
  <Children>
    <NavNode key="Service" controller="About" action="Service" area="" text="Service" iconCssClass=""></NavNode>
    <NavNode key="Company" controller="About" action="Index" area="" text="Company" iconCssClass=""></NavNode>
  </Children>
</NavNode>

Notice the watch variables in the screenshot below. Request path is /About, which is the "Company" route. The resolved CurrentNode is /About/Service.

[joeaudette]

Ok, I've figured out that our logic using .IndexOf or .StartsWith is flawed because it favors the first match which may not be the best match. I've updated it now so that it will try an exact match first and only if not found with exact match will it fallback to the .IndexOf and .StartsWith logic to find a match.

I was able to replicate this problem in the demo app and fix it with the above change. I've published an updated nuget https://www.nuget.org/packages/cloudscribe.Web.Navigation/2.1.21