-
Notifications
You must be signed in to change notification settings - Fork 357
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
UriResolver doesn't handle Windows directory separators #258
Comments
We have some tests for this to help with regression: It seems like we need to incorporate @jojo1981 thoughts? |
@bighappyface I agree the |
@bighappyface The big question here is if we need to fix this. In RFC3986 a path part may only contain forward slashes and no backslashes slashed. Also tested this behavior with: http://php.net/manual/en/function.parse-url.php
NOTE: the extra slash in the 2nd example: "file:///C:/code/my-schema.json" |
IMO the Instead I think there could be a mention about this in the documentation. In addition a notice or an error with a message showing the original URI could be triggered if the parsed scheme is Maybe the slashes could be automatically handled elsewhere, but not in the |
It seems this issue is still open, and as far as I know there's no clear workaround as the issue is spawned in the JsonSchema codebase itself, because it uses |
I also met a similar problem. @Cryszon @jojo1981 |
I think we also ran into this problem at @pronamic and were able to reproduce it via a PHPUnit test / GitHub Action that runs on https://github.com/pronamic/wp-mollie/actions/runs/6097473885/job/16545183100 Perhaps this will help maintainers to address this issue, workflow file: https://github.com/pronamic/wp-mollie/actions/runs/6097473885/workflow
|
@Cryszon If this is still a valid issue we are happy to pursue to see if we can support windows file paths. You inputs would be valued. After checking rfc3986 I can see a
But these are still non standard URIs. I guess in order to completely support Windows we would need active involvement of a person using Windows and some in depth knowledge of this library. |
The issue is not relevant for me personally as I've since moved on to WSL and Docker for my development environment. In a perfect world every system would implement RFCs and standards accurately but the reality is that we must be prepared to make compromises to ensure a smooth user experience. Regardless of whether this issue is fixed or not, I think users should at least be made aware of it by mentioning it somewhere like an error message or in the documentation. Especially if the issue is easily fixable by user code. While I do agree that comprehensive Windows support requires more effort beyond fixing a single directory separator issue, my capacity to assist on that scale is very limited. |
Regex in
UriResolver->parse()
doesn't correctly parse a file path if it contains Windows directory separators. This makes the$basePath
become an empty string and breaks relative$ref
s.The
\
directory separators are inserted whenrealpath()
is called on a Windows platform. The regex doesn't necessarily need to be changed (although it is a bit messy and silently returns an empty path, which made the issue hard to debug). A simple workaround is to replace directory separators when resolving the initial schema file as illustrated below.The text was updated successfully, but these errors were encountered: