[RFC] netx: add support for parsing and saving deployment.config files

[Omair Majid]

On 10/26/2010 05:37 PM, Deepak Bhole wrote:
> * Omair Majid<omajid at redhat.com>  [2010-10-25 09:42]:
>> On 10/21/2010 05:39 PM, Deepak Bhole wrote:
>>> * Omair Majid<omajid at redhat.com>   [2010-10-21 17:26]:
>>>> On 10/21/2010 05:05 PM, Deepak Bhole wrote:
>>>>> * Omair Majid<omajid at redhat.com>    [2010-10-18 11:48]:
>>>>>> Hi,
>>>>>>
>>>>>> As described on [1], The Java Plug-in and Java Web Start support
>>>>>> using various deployment.properties and deployment.config files to
>>>>>> set behaviour of these tools. The patch adds support for parsing and
>>>>>> saving these files to netx. This patch does not actually use any of
>>>>>> these settings; it just adds support so other parts of netx can
>>>>>> start using them.
>>>>>>
>>>>>> This was filed as a bug by someone against the original netx project [2].
>>>>>>
>>>>>> Any comments or concerns?
>>>>>>
>>>>>> Thanks,
>>>>>> Omair
>>>>>>
>>>>>> [1] http://download.oracle.com/javase/1.5.0/docs/guide/deployment/deployment-guide/properties.html
>>>>>> [2] http://sourceforge.net/tracker/?func=detail&aid=2832947&group_id=72541&atid=534854
>>>>>
>>>>> <snip>
>>>>>
>>>>>> +
>>>>>> +    /**
>>>>>> +     * Loads properties properties file, if one exists
>>>>>> +     *
>>>>>
>>>>> Minor typo above.
>>>>
>>>> Fixed.
>>>>
>>>>>
>>>>>> +     * @param type the ConfigType to load
>>>>>> +     * @param file the File to load Properties from
>>>>>
>>>>> <snip>
>>>>>
>>>>>> +            /* exit if there is a fatal exception loading the configuration */
>>>>>> +            if (isApplication) {
>>>>>> +                System.out.println(getMessage("RConfigurationError"));
>>>>>> +                System.exit(-1);
>>>>>
>>>>>
>>>>> Is there a reason you chose negative exit code? NetX uses +1 everywhere
>>>>> else.
>>>>>
>>>>
>>>> Ah, I didnt see that netx uses 1. I have fixed it now.
>>>>
>>>> Thanks for reviewing the patch. Area there other issues I should fix?
>>>>
>>>
>>> Nope, rest looks fine. Ok for commit to HEAD.
>>>
>>
>> Thanks for the review Deepak. Here is a a slightly updated version
>> that I plan to commit. I made a few minor tweaks to make this class
>> easier to use.
>>
>> 1. The class is now final and public so it can be used by other
>> packages not part of net.sourceforge.jnlp.runtime
>> 2. I renamed the methods so they make more sense. "save" and "load"
>> are now used for saving and loading the configuration respectively.
>> 3. Security checks have been added for getProperty(),
>> getPropertyNames() and setProperty().
>>
>
> save() should have an access check as well, as someone could call it and
> then parse the exception error message to figure out user.home. A check
> for file read access to the userPropertiesFile should be adequate.
>
> load() doesn't need to have it as there is no chain that ends up
> outputting file names, but it would still be a good idea to add a check
> there in case someone adds an output somewhere without realizing it.
>
> After above changes, ok for HEAD.
>

Thanks for the review. I accidentally pushed without these fixes; I have 
now pushed a new changeset to add these security checks.

Cheers,
Omair