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

windows support is incomplete: panic on invalid path #72

Closed
fredbi opened this issue Dec 5, 2023 · 0 comments
Closed

windows support is incomplete: panic on invalid path #72

fredbi opened this issue Dec 5, 2023 · 0 comments

Comments

@fredbi
Copy link
Member

fredbi commented Dec 5, 2023

on windows, go1.21.4:

  • go test -v
    ... panic ... invalid memory address etc..

loading.go: 69

  • analysis:
    • in the windows-specific code path, the error is not checked when parsing the URL
    • so we may have a nil URL, and therefore a panic
fredbi added a commit to fredbi/swag that referenced this issue Dec 5, 2023
On windows, there are quite a few specifics when it comes to resolving
a local file. This complexity stemmed from the desire to support
common, albeit invalid URI constructs for windows.

Overall, the original sin of this function has been to ignore the host
part of an URI, but now we can't really break this behavior.

* fixes go-openapi#72
* refactored the local path pre-processing, so the windows-specifics
  appear more clearly
* fixed a few inconsistencies on windows file URIs and guarded against a panic

* test: refactored tests for the local file strategy, now table-driven and
  more systematic
* doc: added detailed documentation to LoadStrategy()
* ci: re-enacted tests on windows
* ci: muted past linting issues: relinting is deferred to another PR

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
fredbi added a commit to fredbi/swag that referenced this issue Dec 5, 2023
On windows, there are quite a few specifics when it comes to resolving
a local file. This complexity stemmed from the desire to support
common, albeit invalid URI constructs for windows.

Overall, the original sin of this function has been to ignore the host
part of an URI, but now we can't really break this behavior.

* fixes go-openapi#72
* refactored the local path pre-processing, so the windows-specifics
  appear more clearly
* fixed a few inconsistencies on windows file URIs and guarded against a panic

* test: refactored tests for the local file strategy, now table-driven and
  more systematic
* doc: added detailed documentation to LoadStrategy()
* ci: re-enacted tests on windows
* ci: temporarily muted past linting issues: relinting is deferred to another PR

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
fredbi added a commit to fredbi/swag that referenced this issue Dec 5, 2023
On windows, there are quite a few specifics when it comes to resolving
a local file. This complexity stemmed from the desire to support
common, albeit invalid URI constructs for windows.

Overall, the original sin of this function has been to ignore the host
part of an URI, but now we can't really break this behavior.

* fixes go-openapi#72
* refactored the local path pre-processing, so the windows-specifics
  appear more clearly
* fixed a few inconsistencies on windows file URIs and guarded against a panic

* test: refactored tests for the local file strategy, now table-driven and
  more systematic
* doc: added detailed documentation to LoadStrategy()
* ci: re-enacted tests on windows
* ci: temporarily muted past linting issues: relinting is deferred to another PR

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
@fredbi fredbi closed this as completed in 4de0676 Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant