Merge pull request #18986 from arash77/fix-none-alphanumeric-characte…

…rs-in-slug

Fix issue with generating slug for sharing

client/src/components/Sharing/SlugInput.vue

@@ -15,7 +15,7 @@ const initialSlug = props.slug;
 function onChange(value: string) {
     const slugFormatted = value
         .replace(/\s+/g, "-")
-        .replace(/[^a-zA-Z0-9-]/g, "")
+        .replace(/[/:?#]/g, "")
         .toLowerCase();
 
     emit("change", slugFormatted);

lib/galaxy/dependencies/pinned-typecheck-requirements.txt

@@ -23,6 +23,7 @@ types-html5lib==1.1.11.20241018
 types-markdown==3.7.0.20240822
 types-paramiko==3.5.0.20240928
 types-python-dateutil==2.9.0.20241003
+types-python-slugify==8.0.2.20240310
 types-pyyaml==6.0.12.20240917
 types-requests==2.31.0.6
 types-s3transfer==0.10.3

lib/galaxy/managers/base.py

@@ -1294,13 +1294,6 @@ def raise_filter_err(attr, op, val, msg):
     raise exceptions.RequestParameterInvalidException(msg, column=attr, operation=op, val=val)
 
 
-def is_valid_slug(slug):
-    """Returns true iff slug is valid."""
-
-    VALID_SLUG_RE = re.compile(r"^[a-z0-9\-]+$")
-    return VALID_SLUG_RE.match(slug)
-
-
 class SortableManager:
     """A manager interface for parsing order_by strings into actual 'order by' queries."""
 

lib/galaxy/managers/pages.py

@@ -248,7 +248,7 @@ def create_page(self, trans, payload: CreatePagePayload):
             raise exceptions.ObjectAttributeMissingException("Page name is required")
         elif not payload.slug:
             raise exceptions.ObjectAttributeMissingException("Page id is required")
-        elif not base.is_valid_slug(payload.slug):
+        elif not sharable.SlugBuilder.is_valid_slug(payload.slug):
             raise exceptions.ObjectAttributeInvalidException(
                 "Page identifier must consist of only lowercase letters, numbers, and the '-' character"
             )

lib/galaxy/managers/sharable.py

@@ -11,7 +11,6 @@
 """
 
 import logging
-import re
 from typing import (
     Any,
     List,
@@ -20,6 +19,7 @@
     Type,
 )
 
+from slugify import slugify
 from sqlalchemy import (
     exists,
     false,
@@ -291,7 +291,7 @@ def set_slug(self, item, new_slug, user, flush=True):
         Validate and set the new slug for `item`.
         """
         # precondition: has been validated
-        if not self.is_valid_slug(new_slug):
+        if not SlugBuilder.is_valid_slug(new_slug):
             raise exceptions.RequestParameterInvalidException("Invalid slug", slug=new_slug)
 
         if item.slug == new_slug:
@@ -309,23 +309,6 @@ def set_slug(self, item, new_slug, user, flush=True):
                 session.commit()
         return item
 
-    def is_valid_slug(self, slug):
-        """
-        Returns true if `slug` is valid.
-        """
-        VALID_SLUG_RE = re.compile(r"^[a-z0-9\-]+$")
-        return VALID_SLUG_RE.match(slug)
-
-    def _slugify(self, start_with):
-        # Replace whitespace with '-'
-        slug_base = re.sub(r"\s+", "-", start_with)
-        # Remove all non-alphanumeric characters.
-        slug_base = re.sub(r"[^a-zA-Z0-9\-]", "", slug_base)
-        # Remove trailing '-'.
-        if slug_base.endswith("-"):
-            slug_base = slug_base[:-1]
-        return slug_base
-
     def _default_slug_base(self, item):
         # override in subclasses
         if hasattr(item, "title"):
@@ -341,7 +324,7 @@ def get_unique_slug(self, item):
 
         # Setup slug base.
         if cur_slug is None or cur_slug == "":
-            slug_base = self._slugify(self._default_slug_base(item))
+            slug_base = slugify(self._default_slug_base(item), allow_unicode=True)
         else:
             slug_base = cur_slug
 
@@ -580,6 +563,11 @@ def create_item_slug(self, sa_session, item) -> bool:
         item.slug = new_slug
         return item.slug == cur_slug
 
+    @classmethod
+    def is_valid_slug(self, slug):
+        """Returns true if slug is valid."""
+        return slugify(slug, allow_unicode=True) == slug
+
 
 def slug_exists(session, model_class, user, slug, ignore_deleted=False):
     stmt = select(exists().where(model_class.user == user).where(model_class.slug == slug))

lib/galaxy/schema/schema.py

@@ -3678,7 +3678,7 @@ class PageSummaryBase(Model):
         ...,  # Required
         title="Identifier",
         description="The title slug for the page URL, must be unique.",
-        pattern=r"^[a-z0-9\-]+$",
+        pattern=r"^[^/:?#]+$",
     )
 
 

lib/galaxy/webapps/base/controller.py

@@ -1090,7 +1090,7 @@ class SharableMixin:
 
     def _is_valid_slug(self, slug):
         """Returns true if slug is valid."""
-        return managers_base.is_valid_slug(slug)
+        return SlugBuilder.is_valid_slug(slug)
 
     @web.expose
     @web.require_login("modify Galaxy items")
@@ -1123,6 +1123,11 @@ def share(self, trans, id=None, email="", **kwd):
     @web.expose
     def display_by_username_and_slug(self, trans, username, slug, **kwargs):
         """Display item by username and slug."""
+        # Ensure slug is in the correct format.
+        slug = slug.encode("latin1").decode("utf-8")
+        self._display_by_username_and_slug(trans, username, slug, **kwargs)
+
+    def _display_by_username_and_slug(self, trans, username, slug, **kwargs):
         raise NotImplementedError()
 
     def get_item(self, trans, id):

lib/galaxy/webapps/galaxy/controllers/dataset.py

@@ -431,8 +431,7 @@ def _get_dataset_for_edit(self, trans, dataset_id):
             trans.app.security_agent.set_dataset_permission(data.dataset, permissions)
         return data, None
 
-    @web.expose
-    def display_by_username_and_slug(self, trans, username, slug, filename=None, preview=True, **kwargs):
+    def _display_by_username_and_slug(self, trans, username, slug, filename=None, preview=True, **kwargs):
         """Display dataset by username and slug; because datasets do not yet have slugs, the slug is the dataset's id."""
         dataset = self._check_dataset(trans, slug)
         # Filename used for composite types.

lib/galaxy/webapps/galaxy/controllers/history.py

@@ -88,8 +88,7 @@ def view(self, trans, id=None, show_deleted=False, show_hidden=False, use_panels
             "allow_user_dataset_purge": trans.app.config.allow_user_dataset_purge,
         }
 
-    @web.expose
-    def display_by_username_and_slug(self, trans, username, slug, **kwargs):
+    def _display_by_username_and_slug(self, trans, username, slug, **kwargs):
         """
         Display history based on a username and slug.
         """

lib/galaxy/webapps/galaxy/controllers/page.py

@@ -170,8 +170,7 @@ def display(self, trans, id, **kwargs):
             raise web.httpexceptions.HTTPNotFound()
         return self.display_by_username_and_slug(trans, page.user.username, page.slug)
 
-    @web.expose
-    def display_by_username_and_slug(self, trans, username, slug, **kwargs):
+    def _display_by_username_and_slug(self, trans, username, slug, **kwargs):
         """Display page based on a username and slug."""
 
         # Get page.

lib/galaxy/webapps/galaxy/controllers/visualization.py

@@ -111,8 +111,7 @@ def imp(self, trans, id, **kwargs):
                 use_panels=True,
             )
 
-    @web.expose
-    def display_by_username_and_slug(self, trans, username, slug, **kwargs):
+    def _display_by_username_and_slug(self, trans, username, slug, **kwargs):
         """Display visualization based on a username and slug."""
 
         # Get visualization.

lib/galaxy/webapps/galaxy/controllers/workflow.py

@@ -43,8 +43,7 @@ class WorkflowController(BaseUIController, SharableMixin, UsesStoredWorkflowMixi
     def __init__(self, app: StructuredApp):
         super().__init__(app)
 
-    @web.expose
-    def display_by_username_and_slug(self, trans, username, slug, format="html", **kwargs):
+    def _display_by_username_and_slug(self, trans, username, slug, format="html", **kwargs):
         """
         Display workflow based on a username and slug. Format can be html, json, or json-download.
         """

lib/galaxy/webapps/galaxy/services/visualizations.py

@@ -8,10 +8,7 @@
 )
 
 from galaxy import exceptions
-from galaxy.managers.base import (
-    is_valid_slug,
-    security_check,
-)
+from galaxy.managers.base import security_check
 from galaxy.managers.context import ProvidesUserContext
 from galaxy.managers.sharable import (
     slug_exists,
@@ -302,7 +299,7 @@ def _create_visualization(
         # Error checking.
         if slug:
             slug_err = ""
-            if not is_valid_slug(slug):
+            if not SlugBuilder.is_valid_slug(slug):
                 slug_err = (
                     "visualization identifier must consist of only lowercase letters, numbers, and the '-' character"
                 )

lib/galaxy_test/api/test_histories.py

@@ -544,6 +544,12 @@ def test_anonymous_can_import_published(self):
             }
             self.dataset_populator.import_history(import_data)
 
+    def test_publish_non_alphanumeric(self):
+        history_name = "تاریخچه"
+        history_id = self.dataset_populator.new_history(name=history_name)
+        response = self.dataset_populator.make_public(history_id)
+        assert history_name in response["username_and_slug"]
+
     def test_immutable_history_update_fails(self):
         history_id = self._create_history("TestHistoryForImmutability")["id"]
 

packages/app/setup.cfg

@@ -64,6 +64,7 @@ install_requires =
     pulsar-galaxy-lib>=0.15.0.dev0
     pydantic>=2.7.4
     pysam>=0.21
+    python-slugify
     PyJWT
     PyYAML
     refgenconf>=0.12.0

pyproject.toml

@@ -83,6 +83,7 @@ dependencies = [
     "python-dateutil",
     "python-magic",
     "python-multipart",  # required to support form parsing in FastAPI/Starlette
+    "python-slugify",
     "PyYAML",
     "refgenconf>=0.12.0",
     "regex",
@@ -172,6 +173,7 @@ typecheck = [
     "types-Markdown",
     "types-paramiko",
     "types-python-dateutil",
+    "types-python-slugify",
     "types-PyYAML",
     "types-requests",
     "types-setuptools",