diff --git a/archivebox/cli/archivebox_add.py b/archivebox/cli/archivebox_add.py index 741d02ac..3f8f5998 100644 --- a/archivebox/cli/archivebox_add.py +++ b/archivebox/cli/archivebox_add.py @@ -142,7 +142,7 @@ def add( persona_name = (persona or "Default").strip() or "Default" plugins = plugins or "" - persona_obj, _ = Persona.objects.get_or_create(name=persona_name) + persona_obj = Persona.get_or_create_named(persona_name) persona_obj.ensure_dirs() effective_persona_config = get_config(persona=persona_obj) diff --git a/archivebox/core/forms.py b/archivebox/core/forms.py index c42106eb..cbd06223 100644 --- a/archivebox/core/forms.py +++ b/archivebox/core/forms.py @@ -6,7 +6,6 @@ from decimal import Decimal, InvalidOperation, ROUND_CEILING from django import forms from archivebox.misc.util import URL_REGEX, find_all_urls, parse_filesize_to_bytes -from taggit.utils import edit_string_for_tags, parse_tags from archivebox.base_models.admin import KeyValueWidget from archivebox.crawls.schedule_util import validate_schedule from archivebox.config.common import get_config, parse_delete_after @@ -30,6 +29,67 @@ DEPTH_CHOICES = ( ) +def _split_strip(value: str, delimiter: str) -> list[str]: + return [part.strip() for part in value.split(delimiter) if part.strip()] + + +def parse_tag_string(value: str | None) -> list[str]: + """Parse the legacy tag-editing format without depending on django-taggit.""" + if not value: + return [] + + if "," not in value and '"' not in value: + return sorted(set(_split_strip(value, " "))) + + tags: list[str] = [] + buffer: list[str] = [] + deferred_chunks: list[str] = [] + saw_unquoted_comma = False + in_quote = False + chars = iter(value) + + try: + while True: + char = next(chars) + if char == '"': + if buffer: + deferred_chunks.append("".join(buffer)) + buffer = [] + in_quote = True + char = next(chars) + while char != '"': + buffer.append(char) + char = next(chars) + tag = "".join(buffer).strip() + if tag: + tags.append(tag) + buffer = [] + in_quote = False + else: + if not saw_unquoted_comma and char == ",": + saw_unquoted_comma = True + buffer.append(char) + except StopIteration: + if buffer: + if in_quote and "," in buffer: + saw_unquoted_comma = True + deferred_chunks.append("".join(buffer)) + + delimiter = "," if saw_unquoted_comma else " " + for chunk in deferred_chunks: + tags.extend(_split_strip(chunk, delimiter)) + + return sorted(set(tags)) + + +def edit_string_for_tag_names(tags) -> str: + names = [] + for tag in tags: + name = tag.name + names.append(f'"{name}"' if "," in name or " " in name else name) + return ", ".join(sorted(names)) + + class AddLinkForm(PluginConfigFormMixin, forms.Form): allow_crawl_execution_config_fields = False @@ -423,7 +483,7 @@ class AddLinkForm(PluginConfigFormMixin, forms.Form): class TagWidget(forms.TextInput): def format_value(self, value): if value is not None and not isinstance(value, str): - value = edit_string_for_tags(value) + value = edit_string_for_tag_names(value) return super().format_value(value) @@ -432,12 +492,7 @@ class TagField(forms.CharField): def clean(self, value): value = super().clean(value) - try: - return parse_tags(value) - except ValueError: - raise forms.ValidationError( - "Please provide a comma-separated list of tags.", - ) + return parse_tag_string(value) def has_changed(self, initial, data): # Always return False if the field is disabled since self.bound_data diff --git a/archivebox/core/models.py b/archivebox/core/models.py index 6fc7cf37..6421beb5 100755 --- a/archivebox/core/models.py +++ b/archivebox/core/models.py @@ -37,6 +37,7 @@ from archivebox.misc.util import ( ts_to_date_str, urlencode, htmlencode, + sanitize_html_text, urldecode, validate_url, ) @@ -94,6 +95,12 @@ class Tag(ModelWithUUID): def __str__(self): return self.name + def save(self, *args, **kwargs): + update_fields = kwargs.get("update_fields") + if update_fields is None or "name" in update_fields: + self.name = sanitize_html_text(self.name).strip() + super().save(*args, **kwargs) + @property def slug(self) -> str: """ASCII-safe slugified form of the tag name (derived, not stored).""" @@ -947,6 +954,8 @@ class Snapshot(ModelWithDeleteAfter, ModelWithOutputDir, ModelWithConfig, ModelW if self._state.adding or update_fields is None or "title" in update_fields: self.title = self._normalize_title_candidate(self.title, snapshot_url=self.url or "") or None + if self._state.adding or update_fields is None or "notes" in update_fields: + self.notes = sanitize_html_text(self.notes) # Migrate filesystem if needed (happens automatically on save) existing_snapshot = self.pk and not self._state.adding @@ -2318,7 +2327,7 @@ class Snapshot(ModelWithDeleteAfter, ModelWithOutputDir, ModelWithConfig, ModelW @staticmethod def _normalize_title_candidate(candidate: str | None, *, snapshot_url: str) -> str: - title = " ".join(line.strip() for line in str(candidate or "").splitlines() if line.strip()).strip() + title = " ".join(line.strip() for line in sanitize_html_text(candidate).splitlines() if line.strip()).strip() if not title: return "" if title.lower() in {"pending...", "no title found", "unable to detect page title"}: diff --git a/archivebox/core/tag_util.py b/archivebox/core/tag_util.py index 04e7ef9b..9fbeea22 100644 --- a/archivebox/core/tag_util.py +++ b/archivebox/core/tag_util.py @@ -12,6 +12,7 @@ from django.http import HttpRequest from django.urls import reverse from archivebox.config.common import get_config +from archivebox.misc.util import sanitize_html_text from archivebox.core.routes_util import build_snapshot_url, build_web_url from archivebox.core.models import Snapshot, SnapshotTag, Tag @@ -33,7 +34,7 @@ TAG_HAS_SNAPSHOTS_CHOICES = ( def normalize_tag_name(name: str) -> str: - return (name or "").strip() + return sanitize_html_text(name).strip() def normalize_tag_sort(sort: str = "created_desc") -> str: diff --git a/archivebox/crawls/models.py b/archivebox/crawls/models.py index 6c5b993e..afa47361 100755 --- a/archivebox/crawls/models.py +++ b/archivebox/crawls/models.py @@ -34,7 +34,7 @@ from archivebox.base_models.models import ( ) from archivebox.workers.models import RETRY_AT_MAX, ModelWithStateMachine, BaseStateMachine from archivebox.crawls.schedule_util import next_run_for_schedule, validate_schedule -from archivebox.misc.util import parse_date, validate_url, validate_url_length +from archivebox.misc.util import parse_date, sanitize_html_text, validate_url, validate_url_length if TYPE_CHECKING: from archivebox.core.models import Snapshot @@ -69,9 +69,14 @@ class CrawlSchedule(ModelWithUUID, ModelWithNotes): return str(reverse_lazy("api-1:get_any", args=[self.id])) def save(self, *args, **kwargs): + update_fields = kwargs.get("update_fields") + if update_fields is None or "label" in update_fields: + self.label = sanitize_html_text(self.label).strip() + if update_fields is None or "notes" in update_fields: + self.notes = sanitize_html_text(self.notes) self.schedule = (self.schedule or "").strip() validate_schedule(self.schedule) - self.label = self.label or (self.template.label if self.template else "") + self.label = self.label or (sanitize_html_text(self.template.label).strip() if self.template else "") super().save(*args, **kwargs) if self.template: self.template.safe_update( @@ -279,6 +284,12 @@ class Crawl(ModelWithDeleteAfter, ModelWithOutputDir, ModelWithConfig, ModelWith def save(self, *args, **kwargs): update_fields = kwargs.get("update_fields") + if update_fields is None or "label" in update_fields: + self.label = sanitize_html_text(self.label).strip() + if update_fields is None or "notes" in update_fields: + self.notes = sanitize_html_text(self.notes) + if update_fields is None or "tags_str" in update_fields: + self.tags_str = ",".join(self.parse_tag_names(self.tags_str or "")) sync_tags = update_fields is None or "tags_str" in update_fields old_crawl = type(self).objects.filter(pk=self.pk).first() if self.pk else None previous_tag_names = set() @@ -375,7 +386,7 @@ class Crawl(ModelWithDeleteAfter, ModelWithOutputDir, ModelWithConfig, ModelWith tag_names: list[str] = [] seen: set[str] = set() for raw_tag in raw_tags: - tag_name = str(raw_tag or "").strip() + tag_name = sanitize_html_text(raw_tag).strip() if not tag_name: continue lowered = tag_name.lower() diff --git a/archivebox/misc/util.py b/archivebox/misc/util.py index f8ff1e68..1179d6b0 100644 --- a/archivebox/misc/util.py +++ b/archivebox/misc/util.py @@ -66,6 +66,22 @@ htmlencode = lambda s: s and escape(s, quote=True) htmldecode = lambda s: s and unescape(s) +def sanitize_html_text(value: Any) -> str: + """Strip all HTML from user-editable text before storing it.""" + if value is None: + return "" + import bleach + + return bleach.clean( + str(value), + tags=[], + attributes={}, + protocols=[], + strip=True, + strip_comments=True, + ) + + def ts_to_date_str(ts: Any) -> str | None: parsed = parse_date(ts) return None if parsed is None else parsed.strftime("%Y-%m-%d %H:%M") diff --git a/archivebox/personas/models.py b/archivebox/personas/models.py index ba36a17f..d8f87696 100644 --- a/archivebox/personas/models.py +++ b/archivebox/personas/models.py @@ -18,7 +18,7 @@ from pathlib import Path from typing import TYPE_CHECKING, Any from collections.abc import Mapping -from django.db import models +from django.db import IntegrityError, models from django.db.models.fields.json import KT from django.conf import settings from django.utils import timezone @@ -228,6 +228,18 @@ class Persona(ModelWithConfig): if fcntl is not None: fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN) + @classmethod + def get_or_create_named(cls, name: str) -> "Persona": + persona_name = (name or "Default").strip() or "Default" + persona = cls.objects.filter(name=persona_name).first() + if persona is not None: + return persona + + try: + return cls.objects.create(name=persona_name) + except IntegrityError: + return cls.objects.get(name=persona_name) + def runtime_root_for_crawl(self, crawl) -> Path: return Path(crawl.output_dir) / ".persona" / self.name @@ -331,8 +343,7 @@ class Persona(ModelWithConfig): @classmethod def get_or_create_default(cls) -> "Persona": """Get or create the Default persona.""" - persona, _ = cls.objects.get_or_create(name="Default") - return persona + return cls.get_or_create_named("Default") @classmethod def cleanup_chrome_all(cls) -> int: diff --git a/archivebox/services/runner.py b/archivebox/services/runner.py index 237cd7a5..5f28046a 100644 --- a/archivebox/services/runner.py +++ b/archivebox/services/runner.py @@ -1712,9 +1712,21 @@ def run_due_snapshot(snapshot, *, lock_seconds: int, interactive_interrupts: boo ) finally: # Targeted plugin rows can complete while the Snapshot remains - # paused. Put retry_at back at MAX so the orchestrator leaves the - # paused lifecycle alone until an explicit resume transition. - snapshot.restore_paused_scheduler_marker() + # paused. Put retry_at back at MAX only after the queued rows are + # gone; if a hook was interrupted before projection, keep the + # paused row due so the next runner can retry that targeted work + # without a user-visible resume transition. + if queued_plugins_for_snapshot(str(snapshot.id)): + now = timezone.now() + type(snapshot).objects.filter( + pk=snapshot.pk, + status=snapshot.StatusChoices.PAUSED, + ).update( + retry_at=now, + modified_at=now, + ) + else: + snapshot.restore_paused_scheduler_marker() return True if snapshot.status == Snapshot.StatusChoices.SEALED: if not Snapshot.claim_for_worker(snapshot, lock_seconds=lock_seconds): @@ -1758,6 +1770,14 @@ def run_due_snapshot(snapshot, *, lock_seconds: int, interactive_interrupts: boo retry_at=None, modified_at=timezone.now(), ) + else: + type(snapshot).objects.filter( + pk=snapshot.pk, + status=snapshot.StatusChoices.SEALED, + ).update( + retry_at=timezone.now(), + modified_at=timezone.now(), + ) return True if maintenance_ran: return True diff --git a/archivebox/tests/test_api_v1_cli_add.py b/archivebox/tests/test_api_v1_cli_add.py index b90a11fb..ed46dc7a 100644 --- a/archivebox/tests/test_api_v1_cli_add.py +++ b/archivebox/tests/test_api_v1_cli_add.py @@ -2,6 +2,8 @@ import pytest import json import time from pathlib import Path +from concurrent.futures import ThreadPoolExecutor +from threading import Event from .conftest import ( api_client_request, @@ -266,6 +268,66 @@ def test_basic_success_case_request(client, tmp_path, api_headers): assert Snapshot.objects.count() == 0 +@pytest.mark.timeout(180) +def test_api_cli_add_concurrent_first_time_default_persona_creation(tmp_path): + """Concurrent live API add requests should share one first-created Default persona.""" + init_archive(tmp_path) + with use_archivebox_db(tmp_path): + from archivebox.personas.models import Persona + + Persona.objects.filter(name="Default").delete() + assert Persona.objects.filter(name="Default").count() == 0 + + port = get_free_port() + env = cli_env(port=port, server=True, USE_COLOR="False", SHOW_PROGRESS="False") + api_token = create_admin_and_token(tmp_path) + submitted_urls = [f"https://example.com/api-cli-add-concurrent-persona-{idx}" for idx in range(4)] + start = Event() + + def post_add(url: str): + start.wait(timeout=10) + return live_api_request( + port, + "post", + "/api/v1/cli/add", + api_token=api_token, + timeout=60, + json={ + "urls": [url], + "depth": 0, + "parser": "url_list", + "plugins": "__archivebox_test_no_plugins__", + "index_only": True, + }, + ) + + try: + start_archivebox_server(tmp_path, env=env, port=port) + with ThreadPoolExecutor(max_workers=len(submitted_urls)) as pool: + futures = [pool.submit(post_add, url) for url in submitted_urls] + start.set() + responses = [future.result(timeout=75) for future in futures] + finally: + stop_server(tmp_path) + + assert [response.status_code for response in responses] == [200] * len(responses), [response.text[:500] for response in responses] + bodies = [response.json() for response in responses] + assert all(body["success"] is True for body in bodies) + assert {body["result"]["queued_urls"][0] for body in bodies} == set(submitted_urls) + + with use_archivebox_db(tmp_path): + from archivebox.personas.models import Persona + + assert Persona.objects.filter(name="Default").count() == 1 + crawls = list(Crawl.objects.order_by("urls").values_list("urls", flat=True)) + assert Snapshot.objects.count() == 0 + + expected_crawl_sources = sorted( + json.dumps({"type": "CrawlSeed", "url": url, "depth": 0}, separators=(",", ":")) for url in submitted_urls + ) + assert crawls == expected_crawl_sources + + @pytest.mark.timeout(360) def test_api_cli_add_import_text_formats_preserve_metadata_and_crawl_inner_urls(tmp_path): """REST API add should accept rich import text and queue real inner URLs with metadata preserved.""" diff --git a/archivebox/tests/test_cli_crawl.py b/archivebox/tests/test_cli_crawl.py index 9bd333a5..850f40ef 100644 --- a/archivebox/tests/test_cli_crawl.py +++ b/archivebox/tests/test_cli_crawl.py @@ -417,28 +417,41 @@ def test_crawl_multiple_urls_creates_multiple_snapshots(initialized_archive): assert "https://iana.org" in urls -def test_crawl_from_file_creates_snapshot(initialized_archive): - """Test that crawl can create snapshots from URL text piped on stdin.""" +def test_crawl_path_argument_is_rejected_but_stdin_file_contents_create_snapshot(initialized_archive): + """Local file paths are not URL args; users must pipe file contents through stdin.""" env = cli_env(disable_extractors=True) - # Write URLs to a file urls_file = initialized_archive / "urls.txt" - urls_file.write_text("https://example.com\n") + urls_file.write_text("https://example.com\nhttps://iana.org\n", encoding="utf-8") - run_archivebox_cmd( + path_result = run_archivebox_cmd( + ["crawl", "create", str(urls_file)], + cwd=initialized_archive, + env=env, + ) + assert path_result.returncode == 1 + assert "No URLs provided" in path_result.stderr + + with use_archivebox_db(initialized_archive): + assert Crawl.objects.count() == 0 + assert Snapshot.objects.count() == 0 + + stdin_result = run_archivebox_cmd( ["crawl", "create"], cwd=initialized_archive, env=env, - stdin=urls_file.read_text(), + stdin=urls_file.read_text(encoding="utf-8"), check=True, ) + assert stdin_result.returncode == 0 run_queued_crawls(initialized_archive, env) with use_archivebox_db(initialized_archive): - snapshot = Snapshot.objects.first() + urls = set(Snapshot.objects.values_list("url", flat=True)) - # Should create at least one snapshot (the source file or the URL) - assert snapshot is not None, "Should create at least one snapshot" + assert "https://example.com" in urls + assert "https://iana.org" in urls + assert str(urls_file) not in urls def test_crawl_persists_input_urls_on_crawl(initialized_archive): diff --git a/archivebox/tests/test_core_forms.py b/archivebox/tests/test_core_forms.py new file mode 100644 index 00000000..f9b1b51f --- /dev/null +++ b/archivebox/tests/test_core_forms.py @@ -0,0 +1,31 @@ +import pytest + +from archivebox.core.forms import TagField, TagWidget +from archivebox.core.models import Tag + + +pytestmark = pytest.mark.django_db + + +def test_tag_field_parses_legacy_tag_input_without_taggit(): + field = TagField() + + assert field.clean("alpha beta alpha") == ["alpha", "beta"] + assert field.clean("alpha, beta, Alpha") == ["Alpha", "alpha", "beta"] + assert field.clean('"alpha beta", gamma') == ["alpha beta", "gamma"] + assert field.clean('"alpha,beta", gamma') == ["alpha,beta", "gamma"] + assert field.clean('alpha "beta gamma"') == ["alpha", "beta gamma"] + assert field.clean('"alpha,beta') == ["alpha", "beta"] + + +def test_tag_widget_formats_real_tag_rows_without_taggit(): + tags = [ + Tag.objects.create(name="plain"), + Tag.objects.create(name="two words"), + Tag.objects.create(name="comma,tag"), + ] + + rendered_value = TagWidget().format_value(tags) + + assert rendered_value == '"comma,tag", plain, "two words"' + assert TagField().clean(rendered_value) == ["comma,tag", "plain", "two words"] diff --git a/etc/package.json b/etc/package.json index 198838cc..398d00ab 100644 --- a/etc/package.json +++ b/etc/package.json @@ -1,6 +1,6 @@ { "name": "archivebox", - "version": "0.9.35rc26", + "version": "0.9.35rc27", "repository": "github:ArchiveBox/ArchiveBox", "license": "MIT", "dependencies": { diff --git a/pyproject.toml b/pyproject.toml index 916c2c02..ce3e4bcc 100755 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "archivebox" -version = "0.9.35rc26" +version = "0.9.35rc27" requires-python = ">=3.13" description = "Self-hosted internet archiving solution." authors = [{name = "Nick Sweeting", email = "pyproject.toml@archivebox.io"}] @@ -47,7 +47,7 @@ dependencies = [ "django-signal-webhooks>=0.3.0", "django-admin-data-views>=0.4.1", "django-object-actions>=4.3.0", - "django-taggit==6.1.0", # TODO: remove this in favor of KVTags only + "bleach>=6.2.0", # for: stripping unsafe HTML from user-editable titles, notes, labels, tags ### State Management "python-statemachine[diagrams]>=2.3.6", ### CLI / Logging @@ -79,9 +79,9 @@ dependencies = [ ### Extractor dependencies (optional binaries detected at runtime via shutil.which) ### Binary/Package Management "abxbus==2.5.10", # EventBus API - "abxpkg>=1.11.208", # for: detecting, versioning, and installing binaries via apt/brew/pip/npm - "abx-plugins>=1.11.210", # shared ArchiveBox plugin package with Chrome/Puppeteer dependency wiring - "abx-dl>=1.11.210", # shared ArchiveBox downloader package with blocking install preflight + "abxpkg>=1.11.209", # for: detecting, versioning, and installing binaries via apt/brew/pip/npm + "abx-plugins>=1.11.211", # shared ArchiveBox plugin package with Chrome/Puppeteer dependency wiring + "abx-dl>=1.11.211", # shared ArchiveBox downloader package with blocking install preflight ### UUID7 backport for Python <3.14 "uuid7>=0.1.0; python_version < '3.14'", # provides the uuid_extensions module on Python 3.13 ]