You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Michael Brohl <mi...@ecomify.de> on 2023/05/17 17:03:28 UTC
Re: [jira] [Commented] (OFBIZ-10478) Reducing scope of variables in org.apache.ofbiz.base package
Hi Jacques,
during a vendor import I noticed that there are several utul methods are
being made private and therefore are not usable anymore.
Examples:
UtilDateTime#private static Timestamp getMonthStart(Timestamp stamp, int
daysLater, int monthsLater, TimeZone timeZone, Locale locale)
StringUtil#private static Map<String, String> strToMap(String str,
String delim, boolean trim)
etc.
Can you explain why you reduced the scope?
Has there been any discussion to deprecate the public scope for those
methods?
I'd suggest to revert those changes as it is not reasonable to make a
small subset of those utility methods private.
Thanks,
Michael
Am 30.09.22 um 10:11 schrieb ASF subversion and git services (Jira):
> [ https://issues.apache.org/jira/browse/OFBIZ-10478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17611416#comment-17611416 ]
>
> ASF subversion and git services commented on OFBIZ-10478:
> ---------------------------------------------------------
>
> Commit a1093281a48ba42e7e48438db9eb6deac89e7627 in ofbiz-framework's branch refs/heads/trunk from Jacques Le Roux
> [ https://gitbox.apache.org/repos/asf?p=ofbiz-framework.git;h=a1093281a4 ]
>
> Fixed: Reducing scope of variables in org.apache.ofbiz.base package (OFBIZ-10478)
>
> Reverts several public to private changes that should not have been
> runtime exception: java.lang.NoSuchMethodException
>
>
>> Reducing scope of variables in org.apache.ofbiz.base package
>> ------------------------------------------------------------
>>
>> Key: OFBIZ-10478
>> URL: https://issues.apache.org/jira/browse/OFBIZ-10478
>> Project: OFBiz
>> Issue Type: Sub-task
>> Components: base
>> Affects Versions: Trunk, Upcoming Branch
>> Reporter: Pradhan Yash Sharma
>> Assignee: Jacques Le Roux
>> Priority: Minor
>> Fix For: Upcoming Branch
>>
>> Attachments: OFBIZ-10478.patch
>>
>>
>
>
>
> --
> This message was sent by Atlassian Jira
> (v8.20.10#820010)
Re: [jira] [Commented] (OFBIZ-10478) Reducing scope of variables in org.apache.ofbiz.base package
Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Michael,
You are right, this should not have been done for SAFE util methods.
What a SAFE methods is, is well explained at https://web.mit.edu/javadev/doc/tutorial/java/javaOO/accesscontrol.html
So it's about security. The MIT example explains how to check if a method pose security issues.
Pradhan Yash Sharma's delicate and tedious effort for OFBIZ-10477 parent issue was neglected for 4 years.
That's how we lose good works and maybe even good developers, tired to work for nothing.
Details has been discussed at https://markmail.org/message/6ppybvmz7hu72ifc. Then lazy consensus applied.
So I decided to have a look when I got some time. I missed the SAFE util methods, not to be changed.
Another thing to say is that Groovy is able to call private methods:
https://stackoverflow.com/questions/40929264/how-is-groovy-able-to-access-private-methods-of-a-java-class
So it's not necessary to check Groovy calls, only Java calls are needed.
From a security perspective, I thought it's reasonable to check base, catalina, common and datafile OFBiz packages as they may be concerned.
In general, I can think of 3 options.
1. Get deeper for each method and check if it's reasonable to make private at security perspective. That should be done at creation or change.
2. Revert all util methods w/o checking possible security issues. It does not seem very wise to me.
3. Revert nothing. Not friendly.
Fortunately so far concerned util methods are all in base package and their number is limited.
I can apply option 1 for them. And we can do that later again when/if needed.
Thanks to your report, I'll made public all the reported methods but maybe one exception.
It seems safe to me to dot it to expandGeoGroup(GenericValue) if you can't use expandGeoGroup(String geoId, Delegator delegator)
Because despite both making calls to DB, no secret data are retrieved. But if you can use the 2nd it's maybe better?
There are other SAFE util methods I found (51 at all). Not all were part of OFBIZ-10478.
I have attached a new OFBIZ-10478.patch in OFBIZ-10478 issue. Please check and confirm it's OK with you before I push it.
Jacques
Le 17/05/2023 à 19:12, Michael Brohl a écrit :
> Some more findings (not necessarily complete) are:
>
> The method expandGeoGroup(GenericValue) from the type GeoWorker is not visible
> The method isNonnegativeInteger(String) from the type UtilValidate is not visible
> The method isPositiveInteger(String) from the type UtilValidate is not visible
> The method secondsSinceStart() from the type UtilTimer is not visible
> The method startTimer() from the type UtilTimer is not visible
> The method stringToTimeStamp(String, String, TimeZone, Locale) from the type UtilDateTime is not visible
> The method toDate(int, int, int, int, int, int) from the type UtilDateTime is not visible
> The method toDate(String, String, String, String, String, String) from the type UtilDateTime is not visible
> The method toDateString(Date, String) from the type UtilDateTime is not visible
> The method toLongObject(Object) from the type UtilMisc is not visible
>
> Regards,
>
> Michael
>
>
> Am 17.05.23 um 19:03 schrieb Michael Brohl:
>> Hi Jacques,
>>
>> during a vendor import I noticed that there are several utul methods are being made private and therefore are not usable anymore.
>>
>> Examples:
>>
>> UtilDateTime#private static Timestamp getMonthStart(Timestamp stamp, int daysLater, int monthsLater, TimeZone timeZone, Locale locale)
>> StringUtil#private static Map<String, String> strToMap(String str, String delim, boolean trim)
>>
>> etc.
>>
>> Can you explain why you reduced the scope?
>>
>> Has there been any discussion to deprecate the public scope for those methods?
>>
>> I'd suggest to revert those changes as it is not reasonable to make a small subset of those utility methods private.
>>
>> Thanks,
>>
>> Michael
>>
>>
>> Am 30.09.22 um 10:11 schrieb ASF subversion and git services (Jira):
>>> [
>>> https://issues.apache.org/jira/browse/OFBIZ-10478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17611416#comment-17611416
>>> ]
>>>
>>> ASF subversion and git services commented on OFBIZ-10478:
>>> ---------------------------------------------------------
>>>
>>> Commit a1093281a48ba42e7e48438db9eb6deac89e7627 in ofbiz-framework's branch refs/heads/trunk from Jacques Le Roux
>>> [ https://gitbox.apache.org/repos/asf?p=ofbiz-framework.git;h=a1093281a4 ]
>>>
>>> Fixed: Reducing scope of variables in org.apache.ofbiz.base package (OFBIZ-10478)
>>>
>>> Reverts several public to private changes that should not have been
>>> runtime exception: java.lang.NoSuchMethodException
>>>
>>>
>>>> Reducing scope of variables in org.apache.ofbiz.base package
>>>> ------------------------------------------------------------
>>>>
>>>> Key: OFBIZ-10478
>>>> URL: https://issues.apache.org/jira/browse/OFBIZ-10478
>>>> Project: OFBiz
>>>> Issue Type: Sub-task
>>>> Components: base
>>>> Affects Versions: Trunk, Upcoming Branch
>>>> Reporter: Pradhan Yash Sharma
>>>> Assignee: Jacques Le Roux
>>>> Priority: Minor
>>>> Fix For: Upcoming Branch
>>>>
>>>> Attachments: OFBIZ-10478.patch
>>>>
>>>>
>>>
>>>
>>>
>>> --
>>> This message was sent by Atlassian Jira
>>> (v8.20.10#820010)
Re: [jira] [Commented] (OFBIZ-10478) Reducing scope of variables in org.apache.ofbiz.base package
Posted by Michael Brohl <mi...@ecomify.de>.
Some more findings (not necessarily complete) are:
The method expandGeoGroup(GenericValue) from the type GeoWorker is not
visible
The method isNonnegativeInteger(String) from the type UtilValidate is
not visible
The method isPositiveInteger(String) from the type UtilValidate is not
visible
The method secondsSinceStart() from the type UtilTimer is not visible
The method startTimer() from the type UtilTimer is not visible
The method stringToTimeStamp(String, String, TimeZone, Locale) from the
type UtilDateTime is not visible
The method toDate(int, int, int, int, int, int) from the type
UtilDateTime is not visible
The method toDate(String, String, String, String, String, String) from
the type UtilDateTime is not visible
The method toDateString(Date, String) from the type UtilDateTime is not
visible
The method toLongObject(Object) from the type UtilMisc is not visible
Regards,
Michael
Am 17.05.23 um 19:03 schrieb Michael Brohl:
> Hi Jacques,
>
> during a vendor import I noticed that there are several utul methods
> are being made private and therefore are not usable anymore.
>
> Examples:
>
> UtilDateTime#private static Timestamp getMonthStart(Timestamp stamp,
> int daysLater, int monthsLater, TimeZone timeZone, Locale locale)
> StringUtil#private static Map<String, String> strToMap(String str,
> String delim, boolean trim)
>
> etc.
>
> Can you explain why you reduced the scope?
>
> Has there been any discussion to deprecate the public scope for those
> methods?
>
> I'd suggest to revert those changes as it is not reasonable to make a
> small subset of those utility methods private.
>
> Thanks,
>
> Michael
>
>
> Am 30.09.22 um 10:11 schrieb ASF subversion and git services (Jira):
>> [
>> https://issues.apache.org/jira/browse/OFBIZ-10478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17611416#comment-17611416
>> ]
>>
>> ASF subversion and git services commented on OFBIZ-10478:
>> ---------------------------------------------------------
>>
>> Commit a1093281a48ba42e7e48438db9eb6deac89e7627 in ofbiz-framework's
>> branch refs/heads/trunk from Jacques Le Roux
>> [
>> https://gitbox.apache.org/repos/asf?p=ofbiz-framework.git;h=a1093281a4 ]
>>
>> Fixed: Reducing scope of variables in org.apache.ofbiz.base package
>> (OFBIZ-10478)
>>
>> Reverts several public to private changes that should not have been
>> runtime exception: java.lang.NoSuchMethodException
>>
>>
>>> Reducing scope of variables in org.apache.ofbiz.base package
>>> ------------------------------------------------------------
>>>
>>> Key: OFBIZ-10478
>>> URL: https://issues.apache.org/jira/browse/OFBIZ-10478
>>> Project: OFBiz
>>> Issue Type: Sub-task
>>> Components: base
>>> Affects Versions: Trunk, Upcoming Branch
>>> Reporter: Pradhan Yash Sharma
>>> Assignee: Jacques Le Roux
>>> Priority: Minor
>>> Fix For: Upcoming Branch
>>>
>>> Attachments: OFBIZ-10478.patch
>>>
>>>
>>
>>
>>
>> --
>> This message was sent by Atlassian Jira
>> (v8.20.10#820010)