Fix incorrectly merged 7z & use more metadata#90
Conversation
Summary of ChangesHello @matthew-pye, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the RomM plugin by adding support for 7z archive extraction and incorporating additional metadata from the RomM server to enrich game information. It also addresses issues related to filename handling and file type detection to ensure proper game installation and metadata display. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request successfully refactors the 7z extraction logic into DownloadQueueController, enhances metadata integration from the RomM server, and improves filename handling for single ROMs. However, it introduces significant security risks, including a critical path traversal vulnerability in RomM.cs due to unsanitized filenames from the RomM API, potentially allowing arbitrary file writes. A command injection vulnerability also exists in the new 7z extraction logic in DownloadQueueController.cs due to unescaped double quotes in process arguments. Addressing these by consistently applying Path.GetFileName() to external filenames and properly escaping process arguments is crucial, alongside resolving other identified robustness concerns and minor user experience regressions.
2 participants
Remerge 7z for new download system
When 7z was merged I did the pull request before the new download system was merged so the code was placed in the wrong area and broken the code
New Metadata
Used more of the metadata from the RomM server & makes sure that the filename for single file ROMs has an extension & adds fail-safe for when HasSimpleSingleFile , HasNestedSingleFile & HasMultipleFiles are all false
Old metadata:
New metadata:
Build
Here is a debug build with all the additions that have been added, I have tested it on my Playnite and with a fresh install in sandbox and everything seems to work, the only issue with it is that the sharpcompress.dll isn't packed with the extension but this might be because I didn't pack it right
RomM_9700aa21-447d-41b4-a989-acd38f407d9f_0_4_1.zip