Skip to content

DART-247 Modify rule S4830: Add Dart language #4978

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

Merged
merged 2 commits into from
May 20, 2025

Conversation

antonioaversa
Copy link
Contributor

@antonioaversa antonioaversa commented Apr 29, 2025

DART-247

Review

A dedicated reviewer checked the rule description successfully for:

  • logical errors and incorrect information
  • information gaps and missing content
  • text style and tone
  • PR summary and labels follow the guidelines

@antonioaversa antonioaversa force-pushed the antonio/DART-247-S4830-add-dart-language branch from 49dd5e5 to 3b51ef3 Compare April 29, 2025 12:49
@antonioaversa antonioaversa marked this pull request as ready for review April 29, 2025 13:25
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM. Have a look at the improvement suggestion I made before merging.

Comment on lines 5 to 9
include::../../common/fix/code-rationale.adoc[]

:cert_method_name: badCertificateCallback

include::../../common/fix/code-rationale-override.adoc[]

Choose a reason for hiding this comment

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

This part could be improved:

Suggested change
include::../../common/fix/code-rationale.adoc[]
:cert_method_name: badCertificateCallback
include::../../common/fix/code-rationale-override.adoc[]
The certificate validation gets disabled by implementing a `badCertificateCallback` that always return `true`. It is highly recommended to use the original implementation and to avoid any bypass.

Copy link
Contributor Author

@antonioaversa antonioaversa Apr 30, 2025

Choose a reason for hiding this comment

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

Indeed, "empty" is not exact as the callback actually has a return true, at very least, in the case of Dart.

Unlike in the suggestion above, I have kept the include::../../common/fix/code-rationale.adoc[] (The following code contains examples of disabled certificate validation.), since it still applies to Dart as to any other language, and also to keep the languages as aligned as possible.

Copy link

Quality Gate passed Quality Gate passed for 'rspec-tools'

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

Copy link

Quality Gate passed Quality Gate passed for 'rspec-frontend'

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

@antonioaversa antonioaversa added this pull request to the merge queue May 20, 2025
Merged via the queue into master with commit 726de51 May 20, 2025
10 of 11 checks passed
@antonioaversa antonioaversa deleted the antonio/DART-247-S4830-add-dart-language branch May 20, 2025 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants