solc/wrapper crashes on Solidity version 0.4.10 and earlier when there's a colon in the file path

[haltman-at]

So, versions of Solidity prior to 0.4.20 don't handle colons in file paths well; and since Truffle not too long ago started relabelling all file paths to begin with project:/, this has become an issue for older Solidity support. For Solidity 0.4.11-19, the problem is handleable, and I've put up this Truffle PR to address it.

For Solidity 0.4.10 and earlier, however, a colon in the file path causes solc/wrapper itself to crash. (I don't know why it's only 0.4.10 and earlier, while 0.4.11-19 don't cause a problem here.) To be clear, I tried updating solc/wrapper to 0.8.9 to ensure that the problem still exists on this version.

So, solc/wrapper should be updated so that it doesn't crash on this condition, and we can at least get some output. Whether that's nice clean output in the style of 0.4.20, or messed-up output in the style of 0.4.11-19 that we have to repair... as long as we can at least get something we can work with.

(Should the 0.4.11-19 problem itself be reported as a solc/wrapper issue? Maybe that's something that could itself be fixed in solc/wrapper so that Truffle doesn't have to repair it after the fact? Not sure whether that's appropriate but I'm starting to think maybe it is; obviously it's closely related to this problem.)

(Now I'm wondering if maybe the problems Truffle has had with 0.4.8 and earlier should be solc/wrapper issues, too. :P Probably not, but...)

Edit: Forgot to include a stacktrace, here's the relevant part of the stacktrace:

TypeError: Cannot read property 'length' of null
    at Object.translateJsonCompilerOutput (/home/sniffnoy/truffle/truffle/packages/compile-solidity/node_modules/solc/translate.js:82:13)
    at translateOutput (/home/sniffnoy/truffle/truffle/packages/compile-solidity/node_modules/solc/wrapper.js:275:26)
    at Object.compileStandardWrapper [as compile] (/home/sniffnoy/truffle/truffle/packages/compile-solidity/node_modules/solc/wrapper.js:292:14)

[cameel]

Thanks for the report! I have fixed the crash on 0.4.10/0.4.9: #556. Looks like it was just a bad regex.

The reason that it fails only on 0.4.10 (EDIT: and on 0.4.9) is that it was a transitional version, just before the support for --standard-json was added in 0.4.11. Before 0.4.11 the JSON structure was different. I see that solc-js internally translates that old output into something that's compatible with Standard JSON. The thing is that it's still not 100% the same thing. For example looks like until 0.4.8 the file name was not included at all. This changed in 0.4.9 which is why the crash only affects that version.

Should the 0.4.11-19 problem itself be reported as a solc/wrapper issue?

Yeah, please report it. You probably mean the fact that the filename in the output gets split incorrectly on colons in the contracts dict and you get a part of it included in the contract name? I just had to deal with it in #556. It started happening on 0.4.11 because that's when Standard JSON was added and apparently it was fixed in 0.4.20.

I'm not sure if solc-js should be correcting that automatically (this will make it behave differently than solc --standard-json) but at the very least we could have an option that enables this correction on demand.

(Now I'm wondering if maybe the problems Truffle has had with 0.4.8 and earlier should be solc/wrapper issues, too. :P Probably not, but...)

I suspect it's rather the fact that Standard JSON interface was not available back then and the translation is not 1:1. But please report any issues you have anyway, we might add a workaround. I do think that solc-js is a good place for such compatibility fixes because then multiple tools can benefit from them.

[haltman-at]

Thanks for the report! I have fixed the crash on 0.4.10: #556. Looks like it was just a bad regex.

The reason that it fails only on 0.4.10 is that it was a transitional version, just before the support for --standard-json was added in 0.4.11. Before 0.4.11 the JSON structure was different. I see that solc-js internally translates that old output into something that's compatible with Standard JSON. The thing is that it's still not 100% the same thing. For example looks like until 0.4.9 the file name was not included at all. This changed in 0.4.10 which is why the crash only affects that version.

Oh, fascinating! I should nitpick and note that the crash didn't affect only 0.4.10, it was both 0.4.9 and 0.4.10. Does the PR actually only fix 0.4.10, or does it fix 0.4.9 as well? I'm guessing the latter?

(I didn't test 0.4.8 and earlier as we don't support those, but I would imagine that due to those versions' not including file paths at all, they wouldn't be affected by this bug in the first place.)

Should the 0.4.11-19 problem itself be reported as a solc/wrapper issue?

Yeah, please report it. You probably mean the fact that the filename in the output gets split incorrectly on colons in the contracts dict and you get a part of it included in the contract name? I just had to deal with it in #556. It started happening on 0.4.11 because that's when Standard JSON was added and apparently it was fixed in 0.4.20.

I'm not sure if solc-js should be correcting that automatically (this will make it behave differently than solc --standard-json) but at the very least we could have an option that enables this correction on demand.

Wait, so does #556 correct 0.4.11-0.4.19 as well or not? We don't need it to, we just merged our own PR for cleaning those up, but it would be good to know if we'll be able to get rid of that code later. If it doesn't then yeah I can file an issue, if it does obviously I won't bother.

(Now I'm wondering if maybe the problems Truffle has had with 0.4.8 and earlier should be solc/wrapper issues, too. :P Probably not, but...)

I suspect it's rather the fact that Standard JSON interface was not available back then and the translation is not 1:1. But please report any issues you have anyway, we might add a workaround. I do think that solc-js is a good place for such compatibility fixes because then multiple tools can benefit from them.

Yeah I suspect the problems we run into with versions 0.4.8 and earlier are probably not fixable, but I guess I'll come back later and file an issue just in case they are. :) Who knows, maybe we could get things working all the way back to 0.1.6. :P

[cameel]

Oh, fascinating! I should nitpick and note that the crash didn't affect only 0.4.10, it was both 0.4.9 and 0.4.10. Does the PR actually only fix 0.4.10, or does it fix 0.4.9 as well? I'm guessing the latter?

True. I realized just before you posted and even already managed to edit my comment above :) The PR does fix 0.4.9 too.

Wait, so does #556 correct 0.4.11-0.4.19 as well or not? We don't need it to, we just merged our own PR for cleaning those up, but it would be good to know if we'll be able to get rid of that code later. If it doesn't then yeah I can file an issue, if it does obviously I won't bother.

It does not correct these versions yet. That's a bug in the compiler rather than in solc-js so I'd like to first get an opinion from @chriseth on whether we should be adding workarounds for these here. My personal opinion is that it's better to add them than to leave it broken but we'll see. I hope you'll be able to get rid of that workaround on your side soon :)

Yeah I suspect the problems we run into with versions 0.4.8 and earlier are probably not fixable, but I guess I'll come back later and file an issue just in case they are. :) Who knows, maybe we could get things working all the way back to 0.1.6. :P

They're not fixable directly in compiler binaries but since we already have this compatibility layer, we can at least make it better handle the cases you're running into. The compatibility layer itself might have some bugs and if it does, they should be ironed out.

BTW, Hardhat ran into the same issue recently so making this compatibility layer more robust would benefit multiple projects. Reporting any problems with you can find it would definitely be helpful.

[haltman-at]

OK, put up #557 and #558 based on this discussion. :) Thanks!