Skip to content

Fix range IndexError when processing the radas signing json#389

Open
yma955 wants to merge 2 commits intoCommonjava:mainfrom
yma955:main
Open

Fix range IndexError when processing the radas signing json#389
yma955 wants to merge 2 commits intoCommonjava:mainfrom
yma955:main

Conversation

@yma955
Copy link
Member

@yma955 yma955 commented Feb 27, 2026

No description provided.

@yma955
Copy link
Member Author

yma955 commented Feb 27, 2026

Coveralls is down for maintenance, check will pass after its back.

logger.warning("Invalid entry: %s, skip signature file generation.", file_path)
return
# remove the root path maven-repository, valid file_path: maven-repository/...
filename = parts[1]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should refine this logic. We can not predict that the radas signing path is starting with a $root(default is "maven-repository"), instead we should do some checking as below:

  • use $root from the "handle_maven_upload" in maven.py instead of the fixed "maven-repository" here
  • we should also check if the path is starting with this $root, and then decide how to do this path handling.

Copy link
Member Author

@yma955 yma955 Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ligangty here is just a statement, I didn't predict radas result with fixed prefix "maven-repository" in code, any other root name would also be ok, and in following I use top_level to adapt the root cases.

To extend, we may just think about the result case of file path which not starts with(or contains) root, maybe we could join the top_level with the file path directly without any root name removal, WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds reasonable. If the $root does not exist in the result path, we should directly handle it. But if it exists, it should be as the prefix and we should remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants