Add validation for service and operation names#43
Closed
VegetarianOrc wants to merge 4 commits intomainfrom
Closed
Add validation for service and operation names#43VegetarianOrc wants to merge 4 commits intomainfrom
VegetarianOrc wants to merge 4 commits intomainfrom
Conversation
Collaborator
VegetarianOrc
commented
Jan 14, 2026
VegetarianOrc
commented
- Add _validate_nexus_name() helper that raises ValueError for invalid names
- Validate names are non-empty and not whitespace-only
- Validate names can be URL-encoded (rejects invalid Unicode like lone surrogates)
- Include invalid value in error messages for easier debugging
- Add comprehensive tests for all validation cases
Per the spec: "The service name and operation name MUST not be empty and may contain any arbitrary character sequence as long as they're encoded into the URL." - Add _validate_nexus_name() helper that raises ValueError for invalid names - Validate names are non-empty and not whitespace-only - Validate names can be URL-encoded (rejects invalid Unicode like lone surrogates) - Include invalid value in error messages for easier debugging - Add comprehensive tests for all validation cases Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Explicitly specify UTF-8 encoding when opening test files to avoid Windows defaulting to cp1252 encoding. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…tion - Add exception chaining (`from e`) in _validate_nexus_name() to preserve original exception context for debugging - Use _validate_nexus_name() in OperationDefinition.from_operation() for consistent validation and error messages across all code paths - Update test to match new error message format Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Did we check if any other SDK does this validation? |
Collaborator
Author
I did forget to check before doing this, my bad! Go & Java don't have this in so the responsibility lies in any HTTP implementation to fail requests sent for operations that can't be URL encoded. That seems like a reasonable tradeoff to me. Happy to close this if you agree. |
|
Yeah that's fine with me |
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
2 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.