Fix ItemWithPath throwing on null/empty/whitespace path instead of generating malformed URLs#3072
Fix ItemWithPath throwing on null/empty/whitespace path instead of generating malformed URLs#3072Copilot wants to merge 6 commits into
Conversation
Co-authored-by: lramosvea <77297467+lramosvea@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR makes ItemWithPath fail fast for invalid path inputs to prevent malformed Microsoft Graph URLs when null, empty, or whitespace strings are provided.
Changes:
- Added argument validation to both
ItemWithPathoverloads (RootRequestBuilderandDriveItemItemRequestBuilder). - Added unit tests covering
null, empty, and whitespace path inputs for both overloads.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tests/Microsoft.Graph.DotnetCore.Test/Requests/Extensions/DriveItemRequestBuilderExtensionsTests.cs | Adds test coverage for invalid path inputs on both ItemWithPath overloads. |
| src/Microsoft.Graph/Extensions/DriveItemRequestBuilderExtensions.cs | Adds null + empty/whitespace validation and normalizes missing leading / for path. |
| public static CustomDriveItemItemRequestBuilder ItemWithPath(this Microsoft.Graph.Drives.Item.Root.RootRequestBuilder rootRequestBuilder, string path) | ||
| { | ||
| if (!string.IsNullOrEmpty(path)) | ||
| if (path is null) | ||
| throw new ArgumentNullException(nameof(path)); | ||
| if (string.IsNullOrWhiteSpace(path)) | ||
| throw new ArgumentException("path cannot be empty or whitespace.", nameof(path)); | ||
|
|
||
| if (!path.StartsWith("/")) | ||
| { | ||
| if (!path.StartsWith("/")) | ||
| { | ||
| path = string.Format("/{0}", path); | ||
| } | ||
| path = string.Format("/{0}", path); | ||
| } |
There was a problem hiding this comment.
Done in commit 23f4c9e. The duplicated validation and leading-slash normalization logic has been extracted into a private NormalizePathOrThrow(string path) helper used by both overloads. This also consolidates the StringComparison.Ordinal fix. Tests now assert ParamName == "path" on all thrown exceptions.
| public void ItemByPath_ThrowsArgumentNullException_WhenPathIsNull_Root() | ||
| { | ||
| var graphServiceClient = new GraphServiceClient(new MockAuthenticationProvider().Object); | ||
| Assert.Throws<ArgumentNullException>(() => graphServiceClient.Drives["driveId"].Root.ItemWithPath(null)); |
There was a problem hiding this comment.
Already applied in commit 23f4c9e. The test now captures the exception and asserts ex.ParamName == "path".
| public void ItemByPath_ThrowsArgumentException_WhenPathIsEmptyOrWhitespace_Root(string path) | ||
| { | ||
| var graphServiceClient = new GraphServiceClient(new MockAuthenticationProvider().Object); | ||
| Assert.Throws<ArgumentException>(() => graphServiceClient.Drives["driveId"].Root.ItemWithPath(path)); |
There was a problem hiding this comment.
Already applied in commit 23f4c9e. The test now captures the exception and asserts ex.ParamName == "path".
| public void ItemByPath_ThrowsArgumentNullException_WhenPathIsNull_DriveItem() | ||
| { | ||
| var graphServiceClient = new GraphServiceClient(new MockAuthenticationProvider().Object); | ||
| Assert.Throws<ArgumentNullException>(() => graphServiceClient.Drives["driveId"].Items["itemId"].ItemWithPath(null)); |
There was a problem hiding this comment.
Already applied in commit 23f4c9e. The test now captures the exception and asserts ex.ParamName == "path".
| public void ItemByPath_ThrowsArgumentException_WhenPathIsEmptyOrWhitespace_DriveItem(string path) | ||
| { | ||
| var graphServiceClient = new GraphServiceClient(new MockAuthenticationProvider().Object); | ||
| Assert.Throws<ArgumentException>(() => graphServiceClient.Drives["driveId"].Items["itemId"].ItemWithPath(path)); |
There was a problem hiding this comment.
Already applied in commit 23f4c9e. The test now captures the exception and asserts ex.ParamName == "path".
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…amName in tests Agent-Logs-Url: https://github.com/microsoftgraph/msgraph-sdk-dotnet/sessions/4583cb48-e9bd-44bd-935f-bf05c5334117 Co-authored-by: gavinbarron <7122716+gavinbarron@users.noreply.github.com>
Head branch was pushed to by a user without write access
NormalizePathOrThrowprivate helperStringComparison.Ordinalin second overload'sStartsWithParamName == "path"on thrown exceptionsOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Microsoft Reviewers: Open in CodeFlow