You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@slider.apache.org by "Manoj Samel (JIRA)" <ji...@apache.org> on 2016/05/19 16:43:12 UTC

[jira] [Updated] (SLIDER-1124) If unparsable port range is specified, Slider AM PortScanner.java setPortRange() should throw exception

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

Manoj Samel updated SLIDER-1124:
--------------------------------
    Description: 
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 ???

  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 "-". 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
>
> 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 ???



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