diff --git a/.github/dependabot.yml b/.github/dependabot.yml index aec207e4..eb8ba18c 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,7 +1,66 @@ version: 2 updates: -- package-ecosystem: nuget - directory: "/" - schedule: - interval: daily - open-pull-requests-limit: 10 + # JavaScript dependencies + - package-ecosystem: "npm" + directory: "/js" + schedule: + interval: "weekly" + day: "monday" + open-pull-requests-limit: 10 + labels: + - "dependencies" + - "javascript" + commit-message: + prefix: "chore(deps)" + + # Python dependencies + - package-ecosystem: "pip" + directory: "/python" + schedule: + interval: "weekly" + day: "monday" + open-pull-requests-limit: 10 + labels: + - "dependencies" + - "python" + commit-message: + prefix: "chore(deps)" + + # Rust dependencies + - package-ecosystem: "cargo" + directory: "/rust" + schedule: + interval: "weekly" + day: "monday" + open-pull-requests-limit: 10 + labels: + - "dependencies" + - "rust" + commit-message: + prefix: "chore(deps)" + + # C# dependencies + - package-ecosystem: "nuget" + directory: "/csharp" + schedule: + interval: "weekly" + day: "monday" + open-pull-requests-limit: 10 + labels: + - "dependencies" + - "csharp" + commit-message: + prefix: "chore(deps)" + + # GitHub Actions + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + day: "monday" + open-pull-requests-limit: 5 + labels: + - "dependencies" + - "github-actions" + commit-message: + prefix: "chore(deps)" diff --git a/.github/workflows/csharp.yml b/.github/workflows/csharp.yml index 9ec26904..4b902643 100644 --- a/.github/workflows/csharp.yml +++ b/.github/workflows/csharp.yml @@ -22,10 +22,11 @@ defaults: jobs: findChangedCsFiles: runs-on: ubuntu-latest + timeout-minutes: 10 outputs: isCsFilesChanged: ${{ steps.setIsCsFilesChangedOutput.outputs.isCsFilesChanged }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: fetch-depth: 0 - name: Get changed files using defaults @@ -50,12 +51,13 @@ jobs: needs: [findChangedCsFiles] if: ${{ needs.findChangedCsFiles.outputs.isCsFilesChanged == 'true' }} runs-on: ubuntu-latest + timeout-minutes: 15 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup .NET SDK - uses: actions/setup-dotnet@v3 + uses: actions/setup-dotnet@v4 with: dotnet-version: '8.0.x' # Specify your desired .NET version - name: Restore Dependencies @@ -68,14 +70,15 @@ jobs: pushToNuget: runs-on: ubuntu-latest + timeout-minutes: 15 needs: [test] if: ${{ needs.findChangedCsFiles.outputs.isCsFilesChanged == 'true' && github.event_name == 'push' && github.ref == 'refs/heads/main' }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup .NET SDK - uses: actions/setup-dotnet@v3 + uses: actions/setup-dotnet@v4 with: dotnet-version: '8.0.x' # Ensure this matches your project's target - name: Check if version already published to NuGet.org diff --git a/.github/workflows/js.yml b/.github/workflows/js.yml index 1c453a6b..99354f03 100644 --- a/.github/workflows/js.yml +++ b/.github/workflows/js.yml @@ -22,10 +22,11 @@ defaults: jobs: findChangedJsFiles: runs-on: ubuntu-latest + timeout-minutes: 10 outputs: isJsFilesChanged: ${{ steps.setIsJsFilesChangedOutput.outputs.isJsFilesChanged }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: fetch-depth: 0 - name: Get changed files using defaults @@ -50,8 +51,9 @@ jobs: needs: [findChangedJsFiles] if: ${{ needs.findChangedJsFiles.outputs.isJsFilesChanged == 'true' }} runs-on: ubuntu-latest + timeout-minutes: 15 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Bun @@ -69,8 +71,9 @@ jobs: needs: [test, findChangedJsFiles] if: ${{ needs.findChangedJsFiles.outputs.isJsFilesChanged == 'true' && github.event_name == 'push' && github.ref == 'refs/heads/main' }} runs-on: ubuntu-latest + timeout-minutes: 15 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Bun @@ -106,10 +109,11 @@ jobs: publishRelease: runs-on: ubuntu-latest + timeout-minutes: 10 needs: [publishToNpm] if: ${{ needs.findChangedJsFiles.outputs.isJsFilesChanged == 'true' && needs.publishToNpm.result == 'success' && github.event_name == 'push' && github.ref == 'refs/heads/main' }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Bun @@ -138,9 +142,9 @@ jobs: run: | PACKAGE_VERSION=$(node -p "require('./package.json').version") PACKAGE_NAME=$(node -p "require('./package.json').name") - - # Create release - gh release create "${PACKAGE_VERSION}_js" \ + + # Create release with consistent tag format: js_version + gh release create "js_${PACKAGE_VERSION}" \ --title "[JS] $PACKAGE_VERSION" \ --notes "https://www.npmjs.com/package/$PACKAGE_NAME" env: diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index bfd75c48..89cfd62f 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -22,10 +22,11 @@ defaults: jobs: findChangedPythonFiles: runs-on: ubuntu-latest + timeout-minutes: 10 outputs: isPythonFilesChanged: ${{ steps.setIsPythonFilesChangedOutput.outputs.isPythonFilesChanged }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: fetch-depth: 0 - name: Get changed files @@ -50,12 +51,13 @@ jobs: needs: [findChangedPythonFiles] if: ${{ needs.findChangedPythonFiles.outputs.isPythonFilesChanged == 'true' }} runs-on: ubuntu-latest + timeout-minutes: 15 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Set up Python 3.13 - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: '3.13' - name: Install dependencies @@ -71,12 +73,13 @@ jobs: needs: [test, findChangedPythonFiles] if: ${{ needs.findChangedPythonFiles.outputs.isPythonFilesChanged == 'true' && github.event_name == 'push' && github.ref == 'refs/heads/main' }} runs-on: ubuntu-latest + timeout-minutes: 15 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Set up Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: '3.13' - name: Install dependencies @@ -110,14 +113,15 @@ jobs: publishRelease: runs-on: ubuntu-latest + timeout-minutes: 10 needs: [publishToPyPI] if: ${{ needs.findChangedPythonFiles.outputs.isPythonFilesChanged == 'true' && needs.publishToPyPI.result == 'success' && github.event_name == 'push' && github.ref == 'refs/heads/main' }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Set up Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: '3.13' - name: Check if GitHub release already exists @@ -143,8 +147,8 @@ jobs: PACKAGE_VERSION=$(grep "^version" pyproject.toml | sed 's/version = "\(.*\)"/\1/') PACKAGE_NAME=$(grep "^name" pyproject.toml | sed 's/name = "\(.*\)"/\1/') - # Create release - gh release create "${PACKAGE_VERSION}_python" \ + # Create release with consistent tag format: python_version + gh release create "python_${PACKAGE_VERSION}" \ --title "[Python] $PACKAGE_VERSION" \ --notes "https://pypi.org/project/$PACKAGE_NAME/" env: diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 00b71221..705d14a8 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -22,10 +22,11 @@ defaults: jobs: findChangedRustFiles: runs-on: ubuntu-latest + timeout-minutes: 10 outputs: isRustFilesChanged: ${{ steps.setIsRustFilesChangedOutput.outputs.isRustFilesChanged }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: fetch-depth: 0 - name: Get changed files using defaults @@ -50,16 +51,13 @@ jobs: needs: [findChangedRustFiles] if: ${{ needs.findChangedRustFiles.outputs.isRustFilesChanged == 'true' }} runs-on: ubuntu-latest + timeout-minutes: 20 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Rust - uses: actions-rs/toolchain@v1 - with: - toolchain: stable - profile: minimal - override: true + uses: dtolnay/rust-toolchain@stable - name: Cache Cargo registry uses: actions/cache@v3 with: @@ -84,16 +82,13 @@ jobs: needs: [test, findChangedRustFiles] if: ${{ needs.findChangedRustFiles.outputs.isRustFilesChanged == 'true' && github.event_name == 'push' && github.ref == 'refs/heads/main' }} runs-on: ubuntu-latest + timeout-minutes: 20 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Rust - uses: actions-rs/toolchain@v1 - with: - toolchain: stable - profile: minimal - override: true + uses: dtolnay/rust-toolchain@stable - name: Cache Cargo registry uses: actions/cache@v3 with: @@ -152,25 +147,22 @@ jobs: publishRelease: runs-on: ubuntu-latest + timeout-minutes: 10 needs: [publishToCratesIO] if: ${{ needs.findChangedRustFiles.outputs.isRustFilesChanged == 'true' && needs.publishToCratesIO.result == 'success' && github.event_name == 'push' && github.ref == 'refs/heads/main' }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Rust - uses: actions-rs/toolchain@v1 - with: - toolchain: stable - profile: minimal - override: true + uses: dtolnay/rust-toolchain@stable - name: Check if GitHub release already exists id: release-check run: | PACKAGE_VERSION=$(grep '^version = ' Cargo.toml | head -1 | sed 's/version = "\(.*\)"/\1/') TAG_NAME="rust_$PACKAGE_VERSION" echo "Checking if release $TAG_NAME already exists" - + # Check if release exists if gh release view "$TAG_NAME" >/dev/null 2>&1; then echo "Release $TAG_NAME already exists" @@ -186,9 +178,9 @@ jobs: run: | PACKAGE_NAME=$(grep '^name = ' Cargo.toml | head -1 | sed 's/name = "\(.*\)"/\1/') PACKAGE_VERSION=$(grep '^version = ' Cargo.toml | head -1 | sed 's/version = "\(.*\)"/\1/') - - # Create release - gh release create "${PACKAGE_VERSION}_rust" \ + + # Create release with consistent tag format: rust_version + gh release create "rust_${PACKAGE_VERSION}" \ --title "[Rust] $PACKAGE_VERSION" \ --notes "https://crates.io/crates/$PACKAGE_NAME" env: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 00000000..fc970e6b --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,58 @@ +repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.5.0 + hooks: + - id: trailing-whitespace + - id: end-of-file-fixer + - id: check-yaml + - id: check-added-large-files + args: ['--maxkb=1000'] + - id: check-json + - id: check-toml + - id: check-merge-conflict + - id: mixed-line-ending + + # JavaScript/TypeScript + - repo: https://github.com/pre-commit/mirrors-eslint + rev: v8.56.0 + hooks: + - id: eslint + files: \.(js|ts)$ + args: ['--fix'] + additional_dependencies: + - eslint + + # Python + - repo: https://github.com/psf/black + rev: 24.1.1 + hooks: + - id: black + language_version: python3 + files: \.py$ + + - repo: https://github.com/PyCQA/isort + rev: 5.13.2 + hooks: + - id: isort + files: \.py$ + + - repo: https://github.com/PyCQA/flake8 + rev: 7.0.0 + hooks: + - id: flake8 + files: \.py$ + args: ['--max-line-length=120'] + + # Rust + - repo: https://github.com/doublify/pre-commit-rust + rev: v1.0 + hooks: + - id: fmt + - id: cargo-check + + # Markdown + - repo: https://github.com/igorshubovych/markdownlint-cli + rev: v0.39.0 + hooks: + - id: markdownlint + args: ['--fix'] diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 00000000..8077e636 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,68 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [Unreleased] + +### Added +- Comprehensive code quality improvements across all language implementations +- Input validation and size limits for all parsers +- Proper error types in Rust implementation +- JSDoc documentation for JavaScript/TypeScript implementation +- Security policy (SECURITY.md) +- Pre-commit hooks configuration +- Dependabot for automated dependency updates + +### Changed +- Python minimum version relaxed from 3.13 to 3.9+ +- Updated GitHub Actions to latest versions (v4/v5) +- Replaced deprecated `actions-rs` with `dtolnay/rust-toolchain` +- Standardized release tag format across all workflows (language_version) +- Improved null/undefined checking in JavaScript +- Improved None checking in Python (explicit `is not None`) +- Enhanced C# `Equals()` method to properly compare anonymous links +- Reduced excessive cloning in Rust implementation +- Improved quote escaping to handle edge cases in JavaScript + +### Fixed +- JavaScript Parser: Fixed null/undefined checks to use explicit comparison +- JavaScript Parser: Preserved error stack traces in error handling +- JavaScript Link: Added input validation for constructor parameters +- JavaScript Link: Fixed defensive programming in `simplify()` and `equals()` methods +- JavaScript Link: Improved quote escaping for references containing both single and double quotes +- Python Parser: Fixed None checks to use `is not None` instead of truthiness +- Python Parser: More specific exception handling (no longer catches all exceptions) +- Python Parser: Added input size validation +- Rust lib: Fixed `unwrap()` usage with proper error handling using `if let` +- Rust lib: Added proper `ParseError` type instead of returning String errors +- C# Link: Fixed `Equals()` method to properly handle two anonymous links (both with null IDs) +- CI/CD: Standardized release tag format across all workflows +- CI/CD: Added timeout-minutes to all workflow jobs +- CI/CD: Updated all deprecated GitHub Actions + +## [0.11.2] - 2024-XX-XX + +### Added +- Multi-language support (JavaScript, Python, Rust, C#) +- Comprehensive test suites for all implementations +- CI/CD workflows for automated testing and publishing +- Support for indented syntax +- Support for multiline quoted strings +- Support for mixed indentation modes + +### Fixed +- Various parser improvements and bug fixes + +## [0.11.0] - 2024-XX-XX + +### Added +- Initial multi-language release +- Core parser functionality +- Basic link notation support + +[Unreleased]: https://github.com/link-foundation/links-notation/compare/v0.11.2...HEAD +[0.11.2]: https://github.com/link-foundation/links-notation/releases/tag/v0.11.2 +[0.11.0]: https://github.com/link-foundation/links-notation/releases/tag/v0.11.0 diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 00000000..348b4456 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,49 @@ +# Security Policy + +## Supported Versions + +We release patches for security vulnerabilities for the following versions: + +| Version | Supported | +| ------- | ------------------ | +| 0.11.x | :white_check_mark: | +| < 0.11 | :x: | + +## Reporting a Vulnerability + +We take the security of Links Notation seriously. If you believe you have found a security vulnerability in any of our implementations (JavaScript, Python, Rust, or C#), please report it to us as described below. + +**Please do not report security vulnerabilities through public GitHub issues.** + +Instead, please email us at: +**drakonard@gmail.com** + +You should receive a response within 48 hours. If for some reason you do not, please follow up to ensure we received your original message. + +Please include the following information in your report: + +* Type of issue (e.g. buffer overflow, injection, cross-site scripting, etc.) +* Full paths of source file(s) related to the manifestation of the issue +* The location of the affected source code (tag/branch/commit or direct URL) +* Any special configuration required to reproduce the issue +* Step-by-step instructions to reproduce the issue +* Proof-of-concept or exploit code (if possible) +* Impact of the issue, including how an attacker might exploit the issue + +This information will help us triage your report more quickly. + +## Preferred Languages + +We prefer all communications to be in English. + +## Security Update Process + +1. The security report is received and assigned to a handler +2. The problem is confirmed and a list of affected versions is determined +3. Code is audited to find any similar problems +4. Fixes are prepared for all supported versions +5. Fixes are released as quickly as possible + +## Comments on this Policy + +If you have suggestions on how this process could be improved, please submit a pull request. diff --git a/csharp/Link.Foundation.Links.Notation/Link.cs b/csharp/Link.Foundation.Links.Notation/Link.cs index c5eaeb76..9e68094b 100644 --- a/csharp/Link.Foundation.Links.Notation/Link.cs +++ b/csharp/Link.Foundation.Links.Notation/Link.cs @@ -231,11 +231,29 @@ public static string EscapeReference(string? reference) /// /// Indicates whether the current link is equal to another link. + /// Two anonymous links (both with null Ids) are considered equal if their values are equal. /// /// The link to compare with this link. /// True if the current link is equal to the other parameter; otherwise, false. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public bool Equals(Link other) => Id != null && other.Id != null && EqualityComparerInstance.Equals(Id, other.Id) && (Values ?? Array.Empty>()).EqualTo(other.Values ?? Array.Empty>()); + public bool Equals(Link other) + { + // Both have null IDs - compare only values + if (Id == null && other.Id == null) + { + return (Values ?? Array.Empty>()).EqualTo(other.Values ?? Array.Empty>()); + } + + // Only one has null ID - not equal + if (Id == null || other.Id == null) + { + return false; + } + + // Both have IDs - compare IDs and values + return EqualityComparerInstance.Equals(Id, other.Id) && + (Values ?? Array.Empty>()).EqualTo(other.Values ?? Array.Empty>()); + } /// /// Determines whether two link instances are equal. diff --git a/js/src/Link.js b/js/src/Link.js index 172913c0..560d213e 100644 --- a/js/src/Link.js +++ b/js/src/Link.js @@ -1,43 +1,92 @@ export class Link { + /** + * Create a new Link + * @param {string|null} id - Optional identifier for the link + * @param {Link[]|null} values - Optional array of nested links + * @throws {TypeError} If values is not an array or null + */ constructor(id = null, values = null) { this.id = id; - this.values = values || []; + + // Validate that values is an array if provided + if (values !== null && values !== undefined) { + if (!Array.isArray(values)) { + throw new TypeError('values must be an array or null'); + } + this.values = values; + } else { + this.values = []; + } } + /** + * Convert link to string representation + * @returns {string} String representation of the link + */ toString() { return this.format(false); } + /** + * Get formatted string of all values + * @returns {string} Space-separated string of values + */ getValuesString() { - return (!this.values || this.values.length === 0) ? + return (!this.values || this.values.length === 0) ? '' : this.values.map(v => Link.getValueString(v)).join(' '); } + /** + * Simplify the link structure by unwrapping single-value containers + * @returns {Link} Simplified link + */ simplify() { if (!this.values || this.values.length === 0) { return this; } else if (this.values.length === 1) { return this.values[0]; } else { - const newValues = this.values.map(v => v.simplify()); + const newValues = this.values.map(v => { + // Check if value has simplify method (defensive programming) + return v && typeof v.simplify === 'function' ? v.simplify() : v; + }); return new Link(this.id, newValues); } } + /** + * Combine this link with another link + * @param {Link} other - The link to combine with + * @returns {Link} Combined link + */ combine(other) { return new Link(null, [this, other]); } + /** + * Get string representation of a value + * @param {Link} value - The value to stringify + * @returns {string} String representation + */ static getValueString(value) { - return value.toLinkOrIdString(); + // Defensive check for method existence + return value && typeof value.toLinkOrIdString === 'function' ? value.toLinkOrIdString() : String(value); } + /** + * Escape a reference string by adding quotes if necessary + * @param {string} reference - The reference to escape + * @returns {string} Escaped reference + */ static escapeReference(reference) { if (!reference || reference.trim() === '') { return ''; } - - const needsSingleQuotes = + + const hasSingleQuote = reference.includes("'"); + const hasDoubleQuote = reference.includes('"'); + + const needsQuoting = reference.includes(':') || reference.includes('(') || reference.includes(')') || @@ -45,17 +94,38 @@ export class Link { reference.includes('\t') || reference.includes('\n') || reference.includes('\r') || - reference.includes('"'); - - if (needsSingleQuotes) { + hasDoubleQuote || + hasSingleQuote; + + // Handle edge case: reference contains both single and double quotes + if (hasSingleQuote && hasDoubleQuote) { + // Escape single quotes and wrap in single quotes + return `'${reference.replace(/'/g, "\\'")}'`; + } + + // Prefer single quotes if double quotes are present + if (hasDoubleQuote) { return `'${reference}'`; - } else if (reference.includes("'")) { + } + + // Use double quotes if single quotes are present + if (hasSingleQuote) { return `"${reference}"`; - } else { - return reference; } + + // Use single quotes for special characters + if (needsQuoting) { + return `'${reference}'`; + } + + // No quoting needed + return reference; } + /** + * Convert to string using either just ID or full format + * @returns {string} String representation + */ toLinkOrIdString() { if (!this.values || this.values.length === 0) { return this.id === null ? '' : Link.escapeReference(this.id); @@ -63,20 +133,44 @@ export class Link { return this.toString(); } + /** + * Check equality with another Link + * @param {*} other - Object to compare with + * @returns {boolean} True if links are equal + */ equals(other) { if (!(other instanceof Link)) return false; if (this.id !== other.id) return false; - if (this.values.length !== other.values.length) return false; - - for (let i = 0; i < this.values.length; i++) { - if (!this.values[i].equals(other.values[i])) { - return false; + + // Handle null/undefined values arrays + const thisValues = this.values || []; + const otherValues = other.values || []; + + if (thisValues.length !== otherValues.length) return false; + + for (let i = 0; i < thisValues.length; i++) { + // Defensive check for equals method + if (thisValues[i] && typeof thisValues[i].equals === 'function') { + if (!thisValues[i].equals(otherValues[i])) { + return false; + } + } else { + // Fallback to reference equality + if (thisValues[i] !== otherValues[i]) { + return false; + } } } return true; } - + + /** + * Format the link as a string + * @param {boolean} lessParentheses - If true, omit parentheses where safe + * @param {boolean} isCompoundValue - If true, this is a value in a compound link + * @returns {string} Formatted string + */ format(lessParentheses = false, isCompoundValue = false) { // Empty link if (this.id === null && (!this.values || this.values.length === 0)) { @@ -121,29 +215,39 @@ export class Link { return lessParentheses && !this.needsParentheses(this.id) ? withColon : `(${withColon})`; } + /** + * Format a value within this link + * @param {Link} value - The value to format + * @returns {string} Formatted value string + */ formatValue(value) { - if (!value.format) { - return Link.escapeReference(value.id || ''); + if (!value || !value.format) { + return Link.escapeReference((value && value.id) || ''); } - + // Check if we're in a compound link that was created from path combinations // This is indicated by having a parent context passed through const isCompoundFromPaths = this._isFromPathCombination === true; - + // For compound links from paths, format values with parentheses if (isCompoundFromPaths) { return value.format(false, true); } - + // Simple link with just an ID - don't wrap in parentheses when used as a value if (!value.values || value.values.length === 0) { return Link.escapeReference(value.id); } - + // Complex value with its own structure - format it normally with parentheses return value.format(false, false); } - + + /** + * Check if a string needs to be wrapped in parentheses + * @param {string} str - The string to check + * @returns {boolean} True if parentheses are needed + */ needsParentheses(str) { return str && (str.includes(' ') || str.includes(':') || str.includes('(') || str.includes(')')); } diff --git a/js/src/Parser.js b/js/src/Parser.js index c5c56b57..65e9f13e 100644 --- a/js/src/Parser.js +++ b/js/src/Parser.js @@ -2,24 +2,52 @@ import { Link } from './Link.js'; import * as parserModule from './parser-generated.js'; export class Parser { - constructor() { + /** + * Create a new Parser instance + * @param {Object} options - Parser options + * @param {number} options.maxInputSize - Maximum input size in bytes (default: 10MB) + * @param {number} options.maxDepth - Maximum nesting depth (default: 1000) + */ + constructor(options = {}) { + this.maxInputSize = options.maxInputSize || 10 * 1024 * 1024; // 10MB default + this.maxDepth = options.maxDepth || 1000; } + /** + * Parse Lino notation text into Link objects + * @param {string} input - The Lino notation text to parse + * @returns {Link[]} Array of parsed Link objects + * @throws {Error} If parsing fails + */ parse(input) { + // Validate input + if (typeof input !== 'string') { + throw new TypeError('Input must be a string'); + } + + if (input.length > this.maxInputSize) { + throw new Error(`Input size exceeds maximum allowed size of ${this.maxInputSize} bytes`); + } + try { const rawResult = parserModule.parse(input); return this.transformResult(rawResult); } catch (error) { - throw new Error(`Parse error: ${error.message}`); + // Preserve original error information + const parseError = new Error(`Parse error: ${error.message}`); + parseError.cause = error; + parseError.location = error.location; + throw parseError; } } transformResult(rawResult) { const links = []; const items = Array.isArray(rawResult) ? rawResult : [rawResult]; - + for (const item of items) { - if (item) { + // Use explicit null/undefined check + if (item !== null && item !== undefined) { this.collectLinks(item, [], links); } } @@ -27,7 +55,8 @@ export class Parser { } collectLinks(item, parentPath, result) { - if (!item) return; + // Use explicit null/undefined check + if (item === null || item === undefined) return; // For items with children (indented structure) if (item.children && item.children.length > 0) { @@ -105,9 +134,15 @@ export class Parser { } + /** + * Transform a parsed item into a Link object + * @param {*} item - The item to transform + * @returns {Link|null} The transformed Link or null + */ transformLink(item) { - if (!item) return null; - + // Use explicit null/undefined check + if (item === null || item === undefined) return null; + if (item instanceof Link) { return item; } @@ -124,7 +159,7 @@ export class Parser { link.values = item.values.map(v => this.transformLink(v)); return link; } - + // Default case return new Link(item.id || null, []); } diff --git a/python/links_notation/parser.py b/python/links_notation/parser.py index 4e387047..7e5468b2 100644 --- a/python/links_notation/parser.py +++ b/python/links_notation/parser.py @@ -20,13 +20,21 @@ class Parser: Handles both inline and indented syntax for defining links. """ - def __init__(self): - """Initialize the parser.""" + def __init__(self, max_input_size: int = 10 * 1024 * 1024, max_depth: int = 1000): + """ + Initialize the parser. + + Args: + max_input_size: Maximum input size in bytes (default: 10MB) + max_depth: Maximum nesting depth (default: 1000) + """ self.indentation_stack = [0] self.pos = 0 self.text = "" self.lines = [] self.base_indentation = None + self.max_input_size = max_input_size + self.max_depth = max_depth def parse(self, input_text: str) -> List[Link]: """ @@ -40,7 +48,17 @@ def parse(self, input_text: str) -> List[Link]: Raises: ParseError: If parsing fails + TypeError: If input is not a string + ValueError: If input exceeds maximum size """ + # Validate input type + if not isinstance(input_text, str): + raise TypeError("Input must be a string") + + # Validate input size + if len(input_text) > self.max_input_size: + raise ValueError(f"Input size exceeds maximum allowed size of {self.max_input_size} bytes") + try: if not input_text or not input_text.strip(): return [] @@ -53,7 +71,14 @@ def parse(self, input_text: str) -> List[Link]: raw_result = self._parse_document() return self._transform_result(raw_result) - except Exception as e: + except (TypeError, ValueError): + # Re-raise validation errors without wrapping + raise + except ParseError: + # Re-raise ParseError without wrapping + raise + except (KeyError, IndexError, AttributeError) as e: + # Catch specific parsing-related exceptions raise ParseError(f"Parse error: {str(e)}") from e def _parse_document(self) -> List[Dict]: @@ -254,7 +279,8 @@ def _transform_result(self, raw_result: List[Dict]) -> List[Link]: links = [] for item in raw_result: - if item: + # Use explicit None check + if item is not None: self._collect_links(item, [], links) return links @@ -266,7 +292,8 @@ def _collect_links(self, item: Dict, parent_path: List[Link], result: List[Link] Handles both inline and indented syntax, flattening the hierarchy appropriately. """ - if not item: + # Use explicit None check + if item is None: return children = item.get('children', []) diff --git a/python/pyproject.toml b/python/pyproject.toml index 54653315..117eddb6 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -17,11 +17,15 @@ classifiers = [ "Intended Audience :: Developers", "License :: Public Domain", "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3.9", + "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", "Programming Language :: Python :: 3.13", "Topic :: Software Development :: Libraries :: Python Modules", "Topic :: Text Processing", ] -requires-python = ">=3.13" +requires-python = ">=3.9" dependencies = [] [project.optional-dependencies] diff --git a/rust/src/lib.rs b/rust/src/lib.rs index bd9b49c6..1b63ec28 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -1,6 +1,30 @@ pub mod parser; use std::fmt; +use std::error::Error as StdError; + +/// Error type for Lino parsing +#[derive(Debug)] +pub enum ParseError { + /// Input string is empty or contains only whitespace + EmptyInput, + /// Syntax error during parsing + SyntaxError(String), + /// Internal parser error + InternalError(String), +} + +impl fmt::Display for ParseError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ParseError::EmptyInput => write!(f, "Empty input"), + ParseError::SyntaxError(msg) => write!(f, "Syntax error: {}", msg), + ParseError::InternalError(msg) => write!(f, "Internal error: {}", msg), + } + } +} + +impl StdError for ParseError {} #[derive(Debug, Clone, PartialEq)] pub enum LiNo { @@ -83,14 +107,25 @@ fn flatten_links(links: Vec) -> Vec> { result } -fn flatten_link_recursive(link: &parser::Link, parent: Option>, result: &mut Vec>) { +fn flatten_link_recursive(link: &parser::Link, parent: Option<&LiNo>, result: &mut Vec>) { // Special case: If this is an indented ID (with colon) with children, // the children should become the values of the link (indented ID syntax) if link.is_indented_id && link.id.is_some() && link.values.is_empty() && !link.children.is_empty() { let child_values: Vec> = link.children.iter().map(|child| { // For indented children, if they have single values, extract them - if child.values.len() == 1 && child.values[0].id.is_some() && child.values[0].values.is_empty() && child.values[0].children.is_empty() { - LiNo::Ref(child.values[0].id.clone().unwrap()) + if child.values.len() == 1 && child.values[0].values.is_empty() && child.values[0].children.is_empty() { + // Use if let to safely extract the ID instead of unwrap() + if let Some(ref id) = child.values[0].id { + LiNo::Ref(id.clone()) + } else { + // If no ID, create an empty link + parser::Link { + id: child.id.clone(), + values: child.values.clone(), + children: vec![], + is_indented_id: false, + }.into() + } } else { parser::Link { id: child.id.clone(), @@ -101,26 +136,26 @@ fn flatten_link_recursive(link: &parser::Link, parent: Option>, res } }).collect(); - let current = LiNo::Link { - id: link.id.clone(), - values: child_values + let current = LiNo::Link { + id: link.id.clone(), + values: child_values }; - + let combined = if let Some(parent) = parent { // Wrap parent in parentheses if it's a reference let wrapped_parent = match parent { - LiNo::Ref(ref_id) => LiNo::Link { id: None, values: vec![LiNo::Ref(ref_id)] }, - link => link + LiNo::Ref(ref_id) => LiNo::Link { id: None, values: vec![LiNo::Ref(ref_id.clone())] }, + link => link.clone() }; - - LiNo::Link { - id: None, + + LiNo::Link { + id: None, values: vec![wrapped_parent, current] } } else { current }; - + result.push(combined); return; // Don't process children again } @@ -148,38 +183,38 @@ fn flatten_link_recursive(link: &parser::Link, parent: Option>, res let combined = if let Some(parent) = parent { // Wrap parent in parentheses if it's a reference let wrapped_parent = match parent { - LiNo::Ref(ref_id) => LiNo::Link { id: None, values: vec![LiNo::Ref(ref_id)] }, - link => link + LiNo::Ref(ref_id) => LiNo::Link { id: None, values: vec![LiNo::Ref(ref_id.clone())] }, + link => link.clone() }; - + // Wrap current in parentheses if it's a reference - let wrapped_current = match current.clone() { - LiNo::Ref(ref_id) => LiNo::Link { id: None, values: vec![LiNo::Ref(ref_id)] }, - link => link + let wrapped_current = match ¤t { + LiNo::Ref(ref_id) => LiNo::Link { id: None, values: vec![LiNo::Ref(ref_id.clone())] }, + link => link.clone() }; - - LiNo::Link { - id: None, + + LiNo::Link { + id: None, values: vec![wrapped_parent, wrapped_current] } } else { current.clone() }; - + result.push(combined.clone()); - + // Process children for child in &link.children { - flatten_link_recursive(child, Some(combined.clone()), result); + flatten_link_recursive(child, Some(&combined), result); } } -pub fn parse_lino(document: &str) -> Result, String> { +pub fn parse_lino(document: &str) -> Result, ParseError> { // Handle empty or whitespace-only input by returning empty result if document.trim().is_empty() { return Ok(LiNo::Link { id: None, values: vec![] }); } - + match parser::parse_document(document) { Ok((_, links)) => { if links.is_empty() { @@ -190,12 +225,12 @@ pub fn parse_lino(document: &str) -> Result, String> { Ok(LiNo::Link { id: None, values: flattened }) } } - Err(e) => Err(format!("Parse error: {:?}", e)) + Err(e) => Err(ParseError::SyntaxError(format!("{:?}", e))) } } // New function that matches C# and JS API - returns collection of links -pub fn parse_lino_to_links(document: &str) -> Result>, String> { +pub fn parse_lino_to_links(document: &str) -> Result>, ParseError> { // Handle empty or whitespace-only input by returning empty collection if document.trim().is_empty() { return Ok(vec![]); @@ -211,7 +246,7 @@ pub fn parse_lino_to_links(document: &str) -> Result>, String> Ok(flattened) } } - Err(e) => Err(format!("Parse error: {:?}", e)) + Err(e) => Err(ParseError::SyntaxError(format!("{:?}", e))) } }