jenkins-bot has submitted this change. (
https://gerrit.wikimedia.org/r/c/pywikibot/core/+/1361008?usp=email )
Change subject: [bugfix] Only take a source with a URL scheme as URL for uploads
......................................................................
[bugfix] Only take a source with a URL scheme as URL for uploads
FilePage.upload(), UploadRobot and the upload script took every source
containing '://' as a URL. A local path can contain these characters
too, e.g. the Windows path 'C://Windows/image.png' or a POSIX path
like '/home/user/a://b/image.png'. Such a file was passed as
source_url to the API or downloaded with UploadRobot instead of being
uploaded from disk.
Add tools.is_url() which only accepts values starting with a URL scheme
of at least two characters, so a Windows drive letter is not taken as
a scheme, and use it wherever the upload code decided between URL and
local file.
Bug: T164450
Change-Id: Idb0bd02b0aaf68c5a61d7e24770e93d0335f3a00
---
M ROADMAP.rst
M pywikibot/page/_filepage.py
M pywikibot/specialbots/_upload.py
M pywikibot/tools/__init__.py
M scripts/CHANGELOG.rst
M scripts/upload.py
M tests/file_tests.py
M tests/tools_tests.py
M tests/uploadbot_tests.py
9 files changed, 145 insertions(+), 7 deletions(-)
Approvals:
Xqt: Looks good to me, approved
jenkins-bot: Verified
diff --git a/ROADMAP.rst b/ROADMAP.rst
index 9b82d2f..4a5173f 100644
--- a/ROADMAP.rst
+++ b/ROADMAP.rst
@@ -7,7 +7,10 @@
**Bugfixes**
-* (No changes yet)
+* Only take a source starting with a URL scheme as URL in
:meth:`FilePage.upload()
+ <pywikibot.FilePage.upload>` and :class:`specialbots.UploadRobot`; a local
path
+ containing ``://`` like ``C://image.png`` is uploaded as file.
:func:`tools.is_url`
+ was added for this check. (:phab:`T164450`)
**Code cleanups**
diff --git a/pywikibot/page/_filepage.py b/pywikibot/page/_filepage.py
index 94e2155..577dafd 100644
--- a/pywikibot/page/_filepage.py
+++ b/pywikibot/page/_filepage.py
@@ -22,7 +22,7 @@
from pywikibot.comms import http
from pywikibot.exceptions import NoPageError
from pywikibot.page._page import Page
-from pywikibot.tools import compute_file_hash
+from pywikibot.tools import compute_file_hash, is_url
if TYPE_CHECKING:
@@ -298,6 +298,10 @@
.. version-changed:: 11.8
The *comment* keyword was renamed to *summary*.
+ .. version-changed:: next
+ *source* is only taken as URL if it starts with a URL scheme;
+ a local path like ``C://image.png`` is no longer uploaded as
+ URL.
:keyword summary: Edit summary; if this is not provided, then
filepage.text will be used. An empty summary is not
@@ -335,7 +339,7 @@
otherwise.
"""
filename = url = None
- if '://' in source:
+ if is_url(source):
url = source
else:
filename = source
diff --git a/pywikibot/specialbots/_upload.py b/pywikibot/specialbots/_upload.py
index 370ff3d..cbdddf7 100644
--- a/pywikibot/specialbots/_upload.py
+++ b/pywikibot/specialbots/_upload.py
@@ -24,6 +24,7 @@
from pywikibot.bot import BaseBot, QuitKeyboardInterrupt
from pywikibot.comms import http
from pywikibot.exceptions import APIError, FatalServerError, NoPageError
+from pywikibot.tools import is_url
if TYPE_CHECKING:
@@ -283,7 +284,7 @@
"""
# Isolate the pure name
filename = file_url
- if '://' in filename:
+ if is_url(filename):
# extract the path portion of the URL
filename = urlparse(filename).path
filename = os.path.basename(filename)
@@ -453,7 +454,7 @@
temp_filename: str | None = None
try:
while True:
- if '://' in file_url \
+ if is_url(file_url) \
and (not site.has_right('upload_by_url') or download):
try:
temp_filename = self.read_file_content(file_url)
@@ -475,7 +476,7 @@
f'Upload error: Local file uploads are disabled '
f'on {site}.')
elif error.code == 'copyuploadbaddomain' and not download \
- and '://' in file_url:
+ and is_url(file_url):
pywikibot.error(error)
pywikibot.info('Downloading the file and retry...')
download = True
diff --git a/pywikibot/tools/__init__.py b/pywikibot/tools/__init__.py
index ee8cf87..2071b36 100644
--- a/pywikibot/tools/__init__.py
+++ b/pywikibot/tools/__init__.py
@@ -21,6 +21,7 @@
from functools import total_ordering, wraps
from pathlib import Path
from typing import IO, TYPE_CHECKING, Any, Literal
+from urllib.parse import urlsplit
from warnings import catch_warnings, showwarning, warn
import packaging.version
@@ -78,6 +79,7 @@
'as_filename',
'is_ip_address',
'is_ip_network',
+ 'is_url',
'has_module',
'classproperty',
'suppress_warnings',
@@ -140,6 +142,44 @@
return False
+def is_url(value: str) -> bool:
+ """Check if a value starts with a URL scheme like ``https://``.
+
+ The scheme is found with :func:`urllib.parse.urlsplit`. A single
+ letter is not taken as a scheme because it is a Windows drive
+ letter like in ``C://Windows/image.png``, and the scheme must be
+ followed by ``://``, so a file name like ``abc:def.png`` or a path
+ which only contains ``://`` somewhere else is not a URL either.
+
+ >>> is_url('https://upload.wikimedia.org/image.png')
+ True
+ >>> is_url('C://Windows/image.png')
+ False
+ >>> is_url('/home/user/a://b/image.png')
+ False
+ >>> is_url('abc:def.png')
+ False
+
+ .. version-added:: next
+
+ :param value: Value to check
+ """
+ value = value.lstrip()
+ try:
+ scheme = urlsplit(value).scheme
+ except ValueError:
+ return False
+
+ # Python 3.10 urlsplit() does not check that a scheme starts with an
+ # ASCII letter like Python 3.11+ does.
+ # TODO: remove this check with Pywikibot 13
+ if PYTHON_VERSION < (3, 11) and scheme and not (
+ scheme[0].isascii() and scheme[0].isalpha()):
+ return False
+
+ return len(scheme) > 1 and value[len(scheme):].startswith('://')
+
+
def has_module(module: str, version: str | None = None) -> bool:
"""Check if a module can be imported.
diff --git a/scripts/CHANGELOG.rst b/scripts/CHANGELOG.rst
index f599b4c..7da8e1f 100644
--- a/scripts/CHANGELOG.rst
+++ b/scripts/CHANGELOG.rst
@@ -1,6 +1,14 @@
Scripts Changelog
=================
+12.0.0
+------
+
+upload
+^^^^^^
+
+* Accept a local path containing ``://`` if it is not a URL (:phab:`T164450`)
+
11.10.0
-------
diff --git a/scripts/upload.py b/scripts/upload.py
index 7524843..724548d 100755
--- a/scripts/upload.py
+++ b/scripts/upload.py
@@ -81,6 +81,7 @@
import pywikibot
from pywikibot.bot import suggest_help
from pywikibot.specialbots import UploadRobot
+from pywikibot.tools import is_url
CHUNK_SIZE_REGEX = re.compile(
@@ -193,7 +194,7 @@
description = filepath.read_text(
encoding='utf-8').replace('\r\n', '\n')
- while not ('://' in url or os.path.exists(url)):
+ while not (is_url(url) or os.path.exists(url)):
if not url:
error = 'No input filename given.'
else:
diff --git a/tests/file_tests.py b/tests/file_tests.py
index d7b0b16..58b35a4 100755
--- a/tests/file_tests.py
+++ b/tests/file_tests.py
@@ -12,6 +12,7 @@
import unittest
from contextlib import suppress
from itertools import chain
+from unittest import mock
import pywikibot
from pywikibot import pagegenerators
@@ -286,6 +287,35 @@
self.assertEqual(self.image.latest_file_info.thumbheight, 133)
+class TestFilePageUpload(TestCase):
+
+ """Dry tests for FilePage.upload source handling."""
+
+ net = False
+
+ def _upload(self, source: str) -> dict:
+ """Call FilePage.upload with a mocked site and return kwargs."""
+ page = mock.Mock()
+ pywikibot.FilePage.upload(page, source, summary='test')
+ page.site.upload.assert_called_once()
+ return page.site.upload.call_args.kwargs
+
+ def test_url(self) -> None:
+ """Test that a URL is passed as source_url."""
+ url = 'https://upload.wikimedia.org/image.png'
+ kwargs = self._upload(url)
+ self.assertEqual(kwargs['source_url'], url)
+ self.assertIsNone(kwargs['source_filename'])
+
+ def test_path_with_scheme_separator(self) -> None:
+ """Test that local paths containing ``://`` are passed as file."""
+ for path in ('C://Windows/image.png', '/home/user/a://b/image.png'):
+ with self.subTest(path=path):
+ kwargs = self._upload(path)
+ self.assertEqual(kwargs['source_filename'], path)
+ self.assertIsNone(kwargs['source_url'])
+
+
class TestFilePageDownload(TestCase):
"""Test download of FilePage to local file."""
diff --git a/tests/tools_tests.py b/tests/tools_tests.py
index ec4d46b..9efdc64 100755
--- a/tests/tools_tests.py
+++ b/tests/tools_tests.py
@@ -26,6 +26,7 @@
has_module,
is_ip_address,
is_ip_network,
+ is_url,
suppress_warnings,
)
from pywikibot.tools.collections import GeneratorWrapper
@@ -1095,6 +1096,33 @@
self.assertFalse(is_ip_network(ip_range, strict=False))
+class TestIsUrl(TestCase):
+
+ """Unit test class for is_url."""
+
+ net = False
+
+ def test_urls(self) -> None:
+ """Check values starting with a URL scheme."""
+ for value in ('https://upload.wikimedia.org/image.png',
+ 'http://example.org/image.png',
+ 'HTTPS://example.org/image.png',
+ 'ftp://example.org/image.png',
+ 'svn+ssh://example.org/image.png'):
+ with self.subTest(value=value):
+ self.assertTrue(is_url(value))
+
+ def test_paths(self) -> None:
+ """Check local paths, also those containing ``://``."""
+ for value in ('', 'image.png', '/home/user/image.png',
+ 'C://Windows/image.png', r'C:\Windows\image.png',
+ '/home/user/a://b/image.png', './a://image.png',
+ '://image.png', '1http://example.org/image.png',
+ 'abc:def.png', 'File:image.png'):
+ with self.subTest(value=value):
+ self.assertFalse(is_url(value))
+
+
class TestHasModule(TestCase):
"""Unit test class for has_module."""
diff --git a/tests/uploadbot_tests.py b/tests/uploadbot_tests.py
index 9f9e09d..29aed5b 100755
--- a/tests/uploadbot_tests.py
+++ b/tests/uploadbot_tests.py
@@ -192,6 +192,29 @@
self.assertEqual(result, 'mahveo.png')
self.assertTrue(path.exists())
+ def test_local_path_with_scheme_separator(self) -> None:
+ """Test that a local path containing ``://`` is not downloaded."""
+ for path in ('C://Windows/mahveo.png', '/home/user/a://mahveo.png'):
+ with self.subTest(path=path):
+ site = mock.Mock(file_extensions=['png'])
+ site.has_right.return_value = False
+ imagepage = mock.Mock()
+ imagepage.exists.return_value = False
+ imagepage.file_is_shared.return_value = False
+ imagepage.upload.return_value = True
+ bot = UploadRobot(url=[path], target_site=site,
+ description='test', keep_filename=True,
+ verify_description=False, always=False)
+ with mock.patch.object(bot, 'read_file_content') as fetch, \
+ mock.patch('pywikibot.FilePage',
+ return_value=imagepage):
+ result = bot.upload_file(path)
+
+ self.assertEqual(result, 'mahveo.png')
+ fetch.assert_not_called()
+ imagepage.upload.assert_called_once()
+ self.assertEqual(imagepage.upload.call_args.args, (path, ))
+
def test_downloaded_file_cleanup_after_retry(self) -> None:
"""Test cleanup after falling back from URL to file upload."""
file_url = 'https://yo.wikipedia.org/mahveo.png'
--
To view, visit
https://gerrit.wikimedia.org/r/c/pywikibot/core/+/1361008?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.wikimedia.org/r/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: pywikibot/core
Gerrit-Branch: master
Gerrit-Change-Id: Idb0bd02b0aaf68c5a61d7e24770e93d0335f3a00
Gerrit-Change-Number: 1361008
Gerrit-PatchSet: 8
Gerrit-Owner: Sanskardubey <[email protected]>
Gerrit-Reviewer: Xqt <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
Pywikibot-commits mailing list -- [email protected]
To unsubscribe send an email to [email protected]
