Skip to content

Handle : characters in metadata library paths #2043

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

Open
manuelwedler opened this issue Mar 31, 2025 · 3 comments · May be fixed by #2143
Open

Handle : characters in metadata library paths #2043

manuelwedler opened this issue Mar 31, 2025 · 3 comments · May be fixed by #2143
Assignees
Labels
🪲 bug Something isn't working

Comments

@manuelwedler
Copy link
Contributor

I found an issue in the way the SolidityMetadataContract handles libraries. It is about this line:

const [contractPath, contractName] = libraryKey.split(':');

Here, we assume that the result of split is always a two element array. But actually the path can contain : characters itself. I know about this because the default test contract in the server tests has a : in the path.

@manuelwedler manuelwedler added the 🪲 bug Something isn't working label Apr 1, 2025
@kuzdogan kuzdogan moved this from Triage to Sprint - Candidates in Sourcify Public Apr 14, 2025
@kuzdogan
Copy link
Member

kuzdogan commented Apr 22, 2025

Just thought, isn't this and other related parts are affected from this?

const splitIdentifier = req.body.contractIdentifier.split(":");
if (splitIdentifier.length < 2) {

@manuelwedler
Copy link
Contributor Author

The code in the middleware is correct because we require it to be AT LEAST 2 parts when splitting.

In the verification handler we do

  const splitIdentifier = req.body.contractIdentifier.split(":");
  const contractName = splitIdentifier[splitIdentifier.length - 1];
  const contractPath = splitIdentifier.slice(0, -1).join(":");

This is the correct way to handle it.

@manuelwedler
Copy link
Contributor Author

There is another occurrence that is also wrong:

@kuzdogan kuzdogan moved this from Sprint - Candidates to Sprint - Needs Review in Sourcify Public Apr 28, 2025
@kuzdogan kuzdogan moved this from Sprint - Needs Review to Sprint - Up Next in Sourcify Public Apr 28, 2025
@manuelwedler manuelwedler self-assigned this Apr 28, 2025
@manuelwedler manuelwedler moved this from Sprint - Up Next to Sprint - In Progress in Sourcify Public May 6, 2025
@manuelwedler manuelwedler moved this from Sprint - In Progress to Sprint - Up Next in Sourcify Public May 6, 2025
@manuelwedler manuelwedler moved this from Sprint - Up Next to Sprint - In Progress in Sourcify Public May 7, 2025
@manuelwedler manuelwedler linked a pull request May 7, 2025 that will close this issue
@manuelwedler manuelwedler moved this from Sprint - In Progress to Sprint - Needs Review in Sourcify Public May 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪲 bug Something isn't working
Projects
Status: Sprint - Needs Review
Development

Successfully merging a pull request may close this issue.

2 participants