Fix Tag-Only Protocols validation error#99
Merged
Conversation
… values. Introduced list-based iteration for better validation and error handling, ensuring correct processing of 'unix' paths and flag protocols. Added checks for missing values and invalid arguments for protocols without associated values.
Implemented unit tests to ensure tag-only protocols reject invalid value syntax (both /tag/value and /tag=value) and validate correct combinations. Enhanced error message clarity for invalid inputs.
- Encode protocol code for tag-only protocols before validation check This fixes the bug where tag-only protocols (e.g., http, tls) were not being encoded when followed by valid protocol names, causing them to be dropped from the parsed multiaddr. - Improve error messages to exclude invalid values Error messages for tag-only protocols now only show the protocol name and not the invalid value, making them clearer and more helpful. Fixes the test failures where valid multiaddrs with tag-only protocols followed by other protocols were incorrectly parsed.
Add explicit type annotation to help pyrefly type checker resolve type inference issues when idx is modified in nested loops.
Change idx += N to idx = idx + N in unix path handling to help pyrefly 0.41.0 type checker resolve type inference issues in nested loops.
Contributor
|
@yashksaini-coder Thank you for this PR. please resolve the linting issues and failing test. |
- Simplify unix path handling: use list comprehension instead of nested loop to avoid type inference issues with idx mutation - Simplify flag-protocol lookahead: use direct idx assignment instead of idx = next_idx - 1; idx += 1 pattern - Fix Makefile clean-build: use rm -rf instead of rm -f for .egg files to handle both files and directories These changes resolve the pyrefly 'int is not assignable to int' error caused by complex index mutation patterns confusing the type analyzer.
Contributor
Author
|
@acul71 Hi, seems you have made all failing ci checks passed. I really appreciate this. Anything else remaining on this PR, which we need to resolve, or can we do final review + merge |
Contributor
newsfragment and documentation example |
- Introduced a new section in the examples documentation for tag-only protocols, detailing their usage, validation, error handling, and integration with multiaddr addresses. - Added a Python example file demonstrating these concepts. This enhances the documentation by providing clear guidance on working with tag-only protocols.
- Updated the examples in `tag_only_examples.py` to improve address validation by using a placeholder variable for `Multiaddr` instantiation. - Enhanced error handling messages to provide clearer feedback on invalid addresses. These changes aim to streamline the usage examples and improve the overall clarity of error reporting for tag-only protocols.
Contributor
Author
Done please have a review. |
Contributor
|
@yashksaini-coder : Ready for merge. Appreciate your support and efforts. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: #98
This pull request improves the parsing logic for multiaddresses, specifically enhancing validation for tag-only protocols (protocols that do not accept values). The changes ensure that these protocols correctly reject invalid value assignments, provide clearer error messages, and add comprehensive tests for these cases.
Parsing logic improvements:
multiaddr.pyto use an index-based approach, allowing lookahead and better validation of protocol arguments.codec=None) do not accept values via either/tag/valueor/tag=valuesyntax, raising a clearStringParseErrorif violated. [1] [2]unixprotocol and other value-accepting protocols. [1] [2]Testing enhancements:
test_multiaddr.pyto verify that tag-only protocols reject values with both/tag/valueand/tag=valuesyntaxes, and that valid combinations are accepted.After implementing the changes, we get proper accept values
@seetadev @Nkovaturient @lla-dane Please review this pull request changes.