Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Null reference exception in the Admin theme #15752

Closed
ItaloFSS opened this issue Apr 13, 2024 · 12 comments · Fixed by #15765
Closed

Null reference exception in the Admin theme #15752

ItaloFSS opened this issue Apr 13, 2024 · 12 comments · Fixed by #15765
Labels

Comments

@ItaloFSS
Copy link

ItaloFSS commented Apr 13, 2024

While upgrading our website from 1.6 to 1.82, we ran into an issue with the Admin theme. Line 12 in NavigationItemLink-admin.cshtml is not checking for null, which prevents the theme from rendering.

Original:

if (Model.Href.ToString() == "#")

Changed to:

if ( ( Model.Href?.ToString() ?? "#" ) == "#")

Thanks

@Piedone
Copy link
Member

Piedone commented Apr 14, 2024

Shouldn't we rather throw a custom exception (not an ad-hoc NRE) if Href is null or empty? Since does it make sense for it to ever not have a string?

@ItaloFSS
Copy link
Author

I'm not sure why, but the menu item with the issue is "Azure Blob Options." Throwing an exception will prevent the theme from rendering, making the admin inaccessible.

@Piedone
Copy link
Member

Piedone commented Apr 15, 2024

I can't reproduce this with the latest main or with v1.8.2. Are you sure it's not some custom code of your that causes this?

@ItaloFSS
Copy link
Author

There's no custom code for that module. That's the only one with a null HREF.

image

@Piedone
Copy link
Member

Piedone commented Apr 15, 2024

Don't you have any other custom code in the solution that could affect this though? Because that link specifies a Href (this is the same in 1.8.2), and this properly works in vanilla OC both on 1.8.2 and latest main for me (the URL is there on the UI, visible Href, and no exception is thrown).

What I can imagine is that perhaps you have a custom INavigationProvider that affects this?

@MikeAlhayek
Copy link
Member

@ItaloFSS

Can you please expand these two nodes? I am looking for more clues on what created this menu item.

image

@ItaloFSS
Copy link
Author

The issue was that the Admin controller was not registering, so no action URL was being generated. I was able to fix it by disabling and reenabling the module.

You should still check for null. The entire admin module was inaccessible. At least, it should render an error for the user.

Thanks for the help!

@MikeAlhayek
Copy link
Member

@ItaloFSS

The issue was that the Admin controller was not registering,

Maybe there is a dependency issue. Which admin controller was not available? or which feature you enabled to fix the issue?

@ItaloFSS
Copy link
Author

The Admin controller in OrchardCore.Media.Azure.

image

@MikeAlhayek
Copy link
Member

We are missing something here. I am wondering if you were registering the AdminMenu class in your project somehow

public class AdminMenu : INavigationProvider

This class what added the menu item which appears to be that caused your issue. That class would not be registered unless the OrchardCore.Media.Azure.Storage is activated. And when it is activated, the controller routes will then be available.

Based on your explanation, it seems that that AdminMenu was registered while the feature was not which does not make sense "at least from what I can see in OC code".

@ItaloFSS
Copy link
Author

We don't have an INavigationProvider that's adding the menu item. I looked everywhere for it.

Could it be security? When I disabled and reenabled the module, the claims were added to my identity.

@MikeAlhayek
Copy link
Member

I am not sure. Try disabling the feature that fixed your issue and see if you can do some tracing to see what is actually adding that menu item. Maybe look at the logs to see if there are any additional clues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants