Skip to content
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.

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

Initialize XmlReaders using StreamReaders #6863

Merged
merged 3 commits into from
Sep 29, 2021

Conversation

benvillalobos
Copy link
Member

Fixes #6847

Context

When XmlReader.Create is called and a file path is passed in, it's converted to a URI under the hood. This can mangle multi-byte characters. The solution (in the link above) is to initialize the XmlReader using a stream to the file instead of the path alone.

Changes Made

Call XmlReader.Create with a stream that will detect encodings.

Testing

Tested using repro in #6847

Notes

@benvillalobos
Copy link
Member Author

Digging into the error a bit more, it looks like initializing the XmlReader with a FileScheme Uri causes funky behavior when transforming an Xslt doc that calls the Document function. The Document function looks up data in another document.

From what I can tell, loading the document from a stream is causing issues when calling functions from the loaded xslt. When Document is called from a.xml and looks for b.xml, it now tries to find b.xml in the output bin directory from the current unit test project, instead of temp where the files were generated.


A quick chat with Rainer showed that there's an overload to XmlReader.Create that takes a base path, and passing the original filepath ensures the path stays relative, solving the Document issue and the mangled character issue 🎉

@benvillalobos
Copy link
Member Author

To-Do: Test an xslt doc calling the Document method for a file in the same unicode-style path that causes the error.

@benvillalobos
Copy link
Member Author

To-Do: Test an xslt doc calling the Document method for a file in the same unicode-style path that causes the error.

It fails

image

"C:\src\temp\9-16\11_25\new\???????????????'`?-?        ????U1[?????]U2[????]U3[?????]\proj1\proj1.csproj" (default target) (1) ->
(Foo target) ->
  C:\src\temp\9-16\11_25\new\???????????????'`?-?       ????U1[?????]U2[????]U3[?????]\proj1\proj1.csproj(11,5): error MSB3703: Unable to execute transformation. An error occurred while loading document 'b.xml'. See InnerException for a c
omplete description of the error.

    0 Warning(s)
    1 Error(s)

Time Elapsed 00:00:02.53

We'd need to catch this exception and output a better string here.

@benvillalobos
Copy link
Member Author

imo we merge this and file an issue in the runtime tracking this

@benvillalobos benvillalobos changed the title Initialize XmlReaders with a stream that can detect encodings Initialize XmlReaders using StreamReaders Sep 20, 2021
@Forgind Forgind added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Sep 20, 2021
@benvillalobos
Copy link
Member Author

Issue filed: dotnet/runtime#59353

@benvillalobos benvillalobos merged commit ceb2a05 into dotnet:main Sep 29, 2021
@benvillalobos benvillalobos deleted the xmlreader-streams branch September 29, 2021 16:49
lanfeust69 added a commit to lanfeust69/msbuild that referenced this pull request Oct 13, 2021
Following dotnet#6863, the created XmlReader is no longer responsible for its
underlying stream. This can cause the build process to hold on to the
processed file, preventing its removal. This can especially be a problem
when the transformation is in fact aimed at the input file itself, where
we want to create the transformed file, then move it to the original.
lanfeust69 added a commit to lanfeust69/msbuild that referenced this pull request Oct 14, 2021
Following dotnet#6863, the created XmlReader is no longer responsible for its
underlying stream. This can cause the build process to hold on to the
processed file, preventing its removal. This can especially be a problem
when the transformation is in fact aimed at the input file itself, where
we want to create the transformed file, then move it to the original.
lanfeust69 added a commit to lanfeust69/msbuild that referenced this pull request Oct 14, 2021
Following dotnet#6863, the created XmlReader is no longer responsible for its
underlying stream. This can cause the build process to hold on to the
processed file, preventing its removal. This can especially be a problem
when the transformation is in fact aimed at the input file itself, where
we want to create the transformed file, then move it to the original.
lanfeust69 added a commit to lanfeust69/msbuild that referenced this pull request Oct 14, 2021
Following dotnet#6863, the created XmlReader is no longer responsible for its
underlying stream. This can cause the build process to hold on to the
processed file, preventing its removal. This can especially be a problem
when the transformation is in fact aimed at the input file itself, where
we want to create the transformed file, then move it to the original.
lanfeust69 added a commit to lanfeust69/msbuild that referenced this pull request Oct 14, 2021
Following dotnet#6863, the created XmlReader is no longer responsible for its
underlying stream. This can cause the build process to hold on to the
processed file, preventing its removal. This can especially be a problem
when the transformation is in fact aimed at the input file itself, where
we want to create the transformed file, then move it to the original.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Initialize XmlReader Using A Stream
3 participants