jenkinsci / azure-ad-plugin

Authentication and Authorization with Azure AD
https://plugins.jenkins.io/azure-ad/
MIT License
29 stars 58 forks source link

Invalid usernames crash People API #167

Closed meiswjn closed 3 years ago

meiswjn commented 3 years ago

Version report

Jenkins and plugins versions report:

Jenkins: 2.289.3
OS: Windows Server 2012 R2 - 6.3
---
Office-365-Connector:4.15.0
ace-editor:1.1
analysis-model-api:10.3.0
ansicolor:1.0.0
ant:1.11
antisamy-markup-formatter:2.1
apache-httpcomponents-client-4-api:4.5.13-1.0
artifactory:3.13.1
authentication-tokens:1.4
azure-ad:180.v8b1e80e6f242
azure-commons:1.1.3
azure-credentials:182.v3ccd4a755864
azure-sdk:30.vf3165534d6e8
blueocean:1.24.8
blueocean-autofavorite:1.2.4
blueocean-bitbucket-pipeline:1.24.8
blueocean-commons:1.24.8
blueocean-config:1.24.8
blueocean-core-js:1.24.8
blueocean-dashboard:1.24.8
blueocean-display-url:2.4.1
blueocean-events:1.24.8
blueocean-git-pipeline:1.24.8
blueocean-github-pipeline:1.24.8
blueocean-i18n:1.24.8
blueocean-jwt:1.24.8
blueocean-personalization:1.24.8
blueocean-pipeline-api-impl:1.24.8
blueocean-pipeline-editor:1.24.8
blueocean-pipeline-scm-api:1.24.8
blueocean-rest:1.24.8
blueocean-rest-impl:1.24.8
blueocean-web:1.24.8
bootstrap4-api:4.6.0-3
bootstrap5-api:5.1.0-3
bouncycastle-api:2.24
branch-api:2.6.5
build-blocker-plugin:1.7.7
build-discarder:41.v1e18f33d64a6
build-monitor-plugin:1.12+build.201809061734
build-user-vars-plugin:1.7
build-with-parameters:1.5.1
caffeine-api:2.9.2-29.v717aac953ff3
checks-api:1.7.2
cloudbees-bitbucket-branch-source:2.9.11
cloudbees-disk-usage-simple:0.10
cloudbees-folder:6.16
cobertura:1.16
code-coverage-api:1.4.1
command-launcher:1.6
conditional-buildstep:1.4.1
config-file-provider:3.8.1
confluence-publisher:2.0.6
copyartifact:1.46.1
credentials:2.5
credentials-binding:1.27
dashboard-view:2.17
data-tables-api:1.10.25-3
dependency-check-jenkins-plugin:5.1.1
display-url-api:2.3.5
docker-commons:1.17
docker-java-api:3.1.5.2
docker-plugin:1.2.3
docker-workflow:1.26
dtkit-api:3.0.0
durable-task:1.39
echarts-api:5.1.2-11
email-ext:2.83
envinject:2.4.0
envinject-api:1.7
extended-choice-parameter:0.82
extended-read-permission:3.2
external-monitor-job:1.7
extra-columns:1.24
favorite:2.3.3
file-operations:1.11
font-awesome-api:5.15.4-1
forensics-api:1.3.0
git:4.8.2
git-client:3.9.0
git-server:1.10
github:1.34.1
github-api:1.123
github-branch-source:2.11.2
gitlab-plugin:1.5.20
gradle:1.37.1
groovy:2.4
handlebars:3.0.8
handy-uri-templates-2-api:2.1.8-1.0
htmlpublisher:1.25
http_request:1.11
integrity-plugin:2.4
ivy:2.1
jackson2-api:2.12.4
jacoco:3.3.0
javadoc:1.6
jaxb:2.3.0.1
jdk-tool:1.5
jenkins-design-language:1.24.8
jira:3.5
jjwt-api:0.11.2-9.c8b45b8bb173
jobConfigHistory:2.28.1
jquery:1.12.4-1
jquery3-api:3.6.0-2
jsch:0.1.55.2
junit:1.52
ldap:2.7
list-git-branches-parameter:0.0.9
locale:1.4
lockable-resources:2.11
mailer:1.34
matlab:2.6.0
matrix-auth:2.6.8
matrix-project:1.19
maven-plugin:3.12
metrics:4.0.2.8
momentjs:1.1.1
monitoring:1.88.0
msbuild:1.30
next-executions:1.0.15
nodejs:1.4.0
nuget:1.1
okhttp-api:3.14.9
pam-auth:1.6
parameterized-scheduler:1.0
parameterized-trigger:2.41
pipeline-build-step:2.15
pipeline-graph-analysis:1.11
pipeline-input-step:2.12
pipeline-milestone-step:1.3.2
pipeline-model-api:1.9.1
pipeline-model-definition:1.9.1
pipeline-model-extensions:1.9.1
pipeline-rest-api:2.19
pipeline-stage-step:2.5
pipeline-stage-tags-metadata:1.9.1
pipeline-stage-view:2.19
pipeline-utility-steps:2.10.0
plain-credentials:1.7
plugin-usage-plugin:1.2
plugin-util-api:2.4.0
popper-api:1.16.1-2
popper2-api:2.9.3-1
powershell:1.5
pubsub-light:1.16
resource-disposer:0.16
robot:3.0.1
run-condition:1.5
saml:2.0.8
scm-api:2.6.5
script-security:1.78
snakeyaml-api:1.29.1
sonar:2.13.1
sse-gateway:1.24
ssh-agent:1.23
ssh-credentials:1.19
ssh-slaves:1.33.0
ssh-steps:2.0.0
sshd:3.1.0
structs:1.23
thinBackup:1.10
timestamper:1.13
token-macro:266.v44a80cf277fd
trilead-api:1.0.13
uno-choice:2.5.6
variant:1.4
warnings-ng:9.5.0
windows-slaves:1.8
workflow-aggregator:2.6
workflow-api:2.46
workflow-basic-steps:2.24
workflow-cps:2.93
workflow-cps-global-lib:2.21
workflow-durable-task-step:2.39
workflow-job:2.41
workflow-multibranch:2.26
workflow-scm-step:2.13
workflow-step-api:2.24
workflow-support:3.8
ws-cleanup:0.39
xunit:3.0.2

Reproduction steps

Expected result:

Get a list of all people. Invalid users could be skipped.

Actual result: Internal Server Error. image

Logs show the following stacktrace:

Sep 30, 2021 12:00:54 PM WARNING org.eclipse.jetty.server.handler.ContextHandler$Context log
Error while serving https://jenkins.example.com/asynchPeople/api/json
com.microsoft.graph.http.GraphServiceException: Error code: BadRequest
Error message: The request URI is not valid. Since the segment 'users' refers to a collection, this must be the last segment in the request URI or it must be followed by an function or action that can be bound to it otherwise all intermediate segments must refer to a single resource.

GET https://graph.microsoft.com/v1.0/users/$(([adsisearcher](samaccountname=$env:username)).FindOne().Properties.userprincipalname)
SdkVersion : graph-java/v3.8.0

400 : Bad Request
[...]

[Some information was truncated for brevity, enable debug logging for more details]
    at com.microsoft.graph.http.GraphServiceException.createFromResponse(GraphServiceException.java:419)
    at com.microsoft.graph.http.GraphServiceException.createFromResponse(GraphServiceException.java:378)
    at com.microsoft.graph.http.CoreHttpProvider.handleErrorResponse(CoreHttpProvider.java:501)
    at com.microsoft.graph.http.CoreHttpProvider.processResponse(CoreHttpProvider.java:430)
    at com.microsoft.graph.http.CoreHttpProvider.sendRequestInternal(CoreHttpProvider.java:396)
    at com.microsoft.graph.http.CoreHttpProvider.send(CoreHttpProvider.java:222)
    at com.microsoft.graph.http.CoreHttpProvider.send(CoreHttpProvider.java:199)
    at com.microsoft.graph.http.BaseRequest.send(BaseRequest.java:332)
    at com.microsoft.graph.requests.UserRequest.get(UserRequest.java:138)
    at com.microsoft.jenkins.azuread.AzureSecurityRealm.lambda$null$5(AzureSecurityRealm.java:504)
    at com.github.benmanes.caffeine.cache.BoundedLocalCache.lambda$doComputeIfAbsent$14(BoundedLocalCache.java:2413)
    at java.util.concurrent.ConcurrentHashMap.compute(Unknown Source)
    at com.github.benmanes.caffeine.cache.BoundedLocalCache.doComputeIfAbsent(BoundedLocalCache.java:2411)
    at com.github.benmanes.caffeine.cache.BoundedLocalCache.computeIfAbsent(BoundedLocalCache.java:2394)
    at com.github.benmanes.caffeine.cache.LocalCache.computeIfAbsent(LocalCache.java:108)
    at com.github.benmanes.caffeine.cache.LocalManualCache.get(LocalManualCache.java:62)
    at com.microsoft.jenkins.azuread.AzureSecurityRealm.lambda$createSecurityComponents$6(AzureSecurityRealm.java:492)
    at hudson.security.SecurityRealm.loadUserByUsername2(SecurityRealm.java:413)
    at jenkins.security.UserDetailsCache$Retriever.call(UserDetailsCache.java:165)
    at jenkins.security.UserDetailsCache$Retriever.call(UserDetailsCache.java:154)
    at com.google.common.cache.LocalCache$LocalManualCache$1.load(LocalCache.java:4767)
    at com.google.common.cache.LocalCache$LoadingValueReference.loadFuture(LocalCache.java:3568)
    at com.google.common.cache.LocalCache$Segment.loadSync(LocalCache.java:2350)
    at com.google.common.cache.LocalCache$Segment.lockedGetOrLoad(LocalCache.java:2313)
    at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2228)
Caused: com.google.common.util.concurrent.UncheckedExecutionException
    at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2234)
    at com.google.common.cache.LocalCache.get(LocalCache.java:3965)
    at com.google.common.cache.LocalCache$LocalManualCache.get(LocalCache.java:4764)
    at jenkins.security.UserDetailsCache.loadUserByUsername(UserDetailsCache.java:122)
    at hudson.model.User$UserIDCanonicalIdResolver.resolveCanonicalId(User.java:1246)
    at hudson.model.User$CanonicalIdResolver.resolve(User.java:1187)
    at hudson.model.User.get(User.java:520)
    at hudson.plugins.git.GitChangeSet.findOrCreateUser(GitChangeSet.java:417)
    at hudson.plugins.git.GitChangeSet.getAuthor(GitChangeSet.java:541)
    at hudson.model.View$People.getUserInfo(View.java:756)
    at hudson.model.View$People.<init>(View.java:730)
    at hudson.model.View$AsynchPeople$People.getUsers(View.java:945)
    at org.kohsuke.stapler.export.MethodProperty.getValue(MethodProperty.java:72)
Caused: java.lang.reflect.InvocationTargetException
    at org.kohsuke.stapler.export.MethodProperty.getValue(MethodProperty.java:74)
    at org.kohsuke.stapler.export.ExportInterceptor$1.getValue(ExportInterceptor.java:46)
Caused: java.io.IOException: Failed to write users:null
    at org.kohsuke.stapler.export.ExportInterceptor$1.getValue(ExportInterceptor.java:52)
    at org.kohsuke.stapler.export.Property.writeTo(Property.java:135)
    at org.kohsuke.stapler.export.Model.writeNestedObjectTo(Model.java:223)
    at org.kohsuke.stapler.export.Model.writeTo(Model.java:194)
    at org.kohsuke.stapler.ResponseImpl.writeOne(ResponseImpl.java:294)
    at org.kohsuke.stapler.ResponseImpl.serveExposedBean(ResponseImpl.java:285)
    at org.kohsuke.stapler.ResponseImpl.serveExposedBean(ResponseImpl.java:241)
    at hudson.model.Api.doJson(Api.java:223)
    at java.lang.invoke.MethodHandle.invokeWithArguments(Unknown Source)
    at org.kohsuke.stapler.Function$MethodFunction.invoke(Function.java:396)
Caused: java.lang.reflect.InvocationTargetException
    at org.kohsuke.stapler.Function$MethodFunction.invoke(Function.java:400)
    at org.kohsuke.stapler.Function$InstanceFunction.invoke(Function.java:408)
    at org.kohsuke.stapler.Function.bindAndInvoke(Function.java:212)
    at org.kohsuke.stapler.Function.bindAndInvokeAndServeResponse(Function.java:145)
    at org.kohsuke.stapler.MetaClass$11.doDispatch(MetaClass.java:536)
    at org.kohsuke.stapler.NameBasedDispatcher.dispatch(NameBasedDispatcher.java:58)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:766)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:898)
    at org.kohsuke.stapler.MetaClass$2.doDispatch(MetaClass.java:220)
    at org.kohsuke.stapler.NameBasedDispatcher.dispatch(NameBasedDispatcher.java:58)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:766)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:898)
    at org.kohsuke.stapler.MetaClass$2.doDispatch(MetaClass.java:220)
    at org.kohsuke.stapler.NameBasedDispatcher.dispatch(NameBasedDispatcher.java:58)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:766)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:898)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:694)
    at org.kohsuke.stapler.Stapler.service(Stapler.java:240)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
    at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:799)
    at org.eclipse.jetty.servlet.ServletHandler$ChainEnd.doFilter(ServletHandler.java:1626)
    at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:154)
    at org.jenkinsci.plugins.ssegateway.Endpoint$SSEListenChannelFilter.doFilter(Endpoint.java:248)
    at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:151)
    at jenkins.security.ResourceDomainFilter.doFilter(ResourceDomainFilter.java:76)
    at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:151)
    at jenkins.telemetry.impl.UserLanguages$AcceptLanguageFilter.doFilter(UserLanguages.java:129)
    at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:151)
    at io.jenkins.blueocean.ResourceCacheControl.doFilter(ResourceCacheControl.java:134)
    at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:151)
    at io.jenkins.blueocean.auth.jwt.impl.JwtAuthenticationFilter.doFilter(JwtAuthenticationFilter.java:60)
    at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:151)
    at com.smartcodeltd.jenkinsci.plugin.assetbundler.filters.LessCSS.doFilter(LessCSS.java:47)
    at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:151)
    at net.bull.javamelody.MonitoringFilter.doFilter(MonitoringFilter.java:239)
    at net.bull.javamelody.MonitoringFilter.doFilter(MonitoringFilter.java:215)
    at net.bull.javamelody.PluginMonitoringFilter.doFilter(PluginMonitoringFilter.java:88)
    at org.jvnet.hudson.plugins.monitoring.HudsonMonitoringFilter.doFilter(HudsonMonitoringFilter.java:121)
    at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:151)
    at jenkins.metrics.impl.MetricsFilter.doFilter(MetricsFilter.java:125)
    at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:151)
    at hudson.plugins.locale.LocaleFilter.doFilter(LocaleFilter.java:42)
    at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:151)
    at hudson.util.PluginServletFilter.doFilter(PluginServletFilter.java:157)
    at org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:193)
    at org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1601)
    at hudson.security.csrf.CrumbFilter.doFilter(CrumbFilter.java:159)
    at org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:193)
    at org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1601)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:92)
    at jenkins.security.AcegiSecurityExceptionFilter.doFilter(AcegiSecurityExceptionFilter.java:52)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:97)
    at hudson.security.UnwrapSecurityExceptionFilter.doFilter(UnwrapSecurityExceptionFilter.java:51)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:97)
    at org.springframework.security.web.access.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:119)
    at org.springframework.security.web.access.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:113)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:97)
    at org.springframework.security.web.authentication.AnonymousAuthenticationFilter.doFilter(AnonymousAuthenticationFilter.java:105)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:97)
    at org.springframework.security.web.authentication.rememberme.RememberMeAuthenticationFilter.doFilter(RememberMeAuthenticationFilter.java:101)
    at org.springframework.security.web.authentication.rememberme.RememberMeAuthenticationFilter.doFilter(RememberMeAuthenticationFilter.java:92)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:97)
    at org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter.doFilter(AbstractAuthenticationProcessingFilter.java:218)
    at org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter.doFilter(AbstractAuthenticationProcessingFilter.java:212)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:97)
    at jenkins.security.BasicHeaderProcessor.doFilter(BasicHeaderProcessor.java:93)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:97)
    at org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:110)
    at org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:80)
    at hudson.security.HttpSessionContextIntegrationFilter2.doFilter(HttpSessionContextIntegrationFilter2.java:62)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:97)
    at hudson.security.ChainedServletFilter.doFilter(ChainedServletFilter.java:109)
    at hudson.security.HudsonFilter.doFilter(HudsonFilter.java:168)
    at org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:193)
    at org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1601)
    at org.kohsuke.stapler.compression.CompressionFilter.doFilter(CompressionFilter.java:51)
    at org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:193)
    at org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1601)
    at hudson.util.CharacterEncodingFilter.doFilter(CharacterEncodingFilter.java:82)
    at org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:193)
    at org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1601)
    at org.kohsuke.stapler.DiagnosticThreadNameFilter.doFilter(DiagnosticThreadNameFilter.java:30)
    at org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:193)
    at org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1601)
    at jenkins.security.SuspiciousRequestFilter.doFilter(SuspiciousRequestFilter.java:36)
    at org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:193)
    at org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1601)
    at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:548)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
    at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:578)
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
    at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:235)
    at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1624)
    at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233)
    at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1435)
    at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188)
    at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:501)
    at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1594)
    at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186)
    at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1350)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
    at org.eclipse.jetty.server.Server.handle(Server.java:516)
    at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:388)
    at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:633)
    at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:380)
    at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:277)
    at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)
    at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:105)
    at org.eclipse.jetty.io.ssl.SslConnection$DecryptedEndPoint.onFillable(SslConnection.java:540)
    at org.eclipse.jetty.io.ssl.SslConnection.onFillable(SslConnection.java:395)
    at org.eclipse.jetty.io.ssl.SslConnection$2.succeeded(SslConnection.java:161)
    at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:105)
    at org.eclipse.jetty.io.ChannelEndPoint$1.run(ChannelEndPoint.java:104)
    at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:336)
    at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:313)
    at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:171)
    at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:129)
    at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:383)
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:882)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1036)
    at java.lang.Thread.run(Unknown Source)
timja commented 3 years ago

No need to create an issue and a PR for a trivial fix, thanks for the contribution, let's continue in the PR