TS: Bad approach used as fallback mechanism for gathering IP address

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

TS: Bad approach used as fallback mechanism for gathering IP address

Pavel Janousek
Hi,

I've investigated some issue now and seen there are several places in TS where we can see similar code like this:

static final String someIPproperty = System.getProperty("<some key>", "localhost");

or

static final String someIPproperty = System.getProperty("<some key>", "127.0.0.1");

As you can see above, this fallback mechanism is bound in this case (key value isn't found) to IPv4 only IP address at all. This approach is bad because we can run in other network stack - IPv6 is presented in these days - and in this case the such testcase fails with some strange error due to this issue.

I think we have two option how to resolve this issue:

1) Don't use fallback mechanism at all and fix (= remove) them in present testcases

2) Create auxiliary class which return 127.0.0.1 or ::1 string (but not string "localhost" because we can't be sure how it is translated (*))

(*) common setup in Linux is to translate localhost -> 127.0.0.1 and localhost6 -> ::1


Ad 1) I prefer this option because some forget/miss setting is very easy to catch

Ad 2) This auxiliary class should need to make some magic decision in which IP network stack mode we are now running and return IPv4 or IPv6 lookpack address accordingly.

WDYT about this?

--
Pavel Janousek
Senior JBoss QA Engineer


_______________________________________________
jboss-as7-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
Reply | Threaded
Open this post in threaded view
|

Re: TS: Bad approach used as fallback mechanism for gathering IP address

kkhan

On 2 Mar 2012, at 13:31, Pavel Janousek wrote:

> Hi,
>
> I've investigated some issue now and seen there are several places in TS where we can see similar code like this:
>
> static final String someIPproperty = System.getProperty("<some key>", "localhost");
>
> or
>
> static final String someIPproperty = System.getProperty("<some key>", "127.0.0.1");
>
> As you can see above, this fallback mechanism is bound in this case (key value isn't found) to IPv4 only IP address at all. This approach is bad because we can run in other network stack - IPv6 is presented in these days - and in this case the such testcase fails with some strange error due to this issue.
>
> I think we have two option how to resolve this issue:
>
> 1) Don't use fallback mechanism at all and fix (= remove) them in present testcases
>
> 2) Create auxiliary class which return 127.0.0.1 or ::1 string (but not string "localhost" because we can't be sure how it is translated (*))
>
> (*) common setup in Linux is to translate localhost -> 127.0.0.1 and localhost6 -> ::1
>
>
> Ad 1) I prefer this option because some forget/miss setting is very easy to catch
I don't like this one - having to remember to set all the addresses while developing will be annoying :-)

>
> Ad 2) This auxiliary class should need to make some magic decision in which IP network stack mode we are now running and return IPv4 or IPv6 lookpack address accordingly.
>
> WDYT about this?
>
> --
> Pavel Janousek
> Senior JBoss QA Engineer
>
>
> _______________________________________________
> jboss-as7-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/jboss-as7-dev


_______________________________________________
jboss-as7-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
Reply | Threaded
Open this post in threaded view
|

Re: TS: Bad approach used as fallback mechanism for gathering IP address

Stuart Douglas
In reply to this post by Pavel Janousek

On 03/03/2012, at 12:31 AM, Pavel Janousek wrote:

> Hi,
>
> I've investigated some issue now and seen there are several places in TS where we can see similar code like this:
>
> static final String someIPproperty = System.getProperty("<some key>", "localhost");
>
> or
>
> static final String someIPproperty = System.getProperty("<some key>", "127.0.0.1");
>
> As you can see above, this fallback mechanism is bound in this case (key value isn't found) to IPv4 only IP address at all. This approach is bad because we can run in other network stack - IPv6 is presented in these days - and in this case the such testcase fails with some strange error due to this issue.

The fallbacks are there for cases when no other stack is specified. As of today the test suite should honour all setting specified by the node0 and node1 addresses. If you are running under another network stack and have configured the build properly then these fallbacks should never be hit, they will only be used in the normal mvn install case with not other addresses set.

To be honest this sort of fallback should probably be implemented in the pom, so that the system properties are always defined. Feel free to volunteer to fix it.  

Stuart


>
> I think we have two option how to resolve this issue:
>
> 1) Don't use fallback mechanism at all and fix (= remove) them in present testcases
>
> 2) Create auxiliary class which return 127.0.0.1 or ::1 string (but not string "localhost" because we can't be sure how it is translated (*))
>
> (*) common setup in Linux is to translate localhost -> 127.0.0.1 and localhost6 -> ::1
>
>
> Ad 1) I prefer this option because some forget/miss setting is very easy to catch
>
> Ad 2) This auxiliary class should need to make some magic decision in which IP network stack mode we are now running and return IPv4 or IPv6 lookpack address accordingly.
>
> WDYT about this?
>
> --
> Pavel Janousek
> Senior JBoss QA Engineer
>
>
> _______________________________________________
> jboss-as7-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/jboss-as7-dev


_______________________________________________
jboss-as7-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
Reply | Threaded
Open this post in threaded view
|

Re: TS: Bad approach used as fallback mechanism for gathering IP address

Brian Stansberry
On 3/2/12 8:04 AM, Stuart Douglas wrote:

>
> On 03/03/2012, at 12:31 AM, Pavel Janousek wrote:
>
>> Hi,
>>
>> I've investigated some issue now and seen there are several places in TS where we can see similar code like this:
>>
>> static final String someIPproperty = System.getProperty("<some key>", "localhost");
>>
>> or
>>
>> static final String someIPproperty = System.getProperty("<some key>", "127.0.0.1");
>>
>> As you can see above, this fallback mechanism is bound in this case (key value isn't found) to IPv4 only IP address at all. This approach is bad because we can run in other network stack - IPv6 is presented in these days - and in this case the such testcase fails with some strange error due to this issue.
>
> The fallbacks are there for cases when no other stack is specified. As of today the test suite should honour all setting specified by the node0 and node1 addresses. If you are running under another network stack and have configured the build properly then these fallbacks should never be hit, they will only be used in the normal mvn install case with not other addresses set.
>
> To be honest this sort of fallback should probably be implemented in the pom, so that the system properties are always defined. Feel free to volunteer to fix it.
>

+1; it would be nice to know these are always set in the pom. Then when
reviewing a patch, the presence of "localhost" or "127.0.0.1" is grounds
for kicking it back, with no need to even think about it.

> Stuart
>
>
>>
>> I think we have two option how to resolve this issue:
>>
>> 1) Don't use fallback mechanism at all and fix (= remove) them in present testcases
>>
>> 2) Create auxiliary class which return 127.0.0.1 or ::1 string (but not string "localhost" because we can't be sure how it is translated (*))
>>
>> (*) common setup in Linux is to translate localhost ->  127.0.0.1 and localhost6 ->  ::1
>>
>>
>> Ad 1) I prefer this option because some forget/miss setting is very easy to catch
>>
>> Ad 2) This auxiliary class should need to make some magic decision in which IP network stack mode we are now running and return IPv4 or IPv6 lookpack address accordingly.
>>
>> WDYT about this?
>>
>> --
>> Pavel Janousek
>> Senior JBoss QA Engineer
>>
>>
>> _______________________________________________
>> jboss-as7-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
>
>
> _______________________________________________
> jboss-as7-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/jboss-as7-dev


--
Brian Stansberry
Principal Software Engineer
JBoss by Red Hat
_______________________________________________
jboss-as7-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
Reply | Threaded
Open this post in threaded view
|

Re: TS: Bad approach used as fallback mechanism for gathering IP address

Pavel Janousek
----- Original Message -----

> From: "Brian Stansberry" <[hidden email]>
>
> On 3/2/12 8:04 AM, Stuart Douglas wrote:
> >
> > On 03/03/2012, at 12:31 AM, Pavel Janousek wrote:
> >
> >> Hi,
> >>
> >> I've investigated some issue now and seen there are several places
> >> in TS where we can see similar code like this:
> >>
> >> static final String someIPproperty = System.getProperty("<some
> >> key>", "localhost");
> >>
> >> or
> >>
> >> static final String someIPproperty = System.getProperty("<some
> >> key>", "127.0.0.1");
> >>
> >> As you can see above, this fallback mechanism is bound in this
> >> case (key value isn't found) to IPv4 only IP address at all. This
> >> approach is bad because we can run in other network stack - IPv6
> >> is presented in these days - and in this case the such testcase
> >> fails with some strange error due to this issue.
> >
> > The fallbacks are there for cases when no other stack is specified.
> > As of today the test suite should honour all setting specified by
> > the node0 and node1 addresses. If you are running under another
> > network stack and have configured the build properly then these
> > fallbacks should never be hit, they will only be used in the
> > normal mvn install case with not other addresses set.
> >
> > To be honest this sort of fallback should probably be implemented
> > in the pom, so that the system properties are always defined. Feel
> > free to volunteer to fix it.
> >
>
> +1; it would be nice to know these are always set in the pom. Then
> when
> reviewing a patch, the presence of "localhost" or "127.0.0.1" is
> grounds
> for kicking it back, with no need to even think about it.

So could be a conclusion from related topic - the JIRA about request to testsuite structure maintainer(s) to accommodate and guarantee this request/feature set-up in every place(module) plus clean-up testcases java code to remove this fallback mechanism from java source code at all?

If so, I'll rewrite one already filled placeholder...

Pavel
_______________________________________________
jboss-as7-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
Reply | Threaded
Open this post in threaded view
|

Re: TS: Bad approach used as fallback mechanism for gathering IP address

Brian Stansberry
On 3/6/12 7:58 AM, Pavel Janousek wrote:

> ----- Original Message -----
>> From: "Brian Stansberry"<[hidden email]>
>>
>> On 3/2/12 8:04 AM, Stuart Douglas wrote:
>>>
>>> On 03/03/2012, at 12:31 AM, Pavel Janousek wrote:
>>>
>>>> Hi,
>>>>
>>>> I've investigated some issue now and seen there are several places
>>>> in TS where we can see similar code like this:
>>>>
>>>> static final String someIPproperty = System.getProperty("<some
>>>> key>", "localhost");
>>>>
>>>> or
>>>>
>>>> static final String someIPproperty = System.getProperty("<some
>>>> key>", "127.0.0.1");
>>>>
>>>> As you can see above, this fallback mechanism is bound in this
>>>> case (key value isn't found) to IPv4 only IP address at all. This
>>>> approach is bad because we can run in other network stack - IPv6
>>>> is presented in these days - and in this case the such testcase
>>>> fails with some strange error due to this issue.
>>>
>>> The fallbacks are there for cases when no other stack is specified.
>>> As of today the test suite should honour all setting specified by
>>> the node0 and node1 addresses. If you are running under another
>>> network stack and have configured the build properly then these
>>> fallbacks should never be hit, they will only be used in the
>>> normal mvn install case with not other addresses set.
>>>
>>> To be honest this sort of fallback should probably be implemented
>>> in the pom, so that the system properties are always defined. Feel
>>> free to volunteer to fix it.
>>>
>>
>> +1; it would be nice to know these are always set in the pom. Then
>> when
>> reviewing a patch, the presence of "localhost" or "127.0.0.1" is
>> grounds
>> for kicking it back, with no need to even think about it.
>
> So could be a conclusion from related topic - the JIRA about request to testsuite structure maintainer(s) to accommodate and guarantee this request/feature set-up in every place(module) plus clean-up testcases java code to remove this fallback mechanism from java source code at all?
>
> If so, I'll rewrite one already filled placeholder...
>

I'm not sure I follow what you're saying. I think we need a JIRA to make
sure the appropriate properties are always set by all the testsuite
module poms, and then probably several JIRAs to get rid of the various
places that use System.getProperty("<some key>", "localhost"). Maybe a
single JIRA for the latter if it's a small enough task that one person
can do it.


--
Brian Stansberry
Principal Software Engineer
JBoss by Red Hat
_______________________________________________
jboss-as7-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
Reply | Threaded
Open this post in threaded view
|

Re: TS: Bad approach used as fallback mechanism for gathering IP address

Pavel Janousek
----- Original Message -----

> From: "Brian Stansberry" <[hidden email]>
>
> On 3/6/12 7:58 AM, Pavel Janousek wrote:
> > ----- Original Message -----
> >> From: "Brian Stansberry"<[hidden email]>
> >>
> >> On 3/2/12 8:04 AM, Stuart Douglas wrote:
> >>>
> >>> On 03/03/2012, at 12:31 AM, Pavel Janousek wrote:
> >>>
> >>>> Hi,
> >>>>
> >>>> I've investigated some issue now and seen there are several
> >>>> places
> >>>> in TS where we can see similar code like this:
> >>>>
> >>>> static final String someIPproperty = System.getProperty("<some
> >>>> key>", "localhost");
> >>>>
> >>>> or
> >>>>
> >>>> static final String someIPproperty = System.getProperty("<some
> >>>> key>", "127.0.0.1");
> >>>>
> >>>> As you can see above, this fallback mechanism is bound in this
> >>>> case (key value isn't found) to IPv4 only IP address at all.
> >>>> This
> >>>> approach is bad because we can run in other network stack - IPv6
> >>>> is presented in these days - and in this case the such testcase
> >>>> fails with some strange error due to this issue.
> >>>
> >>> The fallbacks are there for cases when no other stack is
> >>> specified.
> >>> As of today the test suite should honour all setting specified by
> >>> the node0 and node1 addresses. If you are running under another
> >>> network stack and have configured the build properly then these
> >>> fallbacks should never be hit, they will only be used in the
> >>> normal mvn install case with not other addresses set.
> >>>
> >>> To be honest this sort of fallback should probably be implemented
> >>> in the pom, so that the system properties are always defined.
> >>> Feel
> >>> free to volunteer to fix it.
> >>>
> >>
> >> +1; it would be nice to know these are always set in the pom. Then
> >> when
> >> reviewing a patch, the presence of "localhost" or "127.0.0.1" is
> >> grounds
> >> for kicking it back, with no need to even think about it.
> >
> > So could be a conclusion from related topic - the JIRA about
> > request to testsuite structure maintainer(s) to accommodate and
> > guarantee this request/feature set-up in every place(module) plus
> > clean-up testcases java code to remove this fallback mechanism
> > from java source code at all?
> >
> > If so, I'll rewrite one already filled placeholder...
> >
>
> I'm not sure I follow what you're saying. I think we need a JIRA to
> make
> sure the appropriate properties are always set by all the testsuite
> module poms, and then probably several JIRAs to get rid of the
> various
> places that use System.getProperty("<some key>", "localhost"). Maybe
> a
> single JIRA for the latter if it's a small enough task that one
> person
> can do it.

Ok, I'm fine with your suggestion and follow it...

Pavel
_______________________________________________
jboss-as7-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/jboss-as7-dev