Initial patch to create PCL assembly

[Arthur Vickers]

Initial version of the patch to start the contribution process as discussed in email.

Thanks,

Arthur

[Michael Ketting]

Hello Arthur!

Thank you for submitting the patch.
I have created a JIRA issue for implementing the PCL-support: https://www.re-motion.org/jira/browse/RM-6132
and a feature branch in our SVN: https://svn.re-motion.org/svn/Remotion/branches/RM-6132-ReLinq_PCL/

I have already integrated the patch into the feature branch, but there are two issues and one review left to do:
1. We want to keep support for ArrayList.Count and Array.LongLength, so we'll switch this to use reflection to discover the reflection objects. (https://www.re-motion.org/jira/browse/RM-6134)
2. Replacing the MethodHandle-based matching of the generic method definition in MethodInfoBasedNodeTypeRegistry needs some work as the match conditions were not sufficient to prevent ambiguities. I added a test for this. In addition, we will now likely need to do some caching for this case to keep things performing. (https://www.re-motion.org/jira/browse/RM-6136)
Review: I was also able to streamline some of the reflection API calls and will need to review the remaining one in TypeExtensions class.

If you have any input on the generics stuff, it's certainly welcome. Aside from that, I'll probably be able to finish the integration on Wednesday (aka tomorrow).

And lastly, a question: As you know, it was necessary to drop the SecurityRulesAttribute in order to support PCL. We have only applied this attribute to force the Remotion.Linq assembly into Level-1 code access security mode to keep from having to rework our custom exception classes. Now that the custom exception classes have been evicted from Remotion.Linq assembly, the attribute should no longer be needed. But it would be enormously helpful, if you could check with your colleagues whether this change has any other impacts into the partial-trust support of Remotion.Linq. We do have tests within the test-suite that assert the partial trust feature by opening a separate AppDomain with lower trust settings and exercsing some of the re-linq code, but a second opinion in regards to .NET 4 defaults would be appreciated.

Best regards, Michael

[Andrew Peters]

Thanks Michael!

WRT SecurityRulesAttribute, I followed up with Levi our security guru. Here is his reply:

"If you remove [SecurityRules], also remove any assembly-level [AllowPartiallyTrustedCallers] and [SecurityTransparent] attributes you find.

One scenario that will be broken by this change is that you can no longer consume the assembly from partial-trust code if the assembly has been GACed.  If the assembly is bin-deployed / xcopy-deployed alongside the partial-trust code, the library should continue to work."

Cheers,

Andrew.

[Michael Ketting]

Hi Andrew!

Thanks for checking up on this. Of course, with security, answers breed more questions. So, here we go:

If I remove the APTCA, every library that extends re-linq will also have to remove the APTCA from their code, otherwise we get exceptions such as this one:

Inheritance security rules violated by type: 'Remotion.Linq.SqlBackend.SqlStatementModel.SqlSpecificExpressions.SqlRowNumberExpression'. Derived types must either match the security accessibility of the base type or be less accessible.

That would be a significant break we'd have to check with the other users of re-linq via the mailing list, and if we do check this, we'd need to provide a succinct statement on the effects and side-effects so the re-linq users don't have to dig into the security stuff themselves.

On the other hand, if we leave the APTCA applied to re-linq, I did not find any detrimental side effects:

• Any library extending or using re-linq will continue to work, whether or not they themselves apply the attribute.
• Putting re-linq into the GAC should not make a difference when the APTCA is applied.
  

Just for completeness sake: The APTCA was orginally introduced to the re-linq assembly during .NET 3.5 times to support partially trusted callers. The SecurityRules (Level1) attribute was then applied to work around our custom exceptions with serialization support. Since we no longer have those, there's no longer an issue.

Could you maybe check again with Levi what the downside of leaving the APTCA on the assembly would be to re-linq or if I missed something?

Many thanks, Michael

[Andrew Peters]

You're welcome.

The short answer is that partial trust is obsolete going forward. It no longer works in ASP.NET and is also being phased out by the CLR team.

See: http://support.microsoft.com/kb/2698981 and http://stackoverflow.com/questions/16849801/is-trying-to-develop-for-medium-trust-a-lost-cause for details on that.

According to Levi, the ideal thing to do here is remove the support for partial trust. We have done this with our own libraries even though it causes some short term pain. The next best thing is to switch to the multiple assembly PCL model, so that the net45 assembly can continue to specify the security attributes.

The long answer:

"There are two levels of CAS: Level 1 and Level 2.  [SecurityRules] opts you in to Level 1; without this attribute you're opted in to Level 2.  APTCA has different behavior between Level 1 and Level 2, and I was asking you to remove it in order to avoid introducing vulnerabilities into the framework.  The "safe" thing to do what would prevent the exception you were seeing earlier:

- Remove assembly-level [SecurityRules].
- Remove assembly-level APTCA.
- Add assembly-level [SecurityTransparent] if it doesn't already exist.

APTCA aside, the transition from Level 1 to Level 2 CAS is actually quite impactful and has the possibility to introduce runtime breaks.  (The behavior of CAS is radically different between the two levels.)  If you remove [SecurityRules] you'll basically have to retest every single line of code to make sure everything is still behaving properly."

Cheers,

Andrew.

1. We want to keep support for ArrayList.Count and Array.LongLength, so we'll switch this to use reflection to discover the reflection objects. (<a href="https://www.re-motion.org/jira/browse/RM-6134" target="_blank" onmousedown="this.href='https://www.google.com/url?q\75https%3A%2F%2Fwww.re-motion.org%2Fjira%2Fbrowse%2FRM-6134\46sa\75D\46sntz\0751\46usg\75AFQjCNGD1ESsijw5_qBsv2XxhX92pJQt

...

[Ketting, Michael]

Hi Andrew!

Okay, good to know about the planned obsoleteness. Kicking a feature in a downstream component because of an obsolete dependency is always easy to justify. Thanks for the deep-dive summary :)

> - Add assembly-level [SecurityTransparent] if it doesn't already exist.

Since I don't have VS open right now to check this out live, does your explanation also mean that a downstream assembly can still use the APTCA if my assembly applies the SecurityTransparentAttribute? Either way, given the way this is going to be handled in the .NET Framework going forward, it does seem like it's just a matter of having the migration pains now or later and since we're already making some housecleaning right now, this change might just fit into the rest.

If you've been watching the feature branch, I'm still in the process of handling the generics lookup without having the MethodHandle to resort to. Good news, the difficult stuff should be done now, all that's left are a couple of tests and integration. If that holds and there's no more security-related stuff, the PCL support should be on the homestretch. Sorry that Wednesday didn't pan out.

Best regards, Michael

---

From: re-mot...@googlegroups.com [re-mot...@googlegroups.com] on behalf of Andrew Peters [anpet...@live.com]
Sent: Thursday, April 17, 2014 20:18
To: re-mot...@googlegroups.com
Subject: [re-motion-dev] Re: [re-linq] Initial patch to create PCL assembly

--
You received this message because you are subscribed to the Google Groups "re-motion Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to re-motion-de...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Michael Ketting]

Hi Arthur!
Hi Andrew!

I'm happy to report that the integration of your patch is now finished and the new version of re-linq has been published to nuget.

Here's the announcement post on the users-list.
https://groups.google.com/forum/?pli=1#!topic/re-motion-users/aJYFtkc1Zeo

Thanks again for getting the ball rolling on the PCL stuff!

Best regards, Michael

[Andrew Peters]

Thanks Michael!