urlapi: UNC paths in file: URLs on Windows

[sergio-nsk]

• file://host.name/path/file.txt is a valid UNC path \host.name\path\files.txt
  to a non-local file transformed into URI (RFC 8089 Appendix E.3)

• UNC paths on other OSs must be smb: URLs

• file://host.name/path/file.txt is working in browsers, should work in cURL also

[ghost]

Congratulations 🎉. DeepCode analyzed your code in 3.084 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

👉 The DeepCode service and API will be deprecated in August, 2021. Here is the information how to migrate. Thank you for using DeepCode 🙏 ❤️ !

If you are using our plugins, you might be interested in their successors: Snyk's JetBrains plugin and Snyk's VS Code plugin.

[bagder]

file://host.name/path/file.txt is working in browsers, should work in cURL also

And does it with this change?

There's no test case updates in this PR, which probably explains the test failures on Windows.

Shouldn't docs/URL-SYNTAX.md also be updated accordingly?

[sergio-nsk]

And does it with this change?

Yes, it does. I tested it locally in my environment using windows shared paths like file://192.168.1.25/shared/path/...

There's no test case updates in this PR, which probably explains the test failures on Windows.

I should look at the broken tests. Perhaps the tests expecting the previous CURLUE_MALFORMED_INPUT fail now.

Shouldn't docs/URL-SYNTAX.md also be updated accordingly?

Perhaps yes, should be updated.

[bagder]

This seems to have broken test 2080 on many windows builds?

[sergio-nsk]

This seems to have broken test 2080 on many windows builds?

file://dev/null is a valid UNC path \\dev\null, thus I tried to skip this test on Windows, but it did not help:

*** 17,22 ****
--- 17,26 ----
 <server>
 none
 </server>
+ <!-- valid UNC path on win32 -->
+ <features>
+ !Win32
+ </features>
  <name>
 config file with overly long option
  </name>

Where am I wrong?
It seems !Win32 must be !win32. I have updates the change set.

[bagder]

file://dev/null is a valid UNC path

You mean invalid ?

Why is that an invalid URL? It seems to match the syntax. The test case will never actually use the URL for anything but the parsing of the URL should succeed...

[sergio-nsk]

file://dev/null is malformed file: URL in Unix. This is a valid file: URL in Windows.
Valid paths in Unix: file:///dev/null, a traditional file URI for a local file, or file:/dev/null, the minimalistic file URI for a local file.
file://dev/null is a UNC filename in Windows, encodes a host name dev and a share name null, a file path is \\dev\null.
cURL accepts only file://localhost/null or file://127.0.0.1/null as a valid file: URL with a host in Unix. Any host should be accepted in Windows.

[bagder]

Hm, wrong of me, the URL parser fails (and should fail) on the URL already without your PR... The test case assumes it!

[bagder]

You should not disable test 2080 on Windows. You're just hiding a problem as it should work on there too.

[sergio-nsk]

Hm, wrong of me, the URL parser fails (and should fail) on the URL already without your PR... The test case assumes it!

Right, although file://dev/null is a syntactically valid URL, it is useless in Unix with a host name dev, thus the parser returns CURLE_URL_MALFORMAT and the test ensures this. But the path \\dev\null is valid in Windows and the parser returns CURLE_OK in Windows in my PR.

[sergio-nsk]

You should not disable test 2080 on Windows. You're just hiding a problem as it should work on there too.

Is it possible to make a conditional errorcode?

<errorcode>
3
</errorcode>

I can add a test data test7366 for Windows based on test 2080 with

<errorcode>
0
</errorcode>

[bagder]

You should rather modify the URL so that it fails the same independently of operating system!

[sergio-nsk]

I will look what URL in the test can be suitable. I am afraid, a new URL will get a different code flow than expected with using file://dev/null in the test.

[bagder]

Why? You just have to make the URL parse fail consistently on all platforms. How about perhaps adding a white space? file://de v/null"

[sergio-nsk]

Good idea! Whitespaces may be disallowed in file-host names in UNC path, so file://de v/null should work for the test case. I will check it.

[bagder]

Lovely!

How about adding a few tests of this to lib1560.c as well within #ifdef win32 ? We need to make sure that the basics work (and keep working in the future) and that some obvious bad formats are rejected.

[sergio-nsk]

I will do it soon.

Regular DOS or Windows file URIs with vertical line characters in the drive letter construct. For example:

• file:///c|/path/to/file
• file:/c|/path/to/file
• file:c|/path/to/file

@bagder, would you like cURL to support such URIs? The top web browsers support such file: URIs.

[bagder]

@bagder, would you like cURL to support such URIs? The top web browsers support such file: URIs.

To my knowledge users haven't asked for this - I can't recall anyone at least. Neither UNC paths nor support for | in the file URLs, so this is not something we must have... It also seems sensible to do the | handling in a separate PR.

[bagder]

Thanks!