Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] Add support for handling remote file URIs. #629

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Carpe-Wang
Copy link

Summary

This PR adds support for handling remote file URIs within the project. It addresses the issue where the system could not correctly process URIs that point to remote files.

Changes

  • Implemented functionality to download remote files specified by file: URIs.
  • Added unit tests to verify the correctness of this new feature.

Issue Link

Fixes #616

@Carpe-Wang Carpe-Wang changed the title Add support for handling remote file URIs. [fix] Add support for handling remote file URIs. Aug 18, 2024
@bioball
Copy link
Contributor

bioball commented Aug 19, 2024

Hey Carpe, thanks for the PR!

I don't know if this is the right solution to the problem. As far as I can tell, there is no agreed upon protocol when opening files with a remote hostname.

  • Windows machines will use SMB to open the file
  • Safari on macOS will strip the hostname entirely and try to open the file from the local machine
  • Java's URL#openConnection() will use FTP
  • I can't find any examples of using HTTP to fetch the file

Without a protocol here, I think some valid choices are:

  1. Explicitly don't support, and instead throw an error
  2. Support on Windows only
  3. Follow Java's lead and use FTP

I'm kind of leaning towards option 1 because it's the least amount of commitment and allows us to address this if the need arises later.

Curious what @holzensp and @stackoverflow think

@Carpe-Wang
Copy link
Author

@bioball Hi bioball, I got your idea, If both of them also suggest modifying it to directly throw an exception, I will submit a new commit to implement this.

@stackoverflow
Copy link
Contributor

Agree that throwing an error seems like the best solution, as there's no consensus on how to handle host names in files.

@holzensp
Copy link
Contributor

I would have expected NFS on Linux/Solaris/FreeBSD/etc. Agreed; randomly guessing protocols to start hammering seems risky, at best. Exception would be the way to go.

@Carpe-Wang
Copy link
Author

@bioball hi bioball, I change to throw the exception directly. Can you help me to ensure satisfying the requirement?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PklBugException when given a file: URI with a host
4 participants