[RFC][PATCH][icedtea-web]: Added support for signed JNLP file- Updated Patch

[Jiri Vanek]

On 07/18/2011 10:37 PM, Saad Mohammad wrote:
> I have attached the updated copy of Patch1 that involves changes that you recommended.
> Patch2 has not been attached in this email because I haven't made any changes from my previous email.
>
> ...[snip]...
>>> As you have requested, I have added the copyright headers to JNLPMatcher.java and JNLPMatcherTest.java. I am not sure if you would also like me to add it to the jnlp files that are being used for test purposes.
>>> The reason why I didn't add the header to the jnlp files is because I didn't want anything from the copyright header to interfere with the test.
>>> So, that's why I have only added the headers to the two files but please let me know if you would also like me to add them to the jnlp files.
>>>
>>
>> Personally I agree with you. I But I think that headers files must be in each file. But plase wait with this change until jnlpMatcher is in and also/or patch for classloader(+ reproducers) is in to protect readability (then I'm afraid it will be necessary).
>>
>> Still some concerns remains (100% of new code :D )
>
> Okay, I will wait and then implement this change :)
>
>>
>>>
...snip...
>>>       for test in `find -type f` ; do \
>>
>> OK here ^, let i t be. Just FYI, find have argument to find all "NOT this suffix" options. (negation by ! or attribute not...)
>
> Changed this!
>
>>> diff -r 86abbf8be0b1 netx/net/sourceforge/jnlp/JNLPMatcher.java
>>> --- /dev/null    Thu Jan 01 00:00:00 1970 +0000
>>> +++ b/netx/net/sourceforge/jnlp/JNLPMatcher.java    Thu Jul 14 17:11:49 2011 -0400
>>> @@ -0,0 +1,275 @@
>>> +/* JNLPMatcher.java
>>> +   Copyright (C) 2011 Red Hat, Inc.
>>> +
...snip...
>>
>>
>> Study a little bit about Boolean x boolean and about autoboxing in Java. No more changes here then is already written (eg do NOT return BBBBolean from matchNodes)
>
> Changed this as recommended.
>
>>

>
>>
>> Regards J... and.. study interfaces;)
>
> I hope this is the 110% for Patch1! ;)
99% ;)
>
> --
> Cheers,
> Saad Mohammad
>
>
> Patch1b.patch
>
>
> diff -r 86abbf8be0b1 Makefile.am
> --- a/Makefile.am	Thu Jun 23 15:29:45 2011 +0200
> +++ b/Makefile.am	Mon Jul 18 15:48:33 2011 -0400
> @@ -538,7 +538,12 @@
>
>   run-netx-unit-tests: stamps/netx-unit-tests-compile.stamp \
>    $(JUNIT_RUNNER_JAR) $(TESTS_DIR)/$(REPORT_STYLES_DIRNAME)
> -	cp {$(NETX_UNIT_TEST_SRCDIR),$(NETX_UNIT_TEST_DIR)}/net/sourceforge/jnlp/basic.jnlp
> +	filename=" " ; \
> +	cd $(NETX_UNIT_TEST_SRCDIR) ; \
> +	for file in `find . -type f ! -iname *.java`; do\
> +		filename=`echo $$file `; \
> +		cp --parents $$filename $(NETX_UNIT_TEST_DIR) ; \
> +	done ; \
>   	cd $(NETX_UNIT_TEST_DIR) ; \
>   	class_names= ; \
>   	for test in `find -type f` ; do \


Although I have never problems with clear !, it is used to be much safer to use \! (to prevent shell expansion), and same for *.java should be "*.java" (for second one I can be wrong, but it will not work when in working directory will suddenly be any .java file)
> diff -r 86abbf8be0b1 netx/net/sourceforge/jnlp/JNLPMatcher.java
...snip...
> +
> +    /**
> +     * Compares both JNLP files
> +     *
> +     * @return true if both JNLP files are 'matched', otherwise false
> +     */
> +    public boolean isMatch() {
> +
> +        if (match == null)
> +            match = matchNodes(appTemplateNode, launchJNLPNode) ? true : false;
> +
> +        return match;
> +    }
> +

:-/ you had clear code form me:( You do not need to test boolean to return boolean. ^^ Is WRONG. Weather autoboxing is nice or not, Java have it. Then boolean IS Boolean from view of syntax..


  public boolean isMatch() {
       if (match == null){
            match = matchNodes(appTemplateNode, launchJNLPNode);
              }

         return match;
     }

> +    /**
...snip...


Everything else looks good, and I'm looking forward to have it in.


J.