fix: keep jinja2 blocks intact when splitting tags from imported URLs (#1516)#4259
Open
ebarkhordar wants to merge 1 commit into
Open
fix: keep jinja2 blocks intact when splitting tags from imported URLs (#1516)#4259ebarkhordar wants to merge 1 commit into
ebarkhordar wants to merge 1 commit into
Conversation
…dgtlmoon#1516) The URL list importer separates a line into URL and tags by splitting on the first space. A jinja2 block in a watch URL can legitimately contain spaces, so the split cuts the block in half: importing https://example.com/?date={% now 'utc', '%d' %} tag-a gave url="https://example.com/?date={%" and tags="now 'utc', '%d' %} tag-a". The truncated URL then fails is_safe_valid_url() (it jinja2-renders any URL containing '{%', and the fragment is a syntax error), so add_watch() returns None and the watch is silently reported as skipped. Split on the first space that sits outside any {% %} / {{ }} block instead. Whitespace inside a jinja2 block belongs to the URL; only whitespace outside one can separate the tags. Lines with no jinja2 block behave exactly as before.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
import_url_list.run()separates each import line into URL and tags by splitting on the first space:A jinja2 block in a watch URL can legitimately contain spaces, so the split cuts the block in half. Importing
produces
url="https://example.com/?date={%"andtags="now 'utc', '%d' %} tag-a".The truncated URL is then rejected downstream:
add_watch()callsis_safe_valid_url(), which jinja2-renders any URL containing{%, and?date={%is a template syntax error. Soadd_watch()returnsNoneand the line is counted as skipped rather than imported. Jinja2 in a watch URL is a supported feature (is_safe_valid_url()renders it deliberately, and the quick-add form hastest_jinja2_time_offset_in_url_queryfor it), so the same URL is accepted when added through the form and mangled when imported.The invariant
Whitespace inside a jinja2 block is part of the URL. Only whitespace outside
{% %}/{{ }}can separate the URL from its tags.The fix
split_url_and_tags()masks the jinja2 blocks (keeping the length, so offsets still map back to the line) and splits on the first space outside them. Lines without a jinja2 block take exactly the previous path.I kept this to the jinja2 case only. A URL with a literal, un-encoded space (#1568) stays ambiguous, since the import format uses a space as the URL/tag separator, and disambiguating that is a separate decision.
How I verified it
All of this ran in the
test-changedetectioniocontainer built from this repo's Dockerfile at 5eb2638 (I had to strip the BuildKit cache mounts, no buildx on my box, dependencies are otherwise unchanged):test_import_url_with_jinja2_whitespacefails on unmodified master (Imported 0 new UUIDs, the watch is skipped) and passes with the fix.pytest tests/test_import.py: 7 passed, so the existing space-separated tag tests (https://example.com tag1, other tag) still behave the same.pytest tests/unit/: passed.ruff check . --select E9,F63,F7,F82,INT, pertest-only.yml): passes.What I did not verify:
tests/test_jinja2.pyhas 2 failures (test_jinja2_in_url_query,test_jinja2_time_offset_in_url_query) in my environment, but they fail identically on unmodified master with the same 500 from the preview page, so they are pre-existing here and unrelated to the importer. I did not run the playwright/selenium legs of the CI matrix, which this change does not touch.I did not run
pre-commitas a whole: itsruff --fix/ruff-formathooks reformat these two files wholesale (import reordering,class Importer():toclass Importer:, and so on) and they do the same to unmodified master, so I matched the existing file style and checked the lint that CI actually gates on instead. Happy to run the formatter over the files if you would rather have that.This was written with AI assistance. The root cause, the fix, and every result above are mine and were reproduced and run as described.
Fixes #1516