Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/citrine/__version__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "3.5.1"
__version__ = "3.5.2"
9 changes: 7 additions & 2 deletions src/citrine/resources/file_link.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
from pathlib import Path
from tempfile import TemporaryDirectory
from typing import Optional, Tuple, Union, Dict, Iterable, Sequence
from urllib.parse import urlparse, unquote_plus
from urllib.parse import urlparse
from urllib.request import url2pathname
from uuid import UUID

from citrine._rest.collection import Collection
Expand Down Expand Up @@ -647,7 +648,11 @@ def read(self, *, file_link: Union[str, UUID, FileLink]) -> bytes:
file_link = self._resolve_file_link(file_link)

if self._is_local_url(file_link.url): # Read the local file
path = Path(unquote_plus(urlparse(file_link.url).path))
parsed_url = urlparse(file_link.url)
if parsed_url.netloc not in {'', '.', 'localhost'}:
raise ValueError("Non-local UNCs (e.g., Windows network paths) are not supported.")

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new restriction, is it not?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formally it's a new restriction, but we would have broken on this case before because we were not considering the netloc value.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a side note, I would have fixed this rather than raising an exception, except without a Windows box to experiment on, I don't trust that my implementation will be reliable. Stackoverflow

# Space should have been encoded as %20, but just in case it was a +
path = Path(url2pathname(parsed_url.path.replace('+', '%20')))
return path.read_bytes()

if self._is_external_url(file_link.url): # Pull it from where ever it lives
Expand Down
3 changes: 3 additions & 0 deletions tests/resources/test_file_link.py
Original file line number Diff line number Diff line change
Expand Up @@ -837,3 +837,6 @@ def test_exceptions(collection: FileCollection, session):
)
with pytest.raises(NotFound):
collection.get(uid="name")

with pytest.raises(ValueError, match="Windows"):
collection.read(file_link=FileLink('File', 'file://remote/network/file.txt'))