diff --git a/.github/workflows/publish_artifacts.yaml b/.github/workflows/publish_artifacts.yaml index eeae3aba880..4329d28cc81 100644 --- a/.github/workflows/publish_artifacts.yaml +++ b/.github/workflows/publish_artifacts.yaml @@ -51,5 +51,3 @@ jobs: if: "!github.event.release.prerelease" run: npm publish --provenance --access public working-directory: 'client' - env: - NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} diff --git a/lib/galaxy/config/sample/datatypes_conf.xml.sample b/lib/galaxy/config/sample/datatypes_conf.xml.sample index 235f86b135a..f8a39564ee3 100644 --- a/lib/galaxy/config/sample/datatypes_conf.xml.sample +++ b/lib/galaxy/config/sample/datatypes_conf.xml.sample @@ -347,6 +347,7 @@ + @@ -1190,7 +1191,6 @@ - diff --git a/lib/galaxy/datatypes/binary.py b/lib/galaxy/datatypes/binary.py index a0d5da2da4e..464f68bf44d 100644 --- a/lib/galaxy/datatypes/binary.py +++ b/lib/galaxy/datatypes/binary.py @@ -4899,301 +4899,141 @@ class SpatialData(CompressedZarrZipArchive): file_ext = "spatialdata.zip" - # Minimal metadata for elements - MetadataElement( - name="images_count", - desc="Number of SpatialData image elements", - default=0, - readonly=True, - visible=True, - no_value=0, - ) + def _extract_spatialdata_info(self, filename: str) -> Dict[str, Any]: + """Extract information about SpatialData elements from the zarr archive.""" + info: Dict[str, Any] = { + "images": set(), + "labels": set(), + "shapes": set(), + "points": set(), + "tables": set(), + "table_shapes": {}, + } - MetadataElement( - name="labels_count", - desc="Number of SpatialData label elements", - default=0, - readonly=True, - visible=True, - no_value=0, - ) + try: + with zipfile.ZipFile(filename) as zf: + # Find root zarr directory and detect version + root_zarr = is_v3 = None + for file in zf.namelist(): + if file.endswith(".zarr/zarr.json"): + root_zarr, is_v3 = file.rsplit("/", 1)[0], True + break + elif file.endswith(".zarr/.zattrs"): + root_zarr, is_v3 = file.rsplit("/", 1)[0], False + break + if not root_zarr: + return info - MetadataElement( - name="shapes_count", - desc="Number of SpatialData shape elements", - default=0, - readonly=True, - visible=True, - no_value=0, - ) + # Extract elements: .zarr///... + prefix = root_zarr + "/" + for file in zf.namelist(): + if file.startswith(prefix): + parts = file[len(prefix) :].split("/") + if len(parts) >= 2 and parts[1] and not parts[1].startswith(".") and parts[1] != "zarr.json": + if parts[0] in info and parts[0] != "table_shapes": + info[parts[0]].add(parts[1]) - MetadataElement( - name="points_count", - desc="Number of SpatialData point elements", - default=0, - readonly=True, - visible=True, - no_value=0, - ) + # Extract table shapes (AnnData dimensions) + def get_shape(path): + if path in zf.namelist(): + with zf.open(path) as f: + return json.load(f).get("shape", [None])[0] - MetadataElement( - name="tables", - desc="SpatialData table elements", - default=[], - param=metadata.SelectParameter, - multiple=True, - readonly=True, - visible=True, - ) - - MetadataElement( - name="table_shapes", - desc="SpatialData table shapes (n_obs, n_vars)", - default={}, - param=metadata.DictParameter, - readonly=True, - visible=False, - ) - - MetadataElement( - name="coordinate_systems", - desc="SpatialData coordinate systems", - default=[], - param=metadata.SelectParameter, - multiple=True, - readonly=True, - visible=True, - ) - - MetadataElement( - name="spatialdata_version", - desc="SpatialData software version", - default="", - readonly=True, - visible=True, - no_value="", - ) + for table in info["tables"]: + try: + base = f"{root_zarr}/tables/{table}" + ext = "zarr.json" if is_v3 else ".zarray" + n_obs = get_shape(f"{base}/obs/_index/{ext}") + # V3: if no obs/_index, check obs metadata for index column + if is_v3 and n_obs is None: + obs_meta = f"{base}/obs/zarr.json" + if obs_meta in zf.namelist(): + with zf.open(obs_meta) as f: + idx_col = json.load(f).get("attributes", {}).get("_index") + if idx_col: + n_obs = get_shape(f"{base}/obs/{idx_col}/zarr.json") + n_vars = get_shape(f"{base}/var/_index/{ext}") + if n_obs and n_vars: + info["table_shapes"][table] = (n_obs, n_vars) + except Exception: + pass + except Exception: + pass + return info def set_peek(self, dataset: DatasetProtocol, **kwd) -> None: if not dataset.dataset.purged: - # Try to make a metadata like spatialdata file itself - peek_lines = ["SpatialData object"] - - # Show zarr format if available + info = self._extract_spatialdata_info(dataset.get_file_name()) + lines = ["SpatialData object"] if dataset.metadata.zarr_format: - peek_lines[0] += f" (Zarr Format v{dataset.metadata.zarr_format})" + lines[0] += f" (Zarr Format v{dataset.metadata.zarr_format})" - # Show counts for each element type - if getattr(dataset.metadata, "images_count", 0): - peek_lines.append(f"├── Images ({dataset.metadata.images_count})") + # Filter non-empty element types + element_types = [ + ("images", "Images"), + ("labels", "Labels"), + ("shapes", "Shapes"), + ("points", "Points"), + ("tables", "Tables"), + ] + non_empty = [(key, label) for key, label in element_types if info[key]] - if getattr(dataset.metadata, "labels_count", 0): - peek_lines.append(f"├── Labels ({dataset.metadata.labels_count})") + for idx, (key, label) in enumerate(non_empty): + is_last = idx == len(non_empty) - 1 + elements = sorted(info[key]) + lines.append(f"{'└──' if is_last else '├──'} {label} ({len(elements)})") - if getattr(dataset.metadata, "shapes_count", 0): - peek_lines.append(f"├── Shapes ({dataset.metadata.shapes_count})") + prefix = " " if is_last else "│ " + for i, name in enumerate(elements): + display = f"'{name}'" + if key == "tables" and name in info["table_shapes"]: + display += f": AnnData {info['table_shapes'][name]}" + lines.append(f"{prefix}{'└──' if i == len(elements) - 1 else '├──'} {display}") - if getattr(dataset.metadata, "points_count", 0): - peek_lines.append(f"├── Points ({dataset.metadata.points_count})") - - if dataset.metadata.tables: - peek_lines.append(f"└── Tables ({len(dataset.metadata.tables)})") - for tbl in dataset.metadata.tables: - # Add shape information if available - if dataset.metadata.table_shapes and tbl in dataset.metadata.table_shapes: - shape = dataset.metadata.table_shapes[tbl] - peek_lines.append(f" └── '{tbl}': AnnData {shape}") - else: - peek_lines.append(f" └── '{tbl}'") - - # Show coordinate systems if available - if dataset.metadata.coordinate_systems: - peek_lines.append("") - peek_lines.append("with coordinate systems:") - for cs in dataset.metadata.coordinate_systems: - peek_lines.append(f" • {cs}") - - dataset.peek = "\n".join(peek_lines) + dataset.peek = "\n".join(lines) dataset.blurb = f"SpatialData file ({nice_size(dataset.get_size())})" - if dataset.metadata.spatialdata_version: - dataset.blurb += f"\nVersion: {dataset.metadata.spatialdata_version}" else: dataset.peek = "file does not exist" dataset.blurb = "file purged from disk" - def set_meta(self, dataset: DatasetProtocol, overwrite: bool = True, **kwd) -> None: - super().set_meta(dataset, overwrite=overwrite, **kwd) - try: - with zipfile.ZipFile(dataset.get_file_name()) as zf: - # Initialize element dictionaries to track elements by type - images = set() - labels = set() - shapes = set() - points = set() - tables = set() - coordinate_systems = set() - spatialdata_version = "" - - # Find the root zarr directory - root_zarr = None - for file in zf.namelist(): - if file.endswith(".zarr/.zattrs"): - root_zarr = file.replace("/.zattrs", "") - break - - # Read root attributes for version info - if root_zarr: - root_attrs_path = f"{root_zarr}/.zattrs" - try: - with zf.open(root_attrs_path) as f: - root_attrs = json.load(f) - if "spatialdata_attrs" in root_attrs: - spatialdata_attrs = root_attrs["spatialdata_attrs"] - spatialdata_version = spatialdata_attrs.get("spatialdata_software_version", "") - except Exception: - pass - - # Parse all files to extract elements and coordinate systems - for file in zf.namelist(): - # Extract elements based on directory structure - # Expected structure: .zarr///... - if root_zarr and file.startswith(root_zarr + "/"): - rel_parts = file[len(root_zarr) + 1 :].split("/") - if len(rel_parts) >= 2: - element_type = rel_parts[0] - element_name = rel_parts[1] - - # Skip metadata files and empty names - if element_name and not element_name.startswith("."): - if element_type == "images": - images.add(element_name) - elif element_type == "labels": - labels.add(element_name) - elif element_type == "shapes": - shapes.add(element_name) - elif element_type == "points": - points.add(element_name) - elif element_type == "tables": - tables.add(element_name) - - # Extract coordinate system information from .zattrs files - if file.endswith(".zattrs"): - try: - with zf.open(file) as f: - attrs = json.load(f) - - # Check for coordinate transformations - if "coordinateTransformations" in attrs: - transforms = attrs["coordinateTransformations"] - if isinstance(transforms, list): - for transform in transforms: - if isinstance(transform, dict) and "output" in transform: - output = transform["output"] - if isinstance(output, dict) and "name" in output: - coordinate_systems.add(output["name"]) - elif isinstance(output, str): - coordinate_systems.add(output) - - # Check for multiscales (images/labels) - if "multiscales" in attrs: - multiscales = attrs["multiscales"] - if isinstance(multiscales, list): - for ms in multiscales: - if isinstance(ms, dict) and "coordinateTransformations" in ms: - for ct in ms["coordinateTransformations"]: - if isinstance(ct, dict) and "output" in ct: - output = ct["output"] - if isinstance(output, dict) and "name" in output: - coordinate_systems.add(output["name"]) - elif isinstance(output, str): - coordinate_systems.add(output) - - # Check for spatialdata transform attribute (legacy) - if "transform" in attrs: - transform_dict = attrs["transform"] - if isinstance(transform_dict, dict): - coordinate_systems.update(transform_dict.keys()) - except Exception: - pass - - # Set metadata: counts for most elements, but keep tables and - # coordinate system names and table shapes for compatibility. - dataset.metadata.images_count = len(images) - dataset.metadata.labels_count = len(labels) - dataset.metadata.shapes_count = len(shapes) - dataset.metadata.points_count = len(points) - - # Preserve table names and shapes (as before) - dataset.metadata.tables = sorted(tables) - table_shapes = {} - for table_name in tables: - try: - obs_index_path = f"{root_zarr}/tables/{table_name}/obs/_index/.zarray" - var_index_path = f"{root_zarr}/tables/{table_name}/var/_index/.zarray" - n_obs = None - n_vars = None - if obs_index_path in zf.namelist(): - with zf.open(obs_index_path) as f: - obs_array = json.load(f) - n_obs = obs_array.get("shape", [None])[0] - if var_index_path in zf.namelist(): - with zf.open(var_index_path) as f: - var_array = json.load(f) - n_vars = var_array.get("shape", [None])[0] - if n_obs is not None and n_vars is not None: - table_shapes[table_name] = (n_obs, n_vars) - except Exception: - pass - dataset.metadata.table_shapes = table_shapes - dataset.metadata.coordinate_systems = sorted(coordinate_systems) - dataset.metadata.spatialdata_version = spatialdata_version - except Exception: - pass - def sniff(self, filename: str) -> bool: """ - Check if the file is a valid SpatialData zarr archive. - - SpatialData files are Zarr archives with specific structure containing - a root .zattrs file with spatialdata_attrs metadata and element directories - like images/, labels/, shapes/, points/, or tables/. + Check if file is a SpatialData zarr archive (has spatialdata_attrs in root metadata). >>> from galaxy.datatypes.sniff import get_test_fname >>> fname = get_test_fname('subsampled_visium.spatialdata.zip') >>> SpatialData().sniff(fname) True + >>> fname = get_test_fname('subsampled_visium_v3.spatialdata.zip') + >>> SpatialData().sniff(fname) + True >>> fname = get_test_fname('Images.zarr.zip') >>> SpatialData().sniff(fname) False """ - try: with zipfile.ZipFile(filename) as zf: - # First, check if this is a zarr archive at all - if not super().sniff(filename): + if self._find_zarr_metadata_file(zf) is None: return False - # Look for the root .zattrs file with spatialdata_attrs. - # This can distinguish spatialdata from other zarr archives. + # Check root metadata files (.zattrs or zarr.json) for spatialdata_attrs for file in zf.namelist(): - # Look for .zattrs file at the root of the zarr store - # The zarr store can be at root or one level deeper - parts = file.split("/") - # Root level: .zattrs or one level deep: .zarr/.zattrs - if file == ".zattrs" or (len(parts) == 2 and parts[0].endswith(".zarr") and parts[1] == ".zattrs"): + if len(file.split("/")) <= 2 and file.endswith((".zattrs", "zarr.json")): try: with zf.open(file) as f: - attrs = json.load(f) - # Check for SpatialData-specific metadata - if "spatialdata_attrs" in attrs: + meta = json.load(f) + # Standard format or v3 consolidated + if "spatialdata_attrs" in meta.get("attributes", meta): return True + if "metadata" in meta: + for pm in meta["metadata"].values(): + if isinstance(pm, dict) and "spatialdata_attrs" in pm.get("attributes", {}): + return True except Exception: pass - - return False except Exception: - # Any exception during parsing means it's not a valid spatialdata file - return False + pass + return False @build_sniff_from_prefix diff --git a/lib/galaxy/datatypes/test/subsampled_visium_v3.spatialdata.zip b/lib/galaxy/datatypes/test/subsampled_visium_v3.spatialdata.zip new file mode 100644 index 00000000000..6a06d3bd583 Binary files /dev/null and b/lib/galaxy/datatypes/test/subsampled_visium_v3.spatialdata.zip differ diff --git a/lib/galaxy/jobs/runners/pulsar.py b/lib/galaxy/jobs/runners/pulsar.py index 73133bcf5fd..a7799dd1a2d 100644 --- a/lib/galaxy/jobs/runners/pulsar.py +++ b/lib/galaxy/jobs/runners/pulsar.py @@ -516,8 +516,18 @@ class PulsarJobRunner(AsynchronousJobRunner): compute_environment = PulsarComputeEnvironment(client, job_wrapper, remote_job_config) prepare_kwds["compute_environment"] = compute_environment - if job_wrapper.prepare(**prepare_kwds) is False: + try: + job_prepare_ret = job_wrapper.prepare(**prepare_kwds) + except Exception as e: + # If we fail here the error isn't recoverable and we fail the job. + log.exception("failure preparing job %d", job_wrapper.job_id) + job_state = self._job_state(job_wrapper.get_job(), job_wrapper) + job_state.fail_message = str(e) + self.work_queue.put((self.fail_job, job_state)) + job_prepare_ret = False + if job_prepare_ret is False: return command_line, client, remote_job_config, compute_environment, remote_container + self.__prepare_input_files_locally(job_wrapper) remote_metadata = PulsarJobRunner.__remote_metadata(client) dependency_resolution = PulsarJobRunner.__dependency_resolution(client) diff --git a/lib/galaxy/webapps/galaxy/api/proxy.py b/lib/galaxy/webapps/galaxy/api/proxy.py index ece7b44347c..9d7b5ab9469 100644 --- a/lib/galaxy/webapps/galaxy/api/proxy.py +++ b/lib/galaxy/webapps/galaxy/api/proxy.py @@ -47,6 +47,13 @@ def is_valid_url(url: str) -> bool: return False # Ensure there is exactly one scheme try: parsed = urlparse(url) + + # Check if urlparse had to strip/modify any characters + # If the reconstructed URL differs from the original, the URL contained + # invalid characters (like tabs, newlines) that were silently removed + if parsed.geturl() != url: + return False + return all([parsed.scheme in ALLOWED_SCHEMES, parsed.netloc]) except ValueError: return False @@ -104,6 +111,9 @@ class FastAPIProxy: headers=response.headers, ) + except httpx.InvalidURL as e: + # Catch any URL validation errors that slip through our pre-validation + raise RequestParameterInvalidException(f"Invalid URL format: {e}") except httpx.RequestError as e: raise Exception(f"Request error: {e}") finally: diff --git a/lib/galaxy_test/api/test_proxy.py b/lib/galaxy_test/api/test_proxy.py index 52b187ff940..815284a22a3 100644 --- a/lib/galaxy_test/api/test_proxy.py +++ b/lib/galaxy_test/api/test_proxy.py @@ -5,6 +5,7 @@ from unittest.mock import ( ) import pytest +import requests from galaxy.util.unittest_utils import skip_if_github_down from galaxy_test.base.populators import DatasetPopulator @@ -79,6 +80,37 @@ class TestProxyApi(ApiTestCase): self._assert_status_code_is(response, 400) assert response.json()["err_msg"] == "Invalid URL format." + @pytest.mark.parametrize( + "url_with_nonprintable", + [ + f"\t{URL_TO_TEST}", # Tab at beginning + f"{URL_TO_TEST}\t", # Tab at end + "https://example.com/path\twith\ttab", # Tabs in path + "https://exam\tple.com/path", # Tab in domain + f"\r\n{URL_TO_TEST}", # CRLF at beginning + f"{URL_TO_TEST}\r\n", # CRLF at end + ], + ) + def test_url_with_nonprintable_characters(self, url_with_nonprintable: str): + """Test that URLs with non-printable characters return 400 instead of 500. + + When non-printable characters like tabs or newlines are present in the URL, + they should be caught by validation and return 400, not trigger a 500 error. + + This test manually constructs the URL with unencoded non-printable characters + to reproduce the actual issue that occurs when clients send malformed URLs, + bypassing the test framework's automatic URL encoding. + """ + base_url = self.galaxy_interactor.api_url + full_url = f"{base_url}/proxy?url={url_with_nonprintable}" + headers = self.galaxy_interactor.api_key_header(key=None, admin=False, anon=False, headers=None) + # Filter out None values for requests compatibility + filtered_headers = {k: v for k, v in headers.items() if v is not None} + response = requests.get(full_url, headers=filtered_headers) + + self._assert_status_code_is(response, 400) + assert response.json()["err_msg"] == "Invalid URL format." + def test_proxy_handles_encoding(self): """Test handling of responses with content-encoding and proper header filtering. diff --git a/test/integration/test_pulsar_embedded.py b/test/integration/test_pulsar_embedded.py index 1bc4d8d4043..c9377e2880b 100644 --- a/test/integration/test_pulsar_embedded.py +++ b/test/integration/test_pulsar_embedded.py @@ -2,16 +2,22 @@ import os +from galaxy_test.base.populators import DatasetPopulator from galaxy_test.driver import integration_util SCRIPT_DIRECTORY = os.path.abspath(os.path.dirname(__file__)) EMBEDDED_PULSAR_JOB_CONFIG_FILE = os.path.join(SCRIPT_DIRECTORY, "embedded_pulsar_job_conf.yml") -class EmbeddedPulsarIntegrationInstance(integration_util.IntegrationInstance): +class TestEmbeddedPulsarIntegrationInstance(integration_util.IntegrationTestCase): """Describe a Galaxy test instance with embedded pulsar configured.""" framework_tool_and_types = True + dataset_populator: DatasetPopulator + + def setUp(self) -> None: + super().setUp() + self.dataset_populator = DatasetPopulator(self.galaxy_interactor) @classmethod def handle_galaxy_config_kwds(cls, config): @@ -20,8 +26,20 @@ class EmbeddedPulsarIntegrationInstance(integration_util.IntegrationInstance): config["enable_celery_tasks"] = False config["metadata_strategy"] = "directory" + def test_tool_eval_failure(self): + with self.dataset_populator.test_history() as history_id: + dataset = self.dataset_populator.new_dataset(history_id=history_id, content="ABC", ext="tabular") + self.dataset_populator.run_tool( + "gx_data_column", + inputs={"ref_parameter": {"src": "hda", "id": dataset["id"]}, "parameter": 2}, + history_id=history_id, + ) + failed_dataset = self.dataset_populator.get_history_dataset_details(history_id, wait=True, assert_ok=False) + assert failed_dataset["state"] == "error" + assert failed_dataset["misc_info"] == "Parameter 'parameter': requires a value, but no legal values defined" -instance = integration_util.integration_module_instance(EmbeddedPulsarIntegrationInstance) + +instance = integration_util.integration_module_instance(TestEmbeddedPulsarIntegrationInstance) test_tools = integration_util.integration_tool_runner( [