absolutePath was intended to be used on file names, maybe we should check that it does not end in / instead?

Ending with / does not seem to be the problem but containing at least 4 / only and nothing else seems to be.

Examples

////
//////
/////////

(Edit: For the sequences listed above, the parsed filename is always "/". Alternatively, we could fix the call sites by checking if the absolutePath contains at least 4 and only / but that is a little more elaborate)

I actually fuzzed the remove_filename API with random path strings and it turns out the fix is rather robust at least for ASCII input sequences. Not sure what the underlying root cause is.

I agree with @chriseth that requiring _path to always contain a file name would be a nice way to side-step the issue.

Looks like the only place where we use absolutePath() is in CompilerStack::loadMissingSources(). Currently it tries to load the file listed in an import without checking if it's actually a file but for directories it fails anyway:

Error: Source "/tmp/xyz" not found: Not a valid file.
 --> /tmp/test.sol:1:1:
  |
1 | import "./xyz/";
  | ^^^^^^^^^^^^^^^^

We could make it fail immediately if the path ends in the OS-specific directory separator and then absolutePath() could just assert that _path contains a file name as a precondition.

We could make it fail immediately if the path ends in the OS-specific directory separator

The following test currently passes. Are you suggesting that it should fail instead because import string ends with a dir separator?

==== Source: a/b/c.sol ====
contract C {}
==== Source: a/b/d.sol ====
import "./c.sol/";
contract D is C {}

boost::fs is surprisingly robust except for the //// corner case. For example, the following test also passes

==== Source: a/b/c.sol ====
contract C {}
==== Source: a/b/d.sol ====
import ".//c.sol////";
contract D is C {}

Well, yeah, I think this should be an error. To me it looks as if c.sol/ is a directory and if I saw this in someone's code I'd immediately start wondering what's going on, what does it mean to import a whole directory :)

Though the fact that it works right now complicates things. This makes it a breaking change so we still need some other workaround for 0.8.x. The one you did in this PR is good enough in my opinion. But let's see what @chriseth says.

boost::fs is surprisingly robust except for the //// corner case.

I think this has something to do with root names. For example in //a/b/c, the path is actually /b/c and //a is the root name. So it has all kinds of special cases for // at the beginning of the path. This is even more relevant on Windows where something like C: or D: could be a root name. See for example the table under boost:filesystem::absolute(). The only reason it needs a table is the existence of root names :)

Well, yeah, I think this should be an error. To me it looks as if c.sol/ is a directory and if I saw this in someone's code I'd immediately start wondering what's going on, what does it mean to import a whole directory :)

True, that would be a red flag for sure.

Though the fact that it works right now complicates things. This makes it a breaking change so we still need some other workaround for 0.8.x. The one you did in this PR is good enough in my opinion. But let's see what @chriseth says.

Agreed, I was also mainly concerned of unknowingly breaking something by disallowing certain (albeit weird) forms of import path strings.

boost::fs is surprisingly robust except for the //// corner case.

I think this has something to do with root names. For example in //a/b/c, the path is actually /b/c and //a is the root name. So it has all kinds of special cases for // at the beginning of the path. This is even more relevant on Windows where something like C: or D: could be a root name. See for example the table under boost:filesystem::absolute(). The only reason it needs a table is the existence of root names :)

Interesting. Today I learnt :-)

I have created an issue for disallowing the trailing slash (#10980) and added it to the design backlog so that in this PR we can focus on doing just a basic, non-breaking fix.