[ 
https://issues.apache.org/jira/browse/SLIDER-1124?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Manoj Samel updated SLIDER-1124:
--------------------------------
    Description: 
{noformat}
The issue was discovered when a JSON was generated with IDE and instead of "-", 
it somehow inserted a similar looking but different character sequence. In this 
case, Slider AM fails to start with following ERROR

[main] ERROR main.ServiceLauncher - No available ports found in configured 
range {}

Above error gives a impression that all available ports in specified range were 
some how not available; which is not the case.

Json was updated using IDE, and while at first glance it looks like 
"32201-33100", it was really "32201–33100" . The character in the second case 
is not a "-" but actually three characters that together appear somewhat like 
it (but its wider and lower than - ).

So this is neither a "," separated list or "-" range as the code expects and it 
errors out.

It would be useful if such "bad" range is caught up earlier with clearer 
message like invalid or unparsable port range specified.

Looking at the code

SliderAppMaster.java buildPortScanner() reads the key KEY_ALLOWED_PORT_RANGE 
and passes the associated value to portScanner.setPortRange().

In PortScanner.java setPortRange() , it first tries to split on "," or else 
tries to split on "-".  However, there is no "else" part if it does not finds 
the "-" pattern (which will happen in above case). Since there is no else part, 
there is no exception etc. thrown at this point and this.remainingPortsToCheck 
gets set to a empty set, resulting in more obscure error later in 
getAvailablePortViaPortArray(). 

I think it would be good to have a "else" part added to range matchers below 
and a exception with input text thrown at that point - so the misconfigured 
value will be obvious

      Matcher m = SINGLE_NUMBER.matcher(range.trim());
      if (m.find()) {
        inputPorts.add(Integer.parseInt(m.group()));
      } else {
        m = NUMBER_RANGE.matcher(range.trim());
        if (m.find()) {
        } // else is missing ..... Add with a exception ???
{noformat}

  was:
The issue was discovered when a JSON was generated with IDE and instead of "-", 
it somehow inserted a similar looking but different character sequence. In this 
case, Slider AM fails to start with following ERROR

[main] ERROR main.ServiceLauncher - No available ports found in configured 
range {}

Above error gives a impression that all available ports in specified range were 
some how not available; which is not the case.

Json was updated using IDE, and while at first glance it looks like 
"32201-33100", it was really "32201–33100" . The character in the second case 
is not a "-" but actually three characters that together appear somewhat like 
it (but its wider and lower than - ).

So this is neither a "," separated list or "-" range as the code expects and it 
errors out.

It would be useful if such "bad" range is caught up earlier with clearer 
message like invalid or unparsable port range specified.

Looking at the code

SliderAppMaster.java buildPortScanner() reads the key KEY_ALLOWED_PORT_RANGE 
and passes the associated value to portScanner.setPortRange().

In PortScanner.java setPortRange() , it first tries to split on "," or else 
tries to split on {noformat}"-"{noformat}.  However, there is no "else" part if 
it does not finds the "-" pattern (which will happen in above case). Since 
there is no else part, there is no exception etc. thrown at this point and 
this.remainingPortsToCheck gets set to a empty set, resulting in more obscure 
error later in getAvailablePortViaPortArray(). 

I think it would be good to have a "else" part added to range matchers below 
and a exception with input text thrown at that point - so the misconfigured 
value will be obvious

      Matcher m = SINGLE_NUMBER.matcher(range.trim());
      if (m.find()) {
        inputPorts.add(Integer.parseInt(m.group()));
      } else {
        m = NUMBER_RANGE.matcher(range.trim());
        if (m.find()) {
        } // else is missing ..... Add with a exception ???


> If unparsable port range is specified, Slider AM PortScanner.java 
> setPortRange() should throw exception
> -------------------------------------------------------------------------------------------------------
>
>                 Key: SLIDER-1124
>                 URL: https://issues.apache.org/jira/browse/SLIDER-1124
>             Project: Slider
>          Issue Type: Improvement
>          Components: appmaster
>    Affects Versions: Slider 0.80
>            Reporter: Manoj Samel
>
> {noformat}
> The issue was discovered when a JSON was generated with IDE and instead of 
> "-", it somehow inserted a similar looking but different character sequence. 
> In this case, Slider AM fails to start with following ERROR
> [main] ERROR main.ServiceLauncher - No available ports found in configured 
> range {}
> Above error gives a impression that all available ports in specified range 
> were some how not available; which is not the case.
> Json was updated using IDE, and while at first glance it looks like 
> "32201-33100", it was really "32201–33100" . The character in the second case 
> is not a "-" but actually three characters that together appear somewhat like 
> it (but its wider and lower than - ).
> So this is neither a "," separated list or "-" range as the code expects and 
> it errors out.
> It would be useful if such "bad" range is caught up earlier with clearer 
> message like invalid or unparsable port range specified.
> Looking at the code
> SliderAppMaster.java buildPortScanner() reads the key KEY_ALLOWED_PORT_RANGE 
> and passes the associated value to portScanner.setPortRange().
> In PortScanner.java setPortRange() , it first tries to split on "," or else 
> tries to split on "-".  However, there is no "else" part if it does not finds 
> the "-" pattern (which will happen in above case). Since there is no else 
> part, there is no exception etc. thrown at this point and 
> this.remainingPortsToCheck gets set to a empty set, resulting in more obscure 
> error later in getAvailablePortViaPortArray(). 
> I think it would be good to have a "else" part added to range matchers below 
> and a exception with input text thrown at that point - so the misconfigured 
> value will be obvious
>       Matcher m = SINGLE_NUMBER.matcher(range.trim());
>       if (m.find()) {
>         inputPorts.add(Integer.parseInt(m.group()));
>       } else {
>         m = NUMBER_RANGE.matcher(range.trim());
>         if (m.find()) {
>         } // else is missing ..... Add with a exception ???
> {noformat}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to