Bugfix #1124 When CSDL filter and http resource supplied for HIDI input, it fails because ApplyFilter expects a local file but not a URL address #1125
Conversation
…because ApplyFilter expects a local file but not a URL address microsoft#1124
MazZzDaI
requested review from
MaggieKimani1,
baywet,
darrelmiller,
irvinesunday,
millicentachieng,
peombwa and
zengin
as code owners
|
@microsoft-github-policy-service agree [company="Olk inc."] |
|
@microsoft-github-policy-service agree company="Olk inc." |
| { | ||
| XslCompiledTransform transform = GetFilterTransform(); | ||
| stream = ApplyFilter(csdl, csdlFilter, transform); | ||
| stream = csdl.StartsWith("http") ? ApplyFilter(stream, csdlFilter, transform) : ApplyFilter(csdl, csdlFilter, transform); |
There was a problem hiding this comment.
| stream = csdl.StartsWith("http") ? ApplyFilter(stream, csdlFilter, transform) : ApplyFilter(csdl, csdlFilter, transform); | |
| stream = csdl.StartsWith("http", StringComparison.OrdinalIgnoreCase) ? ApplyFilter(new StreamReader(stream), csdlFilter, transform) : ApplyFilter(new StreamReader(csdl), csdlFilter, transform); |
There was a problem hiding this comment.
The original code here seems wrong. GetStream does all the logic to determine if stream is coming from a file or via HTTP. Currently the code is getting the stream and then ignoring it if there is a filter. The solution should be to change ApplyFilter to just accept the stream that comes from GetStream. And the case-insensitive check needs to be added to GetStream.
I'm not sure why the OP closed the PR. This seems like a good thing to fix.
| private static Stream ApplyFilter(string csdl, string entitySetOrSingleton, XslCompiledTransform transform) | ||
| { | ||
| StreamReader inputReader = new(csdl); | ||
| return ApplyFilterForStreamReader(inputReader, entitySetOrSingleton, transform); | ||
| } | ||
|
|
||
| private static Stream ApplyFilter(Stream csdlStream, string entitySetOrSingleton, XslCompiledTransform transform) | ||
| { | ||
| StreamReader inputReader = new(csdlStream); | ||
| return ApplyFilterForStreamReader(inputReader, entitySetOrSingleton, transform); | ||
| } | ||
|
|
There was a problem hiding this comment.
| private static Stream ApplyFilter(string csdl, string entitySetOrSingleton, XslCompiledTransform transform) | |
| { | |
| StreamReader inputReader = new(csdl); | |
| return ApplyFilterForStreamReader(inputReader, entitySetOrSingleton, transform); | |
| } | |
| private static Stream ApplyFilter(Stream csdlStream, string entitySetOrSingleton, XslCompiledTransform transform) | |
| { | |
| StreamReader inputReader = new(csdlStream); | |
| return ApplyFilterForStreamReader(inputReader, entitySetOrSingleton, transform); | |
| } |
| } | ||
|
|
||
| private static Stream ApplyFilter(string csdl, string entitySetOrSingleton, XslCompiledTransform transform) | ||
| private static Stream ApplyFilterForStreamReader(StreamReader inputReader, string entitySetOrSingleton, XslCompiledTransform transform) |
There was a problem hiding this comment.
| private static Stream ApplyFilterForStreamReader(StreamReader inputReader, string entitySetOrSingleton, XslCompiledTransform transform) | |
| private static Stream ApplyFilter(StreamReader inputReader, string entitySetOrSingleton, XslCompiledTransform transform) |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| { | ||
| StreamReader inputReader = new(csdlStream); | ||
| return ApplyFilterForStreamReader(inputReader, entitySetOrSingleton, transform); | ||
| } |
Check warning
Code scanning / CodeQL
Missing Dispose call on local IDisposable
Check warning
Code scanning / CodeQL
Missing Dispose call on local IDisposable
|
@MazZzDaI is there any specific reason why you closed that PR? |
3 participants
Fix of the #1124 issue.
Overload added for Microsoft.OpenApi.Hidi/OpenApiService.cs/ApplyFilter to accept both string and stream variables.