Apply safe_href scheme allowlist to images in safe_mode

[kayaeminn]

Summary

In safe_mode, process_anchor() validates link URLs against _safe_href
(allowlist: http(s), ftp, mailto, tel), but process_image() only passes
the src through _protect_url(), which escapes <>"' but does not restrict the
URL scheme. So in safe_mode:

>>> import markdown2
>>> markdown2.markdown("![x](javascript:alert(1))", safe_mode="escape")
'<p><img src="javascript:alert(1)" alt="x" /></p>\n'

while the equivalent link is correctly neutralized to href="#". This is an
inconsistency within the sanitizer's own model.

Impact

To be clear, this is not an exploitable XSS: javascript:/data: URIs do not
execute in <img src> on modern browsers. I'm raising it as defense-in-depth /
consistency — the behavior is surprising for a sanitizer and could matter for
non-browser or legacy HTML consumers of the output.

Fix

Apply the same _safe_href scheme allowlist to images that links already use.

Behavior before/after (safe_mode='escape')

Input	Before	After
)	src="javascript:alert(1)"	src=""
	kept	src=""
	kept	kept
	kept	kept
default mode (no safe_mode)	unchanged	unchanged

Legitimate absolute/relative image URLs are unaffected, and default (non-safe) mode
is unchanged.

[nicholasserra]

Sounds sane but has some failing tests that look a bit concerning

[kayaeminn]

Good catch, thanks — you were right to be wary.

The original diff was too broad: it applied the full _safe_href allowlist to <img>, which stripped legitimate data:image/... and quote-escaped http src values that markdown2 intentionally supports. That is what broke data_urls_in_safe_mode, basic_safe_mode, and issue603_xss.

I have narrowed it to keep markdown2's existing image model (quote-escaping via _protect_url) and only neutralize javascript:/vbscript: schemes, which are never valid for an <img src>. All 234 tests pass locally now (cd test && python test.py).

To be transparent: this is defense-in-depth — javascript: does not execute from <img src> in modern browsers — so if you would rather not carry the extra branch, feel free to close. Pushed the narrowed commit either way.

🤖 Addressed by Claude Code

[nicholasserra]

@Crozzers would love your opinion on this one

[Crozzers]

Whilst not a necessary change (discussed in #712), it wouldn't hurt to have.

Couple of comments about the implementation, and it also needs some test cases added

[Crozzers]

Is this line necessary? Seems to be subbing out any whitespace or special ascii chars like NULL, but from my testing, a char like that makes the schema invalid.

I tested these snippets in a browser:

<a href="\x00javascript:alert(1)">abc</a>
<a href="java\x00script:alert(1)">abc</a>
<a href="javascript\x00:alert(1)">abc</a>
<a href="java script:alert(1)">abc</a>

None of them produced an alert when clicked.

This should either have a comment added that explains why this is needed (existing comment just explains what is happening, not why), or should be replaced with if url.lower().startswith(...):

[Crozzers]

For anchors, the URL is normalised to href="#". Should we do that here as well for consistency, or is there a reason to leave it empty?