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

adjust XmlReader.Create to passed path with potentially invalid character #9028

Merged

Conversation

YuliiaKovalova
Copy link
Member

@YuliiaKovalova YuliiaKovalova commented Jul 13, 2023

Fixes #8972

Context

#8931 fixed one instance of the issue with build issues caused by localized characters in OS paths.
This PR attempts to address the rest of the same unintended string -> uri conversion

Changes Made

Passing Stream to XmlReader.Create instead of path in order to prevent unintended string -> uri conversion

Testing

N/A

@YuliiaKovalova YuliiaKovalova marked this pull request as draft July 13, 2023 12:46
@YuliiaKovalova YuliiaKovalova marked this pull request as ready for review July 14, 2023 07:26
Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Thanks!
Looks good!

@JaynieBai JaynieBai merged commit f5e2555 into dotnet:main Jul 17, 2023
8 checks passed
YuliiaKovalova added a commit to YuliiaKovalova/msbuild that referenced this pull request Jul 18, 2023
…cter (dotnet#9028)

Fixes dotnet#8972

Context
dotnet#8931 fixed one instance of the issue with build issues caused by localized characters in OS paths.
This PR attempts to address the rest of the same unintended string -> uri conversion

Changes Made
Passing Stream to XmlReader.Create instead of path in order to prevent unintended string -> uri conversion

Testing
N/A
@YuliiaKovalova YuliiaKovalova deleted the bugfix-substitute-string-with-stream branch July 19, 2023 10:15
@rainersigwald
Copy link
Member

/backport to vs17.7

@github-actions
Copy link
Contributor

Started backporting to vs17.7: https://github.com/dotnet/msbuild/actions/runs/5657870847

@github-actions
Copy link
Contributor

@rainersigwald an error occurred while backporting to vs17.7, please check the run log for details!

Error: @rainersigwald is not a repo collaborator, backporting is not allowed. If you're a collaborator please make sure your dotnet team membership visibility is set to Public on https://github.com/orgs/dotnet/people?query=rainersigwald

@rainersigwald
Copy link
Member

/backport to vs17.7

@github-actions
Copy link
Contributor

Started backporting to vs17.7: https://github.com/dotnet/msbuild/actions/runs/5657914190

Copy link
Member

@ericstj ericstj left a comment

Choose a reason for hiding this comment

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

LGTM; @karelz @MihaZupan here's a place where folks are working around URI's escaping of these characters.

@JanKrivanek
Copy link
Member

LGTM; @karelz Karel Zikmund FTE @MihaZupan Miha Zupan FTE here's a place where folks are working around URI's escaping of these characters.

We've already discussed this use-case with @karelz when working on #8931

rainersigwald added a commit to rainersigwald/msbuild that referenced this pull request Jul 26, 2023
These overloads create a URI from the string and can cause problems with
GB18030 certification, because that URI gets normalized in a way that
doesn't work with all characters. We should instead pass a stream
created from the file, as in dotnet#8931 and dotnet#9028.

Formalize that rule for the whole repo.
YuliiaKovalova added a commit that referenced this pull request Aug 2, 2023
* Ban XmlReader overloads that take string

These overloads create a URI from the string and can cause problems with
GB18030 certification, because that URI gets normalized in a way that
doesn't work with all characters. We should instead pass a stream
created from the file, as in #8931 and #9028.

Formalize that rule for the whole repo.

* Add XPathDocument string ctors to banned symbols

* fix 2 more cases for XmlReader.Create call

* Update src/Tasks/XamlTaskFactory/RelationsParser.cs

Co-authored-by: Jan Krivanek <jankrivanek@microsoft.com>

* fix review comments

* Update src/Tasks/XamlTaskFactory/RelationsParser.cs

Co-authored-by: Rainer Sigwald <raines@microsoft.com>

---------

Co-authored-by: Jan Krivanek <jankrivanek@microsoft.com>
Co-authored-by: YuliiaKovalova <ykovalova@microsoft.com>
Co-authored-by: YuliiaKovalova <95473390+YuliiaKovalova@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Audit/Fix code issues with localized characters in paths
5 participants