fix: windows absolute path #140
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes Windows absolute path handling in SQLite DSN parsing (issue #137). The URL parser was incorrectly handling Windows drive letters like C:/, which resulted in invalid paths. The fix adds pattern matching to detect and correctly parse Windows-style absolute paths.
Key changes:
- Added regex-based detection for Windows drive letter paths (`/^/[A-Za-z]://)
- Updated documentation with the correct Windows path format
- Added comprehensive test coverage for path parsing scenarios including Windows, Unix, relative paths, and in-memory databases
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/connectors/sqlite/index.ts | Added Windows drive letter path detection and handling logic |
| src/connectors/tests/sqlite.integration.test.ts | Added comprehensive DSN path parsing test suite covering Windows, Unix, relative paths, and in-memory databases |
| docs/config/server-options.mdx | Corrected Windows path example to use proper DSN format with three slashes |
| } else if (url.pathname.match(/^\/[A-Za-z]:\//)) { | ||
| // Windows absolute path: sqlite:///C:/path/to/db.sqlite |
There was a problem hiding this comment.
The regex pattern only matches forward slashes after the drive letter, but Windows paths can use backslashes. The pattern should be /^\/[A-Za-z]:[\/\\]/ to handle both C:/ and C:\ formats. Although the URL parser typically normalizes to forward slashes, this would make the code more robust.
| } else if (url.pathname.match(/^\/[A-Za-z]:\//)) { | |
| // Windows absolute path: sqlite:///C:/path/to/db.sqlite | |
| } else if (url.pathname.match(/^\/[A-Za-z]:[\/\\]/)) { | |
| // Windows absolute path: sqlite:///C:/path/to/db.sqlite or sqlite:///C:\path\to\db.sqlite |
| if (url.pathname.startsWith("//")) { | ||
| // Absolute path: sqlite:///path/to/db.sqlite | ||
| // Unix absolute path: sqlite:///path/to/db.sqlite | ||
| dbPath = url.pathname.substring(2); // Remove leading // | ||
| } else if (url.pathname.match(/^\/[A-Za-z]:\//)) { |
There was a problem hiding this comment.
The condition order creates a logic error: Windows UNC paths like //server/share would incorrectly match the Unix absolute path check first. The Windows drive letter check should come before the // check, or add an additional condition to exclude Windows drive patterns from the Unix path logic (e.g., url.pathname.startsWith('//') && !url.pathname.match(/^\/\/[A-Za-z]:\//)).
Fix #137