Finding relevant jelly for branch-api

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Finding relevant jelly for branch-api

Raihaan Shouhell
Hi all,

Recently I've been looking into some page loading issues that plague some of my jenkins instances while doing some optimizations I came across this line

https://github.com/jenkinsci/branch-api-plugin/blob/ad83debf8f48d6ad2661ec2549116b3df8cb0f5b/src/main/java/jenkins/branch/OrganizationFolderViewHolder.java#L219

getItems() essentially checks every item for item.read permissions and returns a list of permitted items. I'd like to see if the relevant jelly files that call this has access to items and the use of getItems() (and hasPermission) can be reduced.

Is there any good way of tracing how pages get rendered?

Personally I'm not sure if this is even worth looking into (Some of my organizations have 900+ items) would anyone have any suggestions?

Cheers,
Raihaan

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/505f8bb3-ba63-43c5-a5a4-272089d60d16%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: Finding relevant jelly for branch-api

Jesse Glick-4
On Mon, Jan 20, 2020 at 5:13 AM Raihaan Shouhell
<[hidden email]> wrote:
> I'd like to see if the relevant jelly files that call this has access to items and the use of getItems() (and hasPermission) can be reduced.

Doubtful since this is just overriding

https://javadoc.jenkins.io/hudson/model/View.html#getDisplayName--

If permission checks are a bottleneck here it might be a problem in
the `AuthorizationStrategy`. Alternately, it would probably be fine to
wrap the `getItems` call inside `ACL.as(ACL.SYSTEM)` since someone
with permission to view the organization folder very likely also has
permission to view all the child repositories (it would be an obscure
authorization strategy that decided otherwise), and at worst the
leaked information would be a count of hidden subfolders.

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr1dyG9xiEWwZWFcyoMO7EKTHs32QOAEcX6VsJOC-W3Y-Q%40mail.gmail.com.
Reply | Threaded
Open this post in threaded view
|

Re: Finding relevant jelly for branch-api

Raihaan Shouhell
Thanks for the reply Jesse, my idea was because branch-api wants to show the number of items in its view if core should provide that feature and that way when core calls getItems() in jelly
 you can get the size and therefore skip the need for permissions checks. But I'm not sure if its worth the effort.

On Wednesday, 22 January 2020 08:43:30 UTC+8, Jesse Glick wrote:
On Mon, Jan 20, 2020 at 5:13 AM Raihaan Shouhell
<<a href="javascript:" target="_blank" gdf-obfuscated-mailto="sa8pDg2qEAAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">raihaan...@...> wrote:
> I'd like to see if the relevant jelly files that call this has access to items and the use of getItems() (and hasPermission) can be reduced.

Doubtful since this is just overriding

<a href="https://javadoc.jenkins.io/hudson/model/View.html#getDisplayName--" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fjavadoc.jenkins.io%2Fhudson%2Fmodel%2FView.html%23getDisplayName--\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFJRdQYBVtnYSSPKMaxpqMxLFiQNw&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fjavadoc.jenkins.io%2Fhudson%2Fmodel%2FView.html%23getDisplayName--\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFJRdQYBVtnYSSPKMaxpqMxLFiQNw&#39;;return true;">https://javadoc.jenkins.io/hudson/model/View.html#getDisplayName--

If permission checks are a bottleneck here it might be a problem in
the `AuthorizationStrategy`. Alternately, it would probably be fine to
wrap the `getItems` call inside `ACL.as(ACL.SYSTEM)` since someone
with permission to view the organization folder very likely also has
permission to view all the child repositories (it would be an obscure
authorization strategy that decided otherwise), and at worst the
leaked information would be a count of hidden subfolders.

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/b6feb5ab-51a9-4be3-b8ca-7c20b9997d1f%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: Finding relevant jelly for branch-api

Jesse Glick-4
On Wed, Jan 22, 2020 at 9:48 AM Raihaan Shouhell
<[hidden email]> wrote:
> when core calls getItems() in jelly
>  you can get the size and therefore skip the need for permissions checks

`getItems` would still need to run permission checks, regardless of
where it is implemented.

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr32V%3Dn5PSv5VVPphMy820A53XTCnqyRhkKhyLXBHKgfkg%40mail.gmail.com.
Reply | Threaded
Open this post in threaded view
|

Re: Finding relevant jelly for branch-api

Raihaan Shouhell
Oh i was thinking because ViewImpl in branch-api calls getItems in getDisplayName since view has to call get items it can just check the size from there eliminating the need to call getItems twice or three times. 
Only problem is that view has to be reworked to incorporate this. Might not be worth the trouble.

On Wednesday, 22 January 2020 23:29:45 UTC+8, Jesse Glick wrote:
On Wed, Jan 22, 2020 at 9:48 AM Raihaan Shouhell
<<a href="javascript:" target="_blank" gdf-obfuscated-mailto="bUx-tWnaEAAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">raihaan...@...> wrote:
> when core calls getItems() in jelly
>  you can get the size and therefore skip the need for permissions checks

`getItems` would still need to run permission checks, regardless of
where it is implemented.

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/a69a2a45-8e98-43e7-b82f-ae12a9c5cb97%40googlegroups.com.