Skip to content

Commit e88e003

Browse files
hugo-synclaude
hugo-syn
and
claude
committed
C#: make OData.qll classes public, genericize test fixtures
Match WCF.qll's convention: only the TaintedMember/AdditionalTaintStep wiring classes stay private, everything else that identifies a reusable OData domain concept (ODataActionParametersClass, DeltaClass, ODataBoundType, DeltaMutatingMethod, DeltaGetInstanceMethod) is public. Also renames the test fixtures to generic placeholder names. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent a1f2b81 commit e88e003

3 files changed

Lines changed: 53 additions & 53 deletions

File tree

‎csharp/ql/lib/semmle/code/csharp/frameworks/OData.qll‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,23 @@
1717
* members need to be taint-tracked explicitly.
1818
*/
1919

20-
private import csharp
20+
import csharp
2121
private import semmle.code.csharp.commons.Collections
2222
private import semmle.code.csharp.dataflow.FlowSteps
2323
private import semmle.code.csharp.dataflow.TaintTracking
2424
private import semmle.code.csharp.dataflow.internal.DataFlowPrivate
2525

2626
/** The `ODataActionParameters` dictionary type, across OData library versions. */
27-
private class ODataActionParametersClass extends Class {
27+
class ODataActionParametersClass extends Class {
2828
ODataActionParametersClass() {
2929
this.hasFullyQualifiedName("Microsoft.AspNet.OData", "ODataActionParameters") or
3030
this.hasFullyQualifiedName("Microsoft.AspNetCore.OData.Formatter", "ODataActionParameters") or
3131
this.hasFullyQualifiedName("System.Web.Http.OData", "ODataActionParameters")
3232
}
3333
}
3434

35-
/** An indexer read on an `ODataActionParameters` dictionary, e.g. `parameters["CabFile"]`. */
36-
private class ODataActionParameterRead extends ElementAccess {
35+
/** An indexer read on an `ODataActionParameters` dictionary, e.g. `parameters["Foo"]`. */
36+
class ODataActionParameterRead extends ElementAccess {
3737
ODataActionParameterRead() { this.getQualifier().getType() instanceof ODataActionParametersClass }
3838
}
3939

@@ -43,7 +43,7 @@ private predicate isODataParameterValue(Expr e) {
4343
}
4444

4545
/** The generic `Delta<TStructuralType>` change-tracking class, across OData library versions. */
46-
private class DeltaClass extends UnboundGenericClass {
46+
class DeltaClass extends UnboundGenericClass {
4747
DeltaClass() {
4848
this.getNumberOfTypeParameters() = 1 and
4949
(
@@ -58,7 +58,7 @@ private class DeltaClass extends UnboundGenericClass {
5858
* or type-tested to -- directly, or wrapped in a collection (`List<T>`,
5959
* `IEnumerable<T>`, arrays, ...) -- or a type that is tracked by a `Delta<T>`.
6060
*/
61-
private class ODataBoundType extends ValueOrRefType {
61+
class ODataBoundType extends ValueOrRefType {
6262
ODataBoundType() {
6363
exists(Cast c | isODataParameterValue(c.getExpr()) |
6464
this = c.getTargetType() or
@@ -79,6 +79,22 @@ private class ODataBoundType extends ValueOrRefType {
7979
}
8080
}
8181

82+
/** The `Patch`, `Put`, `CopyChangedValues`, and `CopyUnchangedValues` methods on `Delta<T>`. */
83+
class DeltaMutatingMethod extends Method {
84+
DeltaMutatingMethod() {
85+
this.getDeclaringType() instanceof DeltaClass and
86+
this.hasName(["Patch", "Put", "CopyChangedValues", "CopyUnchangedValues"])
87+
}
88+
}
89+
90+
/** The `GetInstance` method on `Delta<T>`. */
91+
class DeltaGetInstanceMethod extends Method {
92+
DeltaGetInstanceMethod() {
93+
this.getDeclaringType() instanceof DeltaClass and
94+
this.hasName("GetInstance")
95+
}
96+
}
97+
8298
private class CandidateODataMember extends Member {
8399
CandidateODataMember() {
84100
this.isPublic() and
@@ -118,14 +134,6 @@ private class ODataBoundMember extends TaintTracking::TaintedMember, CandidateOD
118134
}
119135
}
120136

121-
/** The `Patch`, `Put`, `CopyChangedValues`, and `CopyUnchangedValues` methods on `Delta<T>`. */
122-
private class DeltaMutatingMethod extends Method {
123-
DeltaMutatingMethod() {
124-
this.getDeclaringType() instanceof DeltaClass and
125-
this.hasName(["Patch", "Put", "CopyChangedValues", "CopyUnchangedValues"])
126-
}
127-
}
128-
129137
/**
130138
* A call to `Delta<T>.Patch`/`Put`/`CopyChangedValues`/`CopyUnchangedValues`
131139
* copies the changes tracked by the `Delta<T>` receiver onto its `original`
@@ -141,14 +149,6 @@ private class DeltaMutatingCallTaintStep extends AdditionalTaintStep {
141149
}
142150
}
143151

144-
/** The `GetInstance` method on `Delta<T>`. */
145-
private class DeltaGetInstanceMethod extends Method {
146-
DeltaGetInstanceMethod() {
147-
this.getDeclaringType() instanceof DeltaClass and
148-
this.hasName("GetInstance")
149-
}
150-
}
151-
152152
/** `Delta<T>.GetInstance()` returns the tracked entity, carrying the same taint as the `Delta<T>` itself. */
153153
private class DeltaGetInstanceTaintStep extends AdditionalTaintStep {
154154
override predicate step(DataFlow::Node node1, DataFlow::Node node2) {

‎csharp/ql/test/library-tests/frameworks/OData/OData.cs‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -29,27 +29,27 @@ namespace Test
2929
using Microsoft.AspNet.OData;
3030
using System.Collections.Generic;
3131

32-
public class FileMetadata
32+
public class EntityMetadata
3333
{
34-
public string Author { get; set; }
34+
public string Owner { get; set; }
3535
}
3636

37-
public class UploadedFile
37+
public class BoundEntity
3838
{
39-
public string FileName { get; set; }
39+
public string Name { get; set; }
4040

41-
public string FileContent { get; set; }
41+
public string Content { get; set; }
4242

43-
public FileMetadata Metadata { get; set; }
43+
public EntityMetadata Metadata { get; set; }
4444

45-
public List<FileMetadata> History { get; set; }
45+
public List<EntityMetadata> Revisions { get; set; }
4646
}
4747

48-
public class SubscriptionRelation
48+
public class RelatedItem
4949
{
50-
public string EventName { get; set; }
50+
public string Label { get; set; }
5151

52-
public string EventType { get; set; }
52+
public string Category { get; set; }
5353
}
5454

5555
public class Widget
@@ -65,37 +65,37 @@ public class UnrelatedType
6565
public string Name { get; set; }
6666
}
6767

68-
public class OrderController
68+
public class SampleController
6969
{
7070
void Sink(object o) { }
7171

7272
void CastFromDictionary(ODataActionParameters parameters)
7373
{
74-
var file = (UploadedFile)parameters["CabFile"];
75-
Sink(file);
76-
Sink(file.FileName);
77-
Sink(file.FileContent);
78-
Sink(file.Metadata.Author);
79-
foreach (var m in file.History)
74+
var entity = (BoundEntity)parameters["Entity"];
75+
Sink(entity);
76+
Sink(entity.Name);
77+
Sink(entity.Content);
78+
Sink(entity.Metadata.Owner);
79+
foreach (var m in entity.Revisions)
8080
{
81-
Sink(m.Author);
81+
Sink(m.Owner);
8282
}
8383
}
8484

8585
void IsAsFromDictionary(ODataActionParameters parameters)
8686
{
87-
if (parameters["NewEvents"] is IEnumerable<SubscriptionRelation> relations1)
87+
if (parameters["Items"] is IEnumerable<RelatedItem> items1)
8888
{
89-
foreach (var item in relations1)
89+
foreach (var item in items1)
9090
{
91-
Sink(item.EventName);
91+
Sink(item.Label);
9292
}
9393
}
9494

95-
var relations2 = parameters["NewEvents"] as IEnumerable<SubscriptionRelation>;
96-
foreach (var item in relations2)
95+
var items2 = parameters["Items"] as IEnumerable<RelatedItem>;
96+
foreach (var item in items2)
9797
{
98-
Sink(item.EventType);
98+
Sink(item.Category);
9999
}
100100
}
101101

‎csharp/ql/test/library-tests/frameworks/OData/OData.expected‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
| OData.cs:72:55:72:64 | parameters | OData.cs:75:18:75:21 | access to local variable file |
2-
| OData.cs:72:55:72:64 | parameters | OData.cs:76:18:76:30 | access to property FileName |
3-
| OData.cs:72:55:72:64 | parameters | OData.cs:77:18:77:33 | access to property FileContent |
4-
| OData.cs:72:55:72:64 | parameters | OData.cs:78:18:78:37 | access to property Author |
5-
| OData.cs:72:55:72:64 | parameters | OData.cs:81:22:81:29 | access to property Author |
6-
| OData.cs:85:55:85:64 | parameters | OData.cs:91:26:91:39 | access to property EventName |
7-
| OData.cs:85:55:85:64 | parameters | OData.cs:98:22:98:35 | access to property EventType |
1+
| OData.cs:72:55:72:64 | parameters | OData.cs:75:18:75:23 | access to local variable entity |
2+
| OData.cs:72:55:72:64 | parameters | OData.cs:76:18:76:28 | access to property Name |
3+
| OData.cs:72:55:72:64 | parameters | OData.cs:77:18:77:31 | access to property Content |
4+
| OData.cs:72:55:72:64 | parameters | OData.cs:78:18:78:38 | access to property Owner |
5+
| OData.cs:72:55:72:64 | parameters | OData.cs:81:22:81:28 | access to property Owner |
6+
| OData.cs:85:55:85:64 | parameters | OData.cs:91:26:91:35 | access to property Label |
7+
| OData.cs:85:55:85:64 | parameters | OData.cs:98:22:98:34 | access to property Category |
88
| OData.cs:102:39:102:43 | delta | OData.cs:105:18:105:30 | access to property Name |
99
| OData.cs:108:45:108:49 | delta | OData.cs:111:18:111:23 | access to property Name |

0 commit comments

Comments
 (0)