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

[Don't Merge Yet] Fix #3906: missing url escape of square backets #3908

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jbaehr
Copy link

@jbaehr jbaehr commented Sep 14, 2020

As System.Uri.GetComponents(...UriFormat.SafeUnescaped) is based on
the outdated RFC 2396 it unescapes angle brackets, which usage is only
declared "unwise" in the original URI RFC from 1998. The uptodate one,
RFC 3986, forbidds them in path segments. This causes trouble with
software based on the newer RFC, such as the OPC API in
System.IO.Packaing.

Instead of hooking a third layer of escape/unescape layer on top of the
existing code, I decided to rewrite the ensureValidName method, hoping
the intend of the code is clearer now. For more details see the comments
in #3906.

Note: I was only able to verify the method isolated in LinqPad. I tried hard but failed to compile paket (and thus run the tests) because of a corporate proxy requiring credentials. Sorry for that; let's hope for the CI.

As System.Uri.GetComponents(...*UriFormat.SafeUnescaped*) is based on
the outdated RFC 2396 it unescapes angle brackets, which usage is only
declared "unwise" in the original URI RFC from 1998. The uptodate one,
RFC 3986, forbidds them in path segments. This causes trouble with
software based on the newer RFC, such as the OPC API in
`System.IO.Packaing`.

Instead of hooking a third layer of escape/unescape layer on top of the
existing code, I decided to rewrite the `ensureValidName` method, hoping
the intend of the code is clearer now. For more details see the comments
in fsprojects#3906.
@jbaehr jbaehr changed the title Fix #3906: missing url escape of square backets [Don't Merge Yet] Fix #3906: missing url escape of square backets Sep 22, 2020
@jbaehr
Copy link
Author

jbaehr commented Sep 22, 2020

Currently, it's not clear if this proposed fix here is for better or worse, cf. #3906 (comment).
So for now, I suggest to not merge.

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.

1 participant