Add UriPatcher to fix URI quirks #698
Conversation
googlebot
added
the
cla: yes
|
Can we get rid of the "problem 1" part due to #696, or does it still affect PCLs? (Sorry if that's explained - I'm still trying to take it all in.) |
|
I understand being confused. It's confusing. I can only mostly hold it in my head, and then just for small bits of time. :) The problem is DLLs. Thanks to #696, no one is going to use NuGet to import our libraries into an application (.exe) that targets .NET 4.0. However, folks might write DLLs that target .NET 4.5 and import our libraries. If that DLL, in turn, is loaded into an application that targets .NET 4.0, some of our requests will fail because we're getting quirked URI behavior. The poster child for this use case is Powershell. Powershell loads |
|
The hardest part for me to remember is that we're writing all this code to handle old versions but it will only ever run on new versions. Everything is .NET 4.5+ now, even if it's lying to an unaware application and acting like 4.0. This is code to tell .NET 4.5+ to stop pretending to be .NET 4.0 when it comes to URIs. That pretending happens when the entry assembly -- which isn't us, and may not be the library that compiled against us -- targets v4.0. |
|
Okay, so we definitely won't be running on 4.0, but the behaviour might be as if we were. I see that referenced in the code, but I'd misread it before. Right, time to read the code itself... |
| // https://github.com/Microsoft/referencesource/blob/d925d870f3cb3f6a/System/net/System/_UriSyntax.cs#L77 | ||
| // https://github.com/Microsoft/referencesource/blob/d925d870f3cb3f6a/System/net/System/_UriSyntax.cs#L352 | ||
|
|
||
| var setUpdatableFlagsMethod = uriParser.GetMethod("SetUpdatableFlags", |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
@jskeet Updated -- PTAL. |
| // As a class library, we can't control app.config or the entry assembly, so we can't take | ||
| // either approach. Instead, we resort to reflection trickery to try to solve these problems | ||
| // if we detect they exist. Sorry. | ||
| static internal class UriPatcher |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
| if (parserField == null) { return; } | ||
| var parserInstance = parserField.GetValue(null); | ||
| if (parserInstance == null) { return; } | ||
| setUpdatableFlagsMethod.Invoke(parserInstance, new object[] { 0 }); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
"Looks least-worst-option to me". (Can't quite bring myself to say it looks good...) |
Tested with: `nunit3-console.exe --domain=None --framework=net-4.0 Google.Apis.Tests.dll` Before, we saw three test failures as described in #636. Now all tests pass.
|
Least-worst is what I was shooting for. At least I said "Sorry". :) |
Add UriPatcher to fix URI quirks
Fixes #636, #643.
Tested with:
nunit3-console.exe --domain=None --framework=net-4.0 Google.Apis.Tests.dllI abandoned a previous approach that modified individual Uri objects in favor of a simpler and less fragile approach that makes process-wide changes. Oh well. It seems unlikely that customers writing to Google APIs are also depending on weird URL behavior.
We may need to move the call to PatchUriQuirks or make similar calls elsewhere, to ensure it anticipates all of our uses of Uri.