dotnetcore / BootstrapBlazor

Bootstrap Blazor is an enterprise-level UI component library based on Bootstrap and Blazor.
https://www.blazor.zone
Apache License 2.0
2.62k stars 299 forks source link

bug(JsonStringLocalizer): Endless loop #4652

Open izanhzh opened 1 day ago

izanhzh commented 1 day ago

Is there an existing issue for this?

Describe the bug

GetStringSafelyGetAllStrings这两个方法中,存在个死循环的可能: image

Expected Behavior

在上述第2步中,增加判断排除自身避免死循环

Interactive render mode

Interactive Server (Interactive server-side rendering (interactive SSR) using Blazor Server)

Steps To Reproduce

xxx

Exceptions (if any)

image

.NET Version

dotnet all version

Anything else?

No response

bb-auto[bot] commented 1 day ago

@izanhzh Thank you for filing this issue. In order for us to investigate this issue, please provide a minimalistic repro project that illustrates the problem.

ArgoZhang commented 1 hour ago

@izanhzh 死循环的根本原因我个人认为是代码导致的

public IStringLocalizer Create(Type resourceSource)
{
    var stringLocalizerFactorys = _serviceProvider.GetServices<IStringLocalizerFactory>();
    IStringLocalizerFactory stringLocalizerFactory;
    if (resourceSource == typeof(Foo))
    {
        stringLocalizerFactory = stringLocalizerFactorys.Single(s => s.GetType().Name == "JsonStringLocalizerFactory");
    }
    else
    {
        stringLocalizerFactory = _serviceProvider.GetServices<IStringLocalizerFactory>().Single(s => s is MockLocalizerFactory);
    }
    return stringLocalizerFactory.Create(resourceSource);
}

这里的逻辑有很大问题

BootstrapBlazor 组件内部已经找到合适并且正确的 IStringLocalizerFactory 服务,这块代码里不应该去取一个错误的服务

izanhzh commented 1 hour ago

@ArgoZhang 我认为JsonStringLocalizer应该做好防御,不能让其他人能够将从外部返回一个JsonStringLocalizer实例给到GetStringSafelyGetAllStrings这两个方法中,别人集成项目中,可能会使用自己定义的IStringLocalizerFactory,然后基于一些情况让其返回BB默认的JsonStringLocalizer,其他情况返回其他的xxxStringLocalizer,上面那个代码只是一种方式,比如我其他的xxxStringLocalizer不通过IOC容器注入,容器中只有BB的JsonStringLocalizer,然后我代码写成某些情况从容器中获取,某些情况通过一些其他手段构造StringLocalizer 当然这也不算严格意义上的BUG,应该调整为是feat

ArgoZhang commented 55 minutes ago

这里没办法做防护的,我认为应该做个检查,发现死循环了报错比较合理。因为你不知道外面代码如何写,那我们就没有办法做防护。

也不行。死循环都无法检查。你的这个单元测试本身就有问题。并不能代表所有问题

你这个代码是服务里再拿一个不正确的服务,所以导致了死循环

ArgoZhang commented 52 minutes ago

@izanhzh 应该找到一个合理的解决方案。不能被你的单元测试误导了

izanhzh commented 50 minutes ago

不是很明白,Utility.GetStringLocalizerFromService 这里不应该杜绝掉JsonStringLocalizer返回么,我认为还是很有必要的,这里防护一下,JsonStringLocalizer就不会陷入循环了,如果Utility.GetStringLocalizerFromService 返回的是JsonStringLocalizer就陷入循环了

izanhzh commented 45 minutes ago

Utility.GetStringLocalizerFromService 过滤掉返回JsonStringLocalizer,接下里就已经是JsonStringLocalizer在处理了,这是正确的预期效果,Utility.GetStringLocalizerFromService 本就不应该有返回JsonStringLocalizer的可能

ArgoZhang commented 37 minutes ago

@izanhzh 这个对。所以我刚才说应该抛异常,我改一下。你看看是否更合理

izanhzh commented 34 minutes ago

@ArgoZhang 我还是觉得没必要抛异常,既然外部返回的就是JsonStringLocalizer,那咱就用JsonStringLocalizer给它处理不就行了,没必要报错呀,这不就符合外部想要的效果了么,这个更灵活呀 😂

izanhzh commented 28 minutes ago

89X`UO@3SXH5}AL6UZCRD~T

ArgoZhang commented 25 minutes ago

@izanhzh 行吧。你这例子也是奇葩写法。这么折腾一圈还搞个死循环,我内部还要判断一下,为啥啊?为啥非要拿我内部的服务搞啊。所以我想抛异常

izanhzh commented 23 minutes ago

还记得前几天有个abp小伙说他用了数据库维护翻译么,又不肯用resolve再跳一次,我闲着就研究了下其他办法了😂

ArgoZhang commented 19 minutes ago

这不是解决问题的办法,所以我要复现工程,我要知道他怎么用。我才能更改设计,让他用的更合理。而不是这样瞎搞,这代码毫无意义,能否给我一个 abp 的例子。我看看到底为啥不行?再改代码,让外面用起来更合理,而不是我内部调用外面,你外面再回去调用我内部。人为的死循环

izanhzh commented 14 minutes ago

我也不知道他怎么弄用数据库维护翻译,不太好构造这个示例,抛开这些而论,我还是坚持认为应该过滤掉外部返回自身实例这个情况

izanhzh commented 7 minutes ago

我这样说看看你能否好接受一点,BB现在用的是Json维护翻译,而我的系统是用数据库维护翻译,这两个无法通用,而我又不想用resolve方案,因为resolve方案会先经过你内部判断才到我自己系统维护的翻译,而我又无法获取到你的Factory类,所以我只能通过字符串的方式来找到你的Factory,这样我就能基于不同的资源命名空间,直接找翻译,BB的就用BB的,我系统的就用我系统的

ArgoZhang commented 5 minutes ago

没问题的。这个保护加一下倒是没问题。我的意思是。这样搞并没有解决 abp 他们说的不能多语言的问题。只是凑巧结果可能对了

ArgoZhang commented 3 minutes ago

数据库的不是问题。核心问题是你要按照我的设计注入你的服务。你不能服务。又去搞我内部服务。这本身就不合理。不优雅。

izanhzh commented 2 minutes ago

所以他想让你将BB的 JsonStringLocalizerJsonStringLocalizerFactory 做成公开的类,提供可重写的虚方法,这样就可以结合自身实现多种方式维护翻译,而不是限定于json,更灵活一点,但是BB基于什么原因不公开,我也不太清楚,我只是想到一种可行的方案