WindowsRuntimeBufferExtensions.ToArray incorrectly checks Capacity

[jefffhaynes]

The following code will throw an ArgumentException:

var buffer = new Windows.Storage.Streams.Buffer(0);
var array = buffer.ToArray();

WindowsRuntimeBufferExtensions.ToArray checks the buffer capacity as a precondition, which isn't used in the method. Obviously the expected behavior would be to return an array of length zero.

I also can't find a unit test for this case.

[danmoseley]

@jefffhaynes I think you're right. Would you like to submit a PR maybe?

[jefffhaynes]

Sure. I'll take a stab at it tomorrow

…

________________________________ From: Dan Moseley <[email protected]> Sent: Wednesday, August 30, 2017 12:27:30 AM To: dotnet/corefx Cc: Jeff Haynes; Mention Subject: Re: [dotnet/corefx] WindowsRuntimeBufferExtensions.ToArray incorrectly checks Capacity (#23671) @jefffhaynes<https://github.com/jefffhaynes> I think you're right. Would you like to submit a PR maybe? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://github.com/dotnet/corefx/issues/23671#issuecomment-325876810>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AJSKRy-1-4RWkUwHWDItS7B13O1G5Vj9ks5sdOSygaJpZM4PGuHM>.

[jefffhaynes]

So far no luck getting the repo to build and test cleanly. I'll work on it more this weekend.

[jefffhaynes]

Maybe you can help me a little - the following does not seem to actually do anything:

build src\System.Runtime.WindowsRuntime\src

Similarly build src\System.Runtime.WindowsRuntime\tests does not seem to do anything. In contrast, running src\System.Collections\src does appear to do something. However, if I make a breaking change somewhere in Systems.Collection and build again it still succeeds. I have not been able to ascertain how to clean a specific directory. I also have been unable to get Visual Studio to build anything despite following the directions on here.

But more perplexing is if I make a breaking change to System.Runtime.WindowsRuntime, clean, and build from root, the build succeeds. This would seem to suggest that somehow that area isn't being built at all.

I'm sure I'm missing something here but I'm kind of stuck. Happy to help if you can point me in the right direction.

EDIT: I should also note that System.Runtime.InteropServices also seems to build but does not seem to build (for example) WindowsRuntimeBufferExtensions.cs.

[juliushardt]

As I see it, WindowsRuntimeBufferExtensions.ToArray() is working as expected.
If you call ToArray() on a buffer of length zero, you effectively get WindowsRuntimeBufferExtensions.ToArray(buffer, sourceIndex: 0, count: buffer.Length) because the former calls the latter.

Since sourceIndex and the capacity of the source are both 0, an exception is thrown. According to the API docs, this behaviour is intended (emphasis mine):

//   T:System.ArgumentException:
//     sourceIndex is greater than or equal to the capacity of source. -or-The number
//     of bytes in source, beginning at sourceIndex, is less than count.
[CLSCompliant(false)]
public static byte[] ToArray(this IBuffer source, uint sourceIndex, int count);

Here's the corresponding unit test.

Missing docs

It looks like the API docs for this method are currently not available on docs.microsoft.com. I created an issue on the docs repo.

Additionally, I think that the docs for the short overload should clarify that calling it on an empty buffer throws. We should file a separate issue once the docs are up again.

Building System.Runtime.WindowsRuntime

I stumbled across that, too, @jefffhaynes. In order to build System.Runtime.WindowsRuntime, you need to add a parameter to the build command: build.cmd -framework=uap, as it defaults to "netcoreapp" if it is omitted. You can read more about that here.

[jefffhaynes]

I see what you’re saying but that behavior makes zero sense and runs contrary to every other interface in .net with similar semantics (array copy, stream write, etc). The implementation actually looks like a copy and paste error since the check serves no purpose and is overprotective of the method. Thanks for the info on building the framework!

…

________________________________ From: Julius Hardt <[email protected]> Sent: Wednesday, September 26, 2018 6:28 PM To: dotnet/corefx Cc: Jeff Haynes; Mention Subject: Re: [dotnet/corefx] WindowsRuntimeBufferExtensions.ToArray incorrectly checks Capacity (#23671) As I see it, WindowsRuntimeBufferExtensions.ToArray() is working as expected. If you call ToArray() on a buffer of length zero, you effectively get WindowsRuntimeBufferExtensions.ToArray(buffer, sourceIndex: 0, count: buffer.Length) because the former calls the latter<https://github.com/dotnet/corefx/blob/e0ba7aa8026280ee3571179cc06431baf1dfaaac/src/System.Runtime.WindowsRuntime/src/System/Runtime/InteropServices/WindowsRuntime/WindowsRuntimeBufferExtensions.cs#L138>. Since sourceIndex and the capacity of the source are both 0, an exception is thrown. According to the API docs, this behaviour is intended (emphasis mine): // T:System.ArgumentException: // sourceIndex is greater than or equal to the capacity of source. -or-The number // of bytes in source, beginning at sourceIndex, is less than count. [CLSCompliant(false)] public static byte[] ToArray(this IBuffer source, uint sourceIndex, int count); Here's the corresponding unit test.<https://github.com/dotnet/corefx/blob/a10890f4ffe0fadf090c922578ba0e606ebdd16c/src/System.Runtime.WindowsRuntime/tests/System/Runtime/InteropServices/WindowsRuntimeBufferExtensionsTests.cs#L430> Missing docs It looks like the API docs for this method are currently not available on docs.microsoft.com. I created an issue<https://github.com/dotnet/docs/issues/7919> on the docs repo. Additionally, I think that the docs for the short overload should clarify that calling it on an empty buffer throws. We should file a separate issue once the docs are up again. Building System.Runtime.WindowsRuntime I stumbled across that, too, @jefffhaynes<https://github.com/jefffhaynes>. In order to build System.Runtime.WindowsRuntime, you need to add a parameter to the build command: build.cmd -framework=uap, as it defaults to "netcoreapp" if it is omitted. You can read more about that here<https://github.com/dotnet/corefx/wiki/Build-and-run-tests#advanced-build-options>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://github.com/dotnet/corefx/issues/23671#issuecomment-424889938>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AJSKR9MyIOKhyPOi23PeRNSLjE7FvqtMks5ue_-OgaJpZM4PGuHM>.

[GSPP]

sourceIndex is greater than or equal to

This should be changed. This should not break any practical code. It's useful, and common, to allow taking a zero length slice from the end of a sequence.

[jefffhaynes]

Just to rant momentarily, something like this never would have been a discussion with .net “classic” and it would have been fixed by now. It’s bugs like this that are still preventing UWP from being more widely adopted.

[iSazonov]

@jeffschwMSFT Is WindowsRuntimeBufferExtensions removed from the repository?

[jeffschwMSFT]

In .NET 5 Preview 6 we removed the built in WinRT support. You can see our blog for details (https://devblogs.microsoft.com/dotnet/announcing-net-5-0-preview-6/). The WinRT support is now being handled by https://github.com/microsoft/CsWinRT.

cc @AaronRobinsonMSFT @jkoritzinsky

[iSazonov]

@jeffschwMSFT Thanks! I wonder that the issue is still open here.

[jkoritzinsky]

I think we just missed this issue. I think we should move this issue to microsoft/CsWinRT, but I don't have the right tools installed to do that transfer. @jeffschwMSFT can you port this issue to microsoft/CsWinRT?

[jeffschwMSFT]

@jkoritzinsky I think the best option would be to copy this content into a new issue and close this one.

[AaronRobinsonMSFT]

Moved to CsWinRT: microsoft/CsWinRT#334