Uploaded image for project: 'Slider'
  1. Slider
  2. SLIDER-1124

If unparsable port range is specified, Slider AM PortScanner.java setPortRange() should throw exception - add else part

    XMLWordPrintableJSON

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: Slider 0.80
    • Fix Version/s: Slider 0.91
    • Component/s: appmaster
    • Labels:
      None

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

        Attachments

        1. SLIDER-1124.3.patch
          5 kB
          Billie Rinaldi
        2. SLIDER-1124.2.patch
          4 kB
          Billie Rinaldi
        3. SLIDER-1124.1.patch
          4 kB
          Billie Rinaldi

          Activity

            People

            • Assignee:
              billie Billie Rinaldi
              Reporter:
              manojsamel Manoj Samel
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: