Merge pull request #7317 from RasaHQ/simplify-is-url

Simplify is_url function
RasaHQ · Nov 23, 2020 · 54aa43d · 54aa43d
2 parents e9aa047 + 6531e14
commit 54aa43d
Show file tree

Hide file tree

Showing 9 changed files with 47 additions and 32 deletions.
diff --git a/.github/workflows/ci-model-regression-on-schedule.yml b/.github/workflows/ci-model-regression-on-schedule.yml
@@ -112,8 +112,8 @@ jobs:
           test -d dataset/$DATASET || (echo "::warning::The ${{ matrix.dataset }} dataset doesn't exist. Skipping the job." \
             && echo "::set-output name=is_config_exists::false" && exit 0)
 
-          echo "::set-env name=DATASET::${DATASET}"
-          echo "::set-env name=CONFIG::${CONFIG}"
+          echo "DATASET=${DATASET}" >> $GITHUB_ENV
+          echo "CONFIG=${CONFIG}" >> $GITHUB_ENV
 
       - name: Set up Python 3.8 🐍
         uses: actions/setup-python@v2
@@ -124,7 +124,7 @@ jobs:
       - name: Read Poetry Version 🔢
         if: steps.set_dataset_config_vars.outputs.is_dataset_exists == 'true' && steps.set_dataset_config_vars.outputs.is_config_exists == 'true'
         run: |
-          echo "::set-env name=POETRY_VERSION::$(scripts/poetry-version.sh)"
+          echo "POETRY_VERSION=$(scripts/poetry-version.sh)" >> $GITHUB_ENV
         shell: bash
 
       - name: Install poetry 🦄

diff --git a/.github/workflows/ci-model-regression.yml b/.github/workflows/ci-model-regression.yml
@@ -215,8 +215,8 @@ jobs:
           test -d dataset/$DATASET || (echo "::warning::The ${{ matrix.dataset }} dataset doesn't exist. Skipping the job." \
             && echo "::set-output name=is_config_exists::false" && exit 0)
 
-          echo "::set-env name=DATASET::${DATASET}"
-          echo "::set-env name=CONFIG::${CONFIG}"
+          echo "DATASET=${DATASET}" >> $GITHUB_ENV
+          echo "CONFIG=${CONFIG}" >> $GITHUB_ENV
 
       - name: Set up Python 3.8 🐍
         uses: actions/setup-python@v2
@@ -227,7 +227,7 @@ jobs:
       - name: Read Poetry Version 🔢
         if: steps.set_dataset_config_vars.outputs.is_dataset_exists == 'true' && steps.set_dataset_config_vars.outputs.is_config_exists == 'true'
         run: |
-          echo "::set-env name=POETRY_VERSION::$(scripts/poetry-version.sh)"
+          echo "POETRY_VERSION=$(scripts/poetry-version.sh)" >> $GITHUB_ENV
         shell: bash
 
       - name: Install poetry 🦄
@@ -360,8 +360,8 @@ jobs:
           test -d dataset/$DATASET || (echo "::warning::The ${{ matrix.dataset }} dataset doesn't exist. Skipping the job." \
             && echo "::set-output name=is_config_exists::false" && exit 0)
 
-          echo "::set-env name=DATASET::${DATASET}"
-          echo "::set-env name=CONFIG::${CONFIG}"
+          echo "DATASET=${DATASET}" >> $GITHUB_ENV
+          echo "CONFIG=${CONFIG}" >> $GITHUB_ENV
 
       - name: Set up Python 3.8 🐍
         uses: actions/setup-python@v2
@@ -372,7 +372,7 @@ jobs:
       - name: Read Poetry Version 🔢
         if: steps.set_dataset_config_vars.outputs.is_dataset_exists == 'true' && steps.set_dataset_config_vars.outputs.is_config_exists == 'true'
         run: |
-          echo "::set-env name=POETRY_VERSION::$(scripts/poetry-version.sh)"
+          echo "POETRY_VERSION=$(scripts/poetry-version.sh)" >> $GITHUB_ENV
         shell: bash
 
       - name: Install poetry 🦄

diff --git a/.github/workflows/continous-integration.yml b/.github/workflows/continous-integration.yml
@@ -69,7 +69,7 @@ jobs:
 
     - name: Read Poetry Version 🔢
       run: |
-        echo "::set-env name=POETRY_VERSION::$(scripts/poetry-version.sh)"
+        echo "POETRY_VERSION=$(scripts/poetry-version.sh)" >> $GITHUB_ENV
       shell: bash
 
     - name: Install poetry 🦄
@@ -145,7 +145,7 @@ jobs:
 
     - name: Read Poetry Version 🔢
       run: |
-        echo "::set-env name=POETRY_VERSION::$(scripts/poetry-version.sh)"
+        echo "POETRY_VERSION=$(scripts/poetry-version.sh)" >> $GITHUB_ENV
       shell: bash
 
     - name: Install poetry 🦄
@@ -288,7 +288,7 @@ jobs:
 
     - name: Read Poetry Version 🔢
       run: |
-        echo "::set-env name=POETRY_VERSION::$(scripts/poetry-version.sh)"
+        echo "POETRY_VERSION=$(scripts/poetry-version.sh)" >> $GITHUB_ENV
       shell: bash
 
     - name: Install poetry 🦄
@@ -346,9 +346,9 @@ jobs:
         shell: bash
         run: |
           if [ -z "${{ secrets.RASABOT_AUTOMERGE_GITHUB_TOKEN }}" ]; then
-            echo ::set-env name=MERGE_TOKEN::${{ secrets.GITHUB_TOKEN }}
+            echo "MERGE_TOKEN=${{ secrets.GITHUB_TOKEN }}" >> $GITHUB_ENV
           else
-            echo ::set-env name=MERGE_TOKEN::${{ secrets.RASABOT_AUTOMERGE_GITHUB_TOKEN }}
+            echo "MERGE_TOKEN=${{ secrets.RASABOT_AUTOMERGE_GITHUB_TOKEN }}" >> $GITHUB_ENV
           fi
       - uses: rasahq/merge-pal-action@master
         with:

diff --git a/.github/workflows/documentation.yml b/.github/workflows/documentation.yml
@@ -38,7 +38,7 @@ jobs:
 
     - name: Read Poetry Version 🔢
       run: |
-        echo "::set-env name=POETRY_VERSION::$(scripts/poetry-version.sh)"
+        echo "POETRY_VERSION=$(scripts/poetry-version.sh)" >> $GITHUB_ENV
       shell: bash
 
     - name: Install poetry 🦄

diff --git a/.github/workflows/vulnerability-scan.yml b/.github/workflows/vulnerability-scan.yml
@@ -32,7 +32,7 @@ jobs:
         IMAGE_NAME=rasa/rasa:latest-scanned
         docker build -f $DOCKERFILE -t $IMAGE_NAME .
 
-        echo "::set-env name=IMAGE_WITH_POETRY_LOCK::$IMAGE_NAME"
+        echo "IMAGE_WITH_POETRY_LOCK=$IMAGE_NAME" >> $GITHUB_ENV
 
     - name: Scan image 🕵️‍♀️🕵️‍♂️
       uses: wochinge/gitrivy@6bf026b

diff --git a/changelog/7317.bugfix.md b/changelog/7317.bugfix.md
@@ -0,0 +1 @@
+Fixed Rasa Open Source not being able to fetch models from certain URLs.
diff --git a/rasa/core/run.py b/rasa/core/run.py
@@ -271,7 +271,8 @@ async def load_agent_on_start(
         )
     except Exception as e:
         rasa.shared.utils.io.raise_warning(
-            f"The model at '{model_path}' could not be loaded. " f"Error: {e}"
+            f"The model at '{model_path}' could not be loaded. "
+            f"Error: {type(e)}: {e}"
         )
         app.agent = None
 

diff --git a/rasa/nlu/utils/__init__.py b/rasa/nlu/utils/__init__.py
@@ -1,5 +1,4 @@
 import os
-import re
 from typing import Any, Optional, Text
 from pathlib import Path
 
@@ -49,23 +48,23 @@ def is_model_dir(model_dir: Text) -> bool:
 def is_url(resource_name: Text) -> bool:
     """Check whether the url specified is a well formed one.
 
-    Regex adapted from https://stackoverflow.com/a/7160778/3001665
-
     Args:
         resource_name: Remote URL to validate
 
-    Returns: `True` if valid, otherwise `False`.
+    Returns:
+        `True` if valid, otherwise `False`.
     """
-    URL_REGEX = re.compile(
-        r"^(?:http|ftp|file)s?://"  # http:// or https:// or file://
-        r"(?:(?:[A-Z0-9](?:[A-Z0-9-]{0,61}[A-Z0-9])?\.)+(?:[A-Z]{2,6}\.?|[A-Z0-9-]{2,}\.?)|"  # domain
-        r"localhost|"  # localhost
-        r"\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})"  # or ip
-        r"(?::\d+)?"  # optional port
-        r"(?:/?|[/?]\S+)$",
-        re.IGNORECASE,
-    )
-    return URL_REGEX.match(resource_name) is not None
+    from urllib import parse
+
+    try:
+        result = parse.urlparse(resource_name)
+    except Exception:
+        return False
+
+    if result.scheme == "file":
+        return bool(result.path)
+
+    return bool(result.scheme in ["http", "https", "ftp", "ftps"] and result.netloc)
 
 
 def remove_model(model_dir: Text) -> bool:

diff --git a/tests/nlu/test_utils.py b/tests/nlu/test_utils.py
@@ -8,6 +8,10 @@
 
 from rasa.shared.exceptions import RasaException
 import rasa.shared.nlu.training_data.message
+from rasa.nlu.tokenizers.convert_tokenizer import (
+    ORIGINAL_TF_HUB_MODULE_URL,
+    RESTRICTED_ACCESS_URL,
+)
 import rasa.shared.utils.io
 import rasa.utils.io as io_utils
 from rasa.nlu import utils
@@ -115,7 +119,17 @@ def test_remove_model_invalid(empty_model_dir):
         ("https://www.google.com", True),
         ("http://google.com", True),
         ("http://www.google.com", True),
-        ("http://a/b/c", False),
+        ("http://www.google.com?foo=bar", True),
+        ("http://a/b/c", True),
+        ("http://localhost:5002/api/projects/default/models/tags/production", True),
+        ("http://rasa-x:5002/api/projects/default/models/tags/production", True),
+        (
+            "http://rasa-x:5002/api/projects/default/models/tags/production?foo=bar",
+            True,
+        ),
+        (ORIGINAL_TF_HUB_MODULE_URL, True),
+        (RESTRICTED_ACCESS_URL, True),
+        ("file:///some/path/file", True),
     ],
 )
 def test_is_url(url: Text, result: bool):