Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions src/Microsoft.OpenApi.Hidi/OpenApiService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ CancellationToken cancellationToken
if (!string.IsNullOrEmpty(csdlFilter))
{
XslCompiledTransform transform = GetFilterTransform();
stream = ApplyFilter(csdl, csdlFilter, transform);
stream = csdl.StartsWith("http") ? ApplyFilter(stream, csdlFilter, transform) : ApplyFilter(csdl, csdlFilter, transform);
Copy link
Member

@baywet baywet Jan 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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);

Copy link
Member

@darrelmiller darrelmiller Jan 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

stream.Position = 0;
}

Expand Down Expand Up @@ -235,20 +235,32 @@ private static XslCompiledTransform GetFilterTransform()
return transform;
}

private static Stream ApplyFilter(string csdl, string entitySetOrSingleton, XslCompiledTransform transform)
private static Stream ApplyFilterForStreamReader(StreamReader inputReader, string entitySetOrSingleton, XslCompiledTransform transform)
Copy link
Member

@baywet baywet Jan 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private static Stream ApplyFilterForStreamReader(StreamReader inputReader, string entitySetOrSingleton, XslCompiledTransform transform)
private static Stream ApplyFilter(StreamReader inputReader, string entitySetOrSingleton, XslCompiledTransform transform)

{
Stream stream;
StreamReader inputReader = new(csdl);
XmlReader inputXmlReader = XmlReader.Create(inputReader);
MemoryStream filteredStream = new();
StreamWriter writer = new(filteredStream);
XsltArgumentList args = new();
args.AddParam("entitySetOrSingleton", "", entitySetOrSingleton);
transform.Transform(inputXmlReader, args, writer);
stream = filteredStream;

return stream;
}

private static Stream ApplyFilter(string csdl, string entitySetOrSingleton, XslCompiledTransform transform)
{
StreamReader inputReader = new(csdl);

Check warning

Code scanning / CodeQL

Missing Dispose call on local IDisposable

Disposable 'StreamReader' is created but not disposed.
return ApplyFilterForStreamReader(inputReader, entitySetOrSingleton, transform);
}

private static Stream ApplyFilter(Stream csdlStream, string entitySetOrSingleton, XslCompiledTransform transform)
{
StreamReader inputReader = new(csdlStream);

Check warning

Code scanning / CodeQL

Missing Dispose call on local IDisposable

Disposable 'StreamReader' is created but not disposed.
return ApplyFilterForStreamReader(inputReader, entitySetOrSingleton, transform);
}

Comment on lines +252 to +263
Copy link
Member

@baywet baywet Jan 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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);
}

/// <summary>
/// Implementation of the validate command
/// </summary>
Expand Down