-
Notifications
You must be signed in to change notification settings - Fork 176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2.11.0: Invoke-MgGraphRequest Broken #2488
Comments
Thanks for opening this. I spent all day troubleshooting a function app due to this issue. |
Same! I just came here to log an issue after wondering why my delta queries were not working any more. The following URL, which is an example from the MS docs, works fine up to 2.10.0:
But fails in 2.11.0:
Looks like the URL is getting mangled. Thanks, |
Confirming the same here, this also broke some other modules that are dependent on this (example: WindowsAutopilotIntune). Escaping those characters in the URI unfortunately doesn't help either. |
I'm also seeing issues with pagination, trying to grab the next link errors out |
FYI for those who are struggling to get
If you still find |
It looks like the module is urlencoding the uri so something like '/v1.0/users?$top=10' is sent to '/v1.0/users%3F%24top%3D10' which graph rejects. |
Looks like 2.11.1 has fixed this. |
I looked at the source and the issue stems from a new method intended to encode the uri, but it uses very rudimentary logic that just splits the uri by the slashes and encodes anything after the https://. This inadvertently encodes query parameters. At a minimum, they'd need to split off the query params and append them after encoding the rest of the url. This looks like it was intended to handle special characters that are in UPNs like external accounts with #EXT#. The funny thing is that currently graph accepts these as is and doesn't require encoding the # symbols. That said, it may be a portent of things to come (please don't get rid of this capability)... Note to devs, if there's a bigger reason this change is needed, you'll need to keep in mind if encoding may need to be performed on certain query parameters like $filter (e.g. /v1.0/users?$filter=userPrincipalname eq 'someone_outlook.com#EXT#@domain.onmicrosoft.com' being used instead of /v1.0/users/someone_outlook.com#EXT#@domain.onmicrosoft.com) so it's not as straightforward as just excluding all query params from encoding... |
Yeah, they just reverted the call to the new helper function uriBuilder.Uri.EscapeDataStrings and return the uriBuilder.Uri unaltered. Quick fix while they figure out what they're going to do next. I'm really surprised this passed automated testing. Any tests that used a single query parameter would have caught this bug :( Just goes to show you that if the person making the change also makes the test, any blind spots will be present in both. I miss the days of dedicated test engineers. They made breaking code an art form 😆 |
Sorry for any inconvenience caused by the bug. A patch has been released in version 2.11.1 to address the bug that affected users of v2.11.0. The patch reverts the change that caused query parameters to be double encoded when using the command. #1947 has been reopened to track encoding of |
Describe the bug
After upgrading to
2.11.0
(published in PSGallery), my calls usingInvoke-MgGraphRequest
all started getting 400 Bad Requests.Downgrading to
2.10.0
fixes the issue.To Reproduce
Steps to reproduce the behavior:
Expected behavior
The API should return the result if I belong to the group id passed, or return a 404 if I don't.
Debug Output
I am intentionally not providing anything here since the screenshot covers it, and the debug output was no more helpful.
Module Version
2.11.0
Environment Data
Screenshots
See above
Additional context
I recognize that
2.11.0
hasn't been formally cut or set as latest in the release, but PSGallery is showing it as latest.The text was updated successfully, but these errors were encountered: