Skip to content

C# Files need to be UTF-8#58

Merged
MichielOda merged 8 commits into
mainfrom
Utf8Banner
Jun 21, 2024
Merged

C# Files need to be UTF-8#58
MichielOda merged 8 commits into
mainfrom
Utf8Banner

Conversation

@MichielOda
Copy link
Copy Markdown
Member

New Error Message

[3.40.1] InvalidFileEncoding

All files are read out as UTF-8 throughout the CI/CD process. If QActions would contain files that are not UTF-8 due to special characters, it could be converted to invalid characters and break your code.

@github-advanced-security
Copy link
Copy Markdown

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

csharpFile is string filePath &&
FileSystem.Instance.File.Exists(filePath))
{
string tempPath = Path.GetTempFileName();

Check failure

Code scanning / SonarCloud

Insecure temporary file creation methods should not be used

<!--SONAR_ISSUE_KEY:AY_iLe6jU-3kdPApRSFT-->'Path.GetTempFileName()' is insecure. Use 'Path.GetRandomFileName()' instead. <p>See more on <a href="https://sonarcloud.io/project/issues?id=SkylineCommunications_Skyline.DataMiner.CICD.Validators&issues=AY_iLe6jU-3kdPApRSFT&open=AY_iLe6jU-3kdPApRSFT&branch=Utf8Banner">SonarCloud</a></p>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 4, 2024

@MichielOda
Copy link
Copy Markdown
Member Author

Copy link
Copy Markdown
Member

@PedroDebevere PedroDebevere left a comment

Choose a reason for hiding this comment

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

  • see SonarCloud remark on Path.GetTempFileName() use
  • Would using the Parsers package to parse the cs files in the projects be a better alternative? (to be verified if that supports shared projects).
  • Is the encoding check for the protocol XML file itself also going to become a validator check?

@MichielOda
Copy link
Copy Markdown
Member Author

  • see SonarCloud remark on Path.GetTempFileName() use
  • Would using the Parsers package to parse the cs files in the projects be a better alternative? (to be verified if that supports shared projects).
  • Is the encoding check for the protocol XML file itself also going to become a validator check?

SonarCloud has been fixed before, but the UI on GitHub doesn't want to update that somehow.
Not sure why I didn't use the Parsers. Probably didn't thought about it.
Missed that in the acceptance criteria, will do that as well

@MichielOda
Copy link
Copy Markdown
Member Author

Validator check for the protocol xml file itself won't be possible without breaking changes. Currently there is no way to know the filepath of the xml file as we only receive the content. As in DIS there is already an info banner that deals with this, I would not include this in this PR. If needed a new task can be made for this breaking change.

@MichielOda MichielOda requested a review from PedroDebevere June 20, 2024 11:59
Comment thread Protocol/Tests/Protocol/QActions/QAction/CheckFileEncoding.cs Outdated
@MichielOda MichielOda requested a review from PedroDebevere June 21, 2024 07:00
@MichielOda MichielOda merged commit 245a222 into main Jun 21, 2024
@MichielOda MichielOda deleted the Utf8Banner branch June 21, 2024 11:07
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.

3 participants