Fix crash on 0.4.10 when the file name contains a colon #556
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ompiler output from 0.4.10
….11-0.4.19 in tests
cameel
force-pushed
the
handle-colons-in-file-names-on-0.4.10
branch
from
bf9d463 to
7c78d4c
Compare
|
Do you think #242 is related? |
axic
reviewed
Oct 12, 2021
| @@ -78,7 +78,7 @@ function translateJsonCompilerOutput (output, libraries) { | |||
| ret['contracts'] = {}; | |||
| for (var contract in output['contracts']) { | |||
| // Split name first, can be `contract`, `:contract` or `filename:contract` | |||
| var tmp = contract.match(/^(([^:]*):)?([^:]+)$/); | |||
| var tmp = contract.match(/^((.*):)?([^:]+)$/); | |||
There was a problem hiding this comment.
So the change is to find the last colon, right?
There was a problem hiding this comment.
Exactly. The assumption is that the contract name itself can never contain a colon so the last one must be the one separating it from the file name.
Well, it's the same kind of issue but in a different situation (input rather than output). We could finish that PR but we should reproduce it first - it does not link to an issue and there's no failing test case there so hard to say if the issue is real and which versions it affects. |
chriseth
approved these changes
Oct 13, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #555.
We provide output translation on older versions (<= 0.4.10) that did not support Standard JSON. The translation uses a regex that assumes that the file name never contains a colon. On <= 0.4.9 the name is empty so it's ok but on 0.4.10 this causes a crash when compiling contracts with a colon in the file name.
Also, it looks like the
contractsdict in standard JSON had incorrectly split names until 0.4.20. On 0.4.11-0.4.19 apparently the compiler joins the file name and contract name into<filename>:<contractName>and then (incorrectly) splits it again on the first colon. This results in a wrong split when there are multiple colons in the file name. This PR fixes tests to be able to handle that. It would probably be better to make this fix on a deeper level and make solc-js return correct Standard JSON for users too.