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

Proposal: Introduce file:// prefix for explicit file loading in multipart data #1073

Closed
h6ah4i opened this issue Nov 16, 2024 · 3 comments
Closed
Labels
enhancement New feature or request

Comments

@h6ah4i
Copy link
Contributor

h6ah4i commented Nov 16, 2024

Hi, I noticed that the current implementation of multipart/form-data attempts to read a file if the specified form value is a string.

https://github.com/k1LoW/runn/blob/10bfec2fc03d0bd49d9c0bda14f08fffcdbca39e/http.go#L256:L271

This implementation caused some errors for me, and I believe the behavior can be improved by introducing a file:// prefix.

  1. runn does not raise an error if the specified file does not exist or if the provided path string is incorrect.
  2. absolute file paths are not accepted.

Introducing a file:// prefix could help address these issues and make the behavior more explicit and reliable.

@k1LoW
Copy link
Owner

k1LoW commented Nov 16, 2024

@h6ah4i Thank you for your proposal!

Indeed, it is better to introduce file:// prefix to check files strictly.

However, I would like to keep the current behavior without prefix at the side.

This is partly for backward compatibility, but also because I want to match the behavior of other path resolution.

So, how about making it so that files are only checked strictly when a file:// prefix is added?

Then we can introduce file:// prefix in other path resolution implementations in the future.

@h6ah4i
Copy link
Contributor Author

h6ah4i commented Nov 16, 2024

@k1LoW Preserving backward compatibility is an important concern as well. I agree with the direction you're taking! 👍

@k1LoW
Copy link
Owner

k1LoW commented Nov 28, 2024

Released as v0.122.0 👍

@k1LoW k1LoW closed this as completed Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants