Skip to content
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

ISY-538: Behebung von Security Issues in isyfact-standards/release/2.x #270

Open
wants to merge 6 commits into
base: release/2.x
Choose a base branch
from

Conversation

huy-tran-msg
Copy link
Contributor

Fix 3 securities issues suggested by Sonarcloud by:

  • Disable access to external entities in XML parsing
  • Handle the exception that could be thrown by "getWriter": IOException

@huy-tran-msg huy-tran-msg changed the base branch from master to release/2.x February 12, 2024 09:15
@huy-tran-msg huy-tran-msg self-assigned this Feb 12, 2024
@huy-tran-msg huy-tran-msg force-pushed the feature/ISY-538-Behebung_von_Security_Issues branch 3 times, most recently from 7462464 to 9e5e93a Compare February 14, 2024 07:40
@BerghoffP BerghoffP added the medium Issues/PRs with medium priority must be processed before operations with low label label Mar 6, 2024
@huy-tran-msg huy-tran-msg force-pushed the feature/ISY-538-Behebung_von_Security_Issues branch from 9e5e93a to 2cad8e5 Compare April 19, 2024 06:32
Copy link

sonarcloud bot commented Apr 24, 2024

@huy-tran-msg huy-tran-msg marked this pull request as draft April 24, 2024 09:42
@evcimene evcimene marked this pull request as ready for review April 26, 2024 06:48
@IsyFact IsyFact deleted a comment from sonarcloud bot May 8, 2024
Copy link
Contributor

@FabianPerl FabianPerl left a comment

Choose a reason for hiding this comment

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

Laut Definition of Done:

Bei Anpassungen am Quellcode werden alle Kommentare der angepassten Datei in das Englische übersetzt

@@ -44,7 +47,7 @@
*
*/
public class BatchProtokollTester {

Logger logger = LoggerFactory.getLogger(BatchProtokollTester.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

Gibt es einen technischen Grund warum hier nicht isy-logging eingesetzt wird?

* Neuer Wert für xpath
*/
public void setXpath(XPath xpath) {
this.xpath = xpath;
Copy link
Contributor

Choose a reason for hiding this comment

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

Durch das Entfernen der zwei public-Methoden in der HelperKlasse BatchProtokollTester kommt es zu einem Breaking Change in etwaigen Anwendungen/Querschnittsanwendungen (auch wenn die Standards davon nicht betroffen sind)!

Copy link
Contributor

@FabianPerl FabianPerl left a comment

Choose a reason for hiding this comment

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

Bitte noch in den Commit-Messages die Ticket-Nummer erwähnen (gemäß Conventional-Commit-Guideline).

Auch noch die Merge-Konflikte beheben

Copy link
Contributor

@FabianPerl FabianPerl left a comment

Choose a reason for hiding this comment

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

In der IsyFact 2.x wird die Versionierung noch nicht berechnet. Muss hier nicht auf 2.5.1 angehoben werden?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium Issues/PRs with medium priority must be processed before operations with low label
Development

Successfully merging this pull request may close these issues.

3 participants