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

Enhance input validation in ShieldsIo class #215

Merged
merged 5 commits into from
Aug 28, 2024
Merged

Conversation

guibranco
Copy link
Owner

@guibranco guibranco commented Aug 28, 2024

Description

  • Improved input validation in the ShieldsIo class to handle null values.
  • Added a return for empty input in the encodeShieldsIoParameters method.
  • Updated badge generation logic to check for null instead of empty values.

Changes walkthrough 📝

Relevant files
Enhancement
ShieldsIo.php
Enhance input validation in ShieldsIo class                           

src/ShieldsIo.php

  • Added a check for empty input in encodeShieldsIoParameters.
  • Updated conditions to check for null instead of using empty.
  • Improved parameter validation for badge generation.
  • +10/-6   

    @guibranco guibranco enabled auto-merge (squash) August 28, 2024 22:32
    @gstraccini gstraccini bot added the ☑️ auto-merge Auto-merge enabled by gstraccini-bot label Aug 28, 2024
    @penify-dev penify-dev bot added the enhancement New feature or request label Aug 28, 2024
    This commit fixes the style issues introduced in fdd8858 according to the output
    from PHP CS Fixer.
    
    Details: #215
    @penify-dev penify-dev bot changed the title Update ShieldsIo.php Enhance input validation in ShieldsIo class Aug 28, 2024
    Copy link
    Contributor

    penify-dev bot commented Aug 28, 2024

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    2, because the changes are straightforward and primarily involve input validation improvements without introducing complex logic.

    🧪 Relevant tests

    No

    ⚡ Possible issues

    No

    🔒 Security concerns

    No

    Copy link

    deepsource-io bot commented Aug 28, 2024

    Here's the code health analysis summary for commits 8a380b6..9570a59. View details on DeepSource ↗.

    Analysis Summary

    AnalyzerStatusSummaryLink
    DeepSource Test coverage LogoTest coverage✅ SuccessView Check ↗
    DeepSource SQL LogoSQL✅ SuccessView Check ↗
    DeepSource Secrets LogoSecrets✅ SuccessView Check ↗
    DeepSource PHP LogoPHP✅ SuccessView Check ↗
    DeepSource Docker LogoDocker✅ SuccessView Check ↗

    Code Coverage Report

    MetricAggregatePhp
    Branch Coverage100%100%
    Composite Coverage95.7% (down 0.1% from main)95.7% (down 0.1% from main)
    Line Coverage95.7% (down 0.1% from main)95.7% (down 0.1% from main)

    💡 If you’re a repository administrator, you can configure the quality gates from the settings.

    Copy link
    Contributor

    penify-dev bot commented Aug 28, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Add type checking to prevent errors when non-string inputs are passed

    Ensure that the encodeShieldsIoParameters function handles cases where the input might not
    be a string, to avoid potential type errors.

    src/ShieldsIo.php [13]

    +if (!is_string($input)) {
    +    return '';
    +}
     $input = str_replace("_", "__", $input);
     
    Suggestion importance[1-10]: 9

    Why: This suggestion addresses a potential bug by adding type checking, which is crucial for preventing runtime errors when non-string inputs are passed.

    9
    Performance
    Reduce redundancy by consolidating parameter checks into a loop

    Consider using a single check for all parameters in generateBadgeUrl to reduce redundancy
    and improve readability.

    src/ShieldsIo.php [26-50]

    -if (isset($label) && $label !== null) {
    -if (isset($content) && $content !== null) {
    -if (isset($color) && $color !== null) {
    -if (isset($style) && $style !== null) {
    -if (isset($labelColor) && $labelColor !== null) {
    -if (isset($logo) && $logo !== null) {
    +foreach (['label', 'content', 'color', 'style', 'labelColor', 'logo'] as $param) {
    +    if (!empty($$param)) {
    +        $badge[] = $$param;
    +    }
    +}
     
    Suggestion importance[1-10]: 8

    Why: This suggestion significantly enhances code readability and reduces redundancy, making it a valuable improvement to the code structure.

    8
    Possible issue
    Ensure consistent return type for empty input in the parameter encoding function

    The check for empty input in the encodeShieldsIoParameters function should return an empty
    string instead of returning the input directly, to ensure consistent output type.

    src/ShieldsIo.php [9-11]

     if (empty($input)) {
    -    return $input;
    +    return '';
     }
     
    Suggestion importance[1-10]: 7

    Why: While returning an empty string can ensure consistent output type, the current implementation may be acceptable depending on the context of usage. This suggestion improves clarity but isn't critical.

    7
    Maintainability
    Simplify null checks by using a more concise condition

    The checks for null values in the generateBadgeUrl function could be simplified by using
    the !empty() function, which checks for both null and empty string values.

    src/ShieldsIo.php [26]

    -if (isset($label) && $label !== null) {
    +if (!empty($label) && isset($label)) {
     
    Suggestion importance[1-10]: 6

    Why: This suggestion improves code readability and maintainability by simplifying the checks, but it does not address a major issue.

    6

    src/ShieldsIo.php Fixed Show fixed Hide fixed
    src/ShieldsIo.php Fixed Show fixed Hide fixed
    src/ShieldsIo.php Fixed Show fixed Hide fixed
    src/ShieldsIo.php Fixed Show fixed Hide fixed
    src/ShieldsIo.php Fixed Show fixed Hide fixed
    src/ShieldsIo.php Fixed Show fixed Hide fixed
    src/ShieldsIo.php Fixed Show fixed Hide fixed
    @github-actions github-actions bot added size/M and removed size/S labels Aug 28, 2024
    Copy link

    codacy-production bot commented Aug 28, 2024

    Coverage summary from Codacy

    See diff coverage on Codacy

    Coverage variation Diff coverage
    -0.05% (target: -1.00%) 100.00%
    Coverage variation details
    Coverable lines Covered lines Coverage
    Common ancestor commit (8a380b6) 236 226 95.76%
    Head commit (9570a59) 233 (-3) 223 (-3) 95.71% (-0.05%)

    Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

    Diff coverage details
    Coverable lines Covered lines Diff coverage
    Pull request (#215) 13 13 100.00%

    Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

    See your quality gate settings    Change summary preferences

    Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

    Copy link

    codecov bot commented Aug 28, 2024

    Codecov Report

    All modified and coverable lines are covered by tests ✅

    Project coverage is 95.59%. Comparing base (8a380b6) to head (9570a59).
    Report is 1 commits behind head on main.

    Additional details and impacted files
    @@            Coverage Diff             @@
    ##             main     #215      +/-   ##
    ==========================================
    - Coverage   95.65%   95.59%   -0.06%     
    ==========================================
      Files           8        8              
      Lines         230      227       -3     
    ==========================================
    - Hits          220      217       -3     
      Misses         10       10              

    ☔ View full report in Codecov by Sentry.
    📢 Have feedback on the report? Share it here.

    @@ -15,36 +15,32 @@
    return $input;
    }

    public function generateBadgeUrl($label, $content, $color, $style, $labelColor, $logo)
    private function addComponent($component, &$badge)

    Check warning

    Code scanning / Phpcs (reported by Codacy)

    Missing doc comment for function addComponent() Warning

    Missing doc comment for function addComponent()
    src/ShieldsIo.php Fixed Show fixed Hide fixed
    src/ShieldsIo.php Fixed Show fixed Hide fixed
    }
    }

    Check warning

    Code scanning / Phpcs (reported by Codacy)

    Expected //end addComponent() Warning

    Expected //end addComponent()
    }
    }

    Check warning

    Code scanning / Phpcs (reported by Codacy)

    Expected 1 blank line before closing function brace; 0 found Warning

    Expected 1 blank line before closing function brace; 0 found
    }
    }

    Check warning

    Code scanning / Phpcs (reported by Codacy)

    Expected 1 blank line before closing function brace; 0 found Warning

    Expected 1 blank line before closing function brace; 0 found
    }
    }

    Check warning

    Code scanning / Phpcs (reported by Codacy)

    Expected 2 blank lines after function; 1 found Warning

    Expected 2 blank lines after function; 1 found
    }
    }

    Check warning

    Code scanning / Phpcs (reported by Codacy)

    Expected //end addQueryString() Warning

    Expected //end addQueryString()
    if (isset($color) && !empty($color)) {
    $badge[] = $color;
    }
    public function generateBadgeUrl($label, $content, $color, $style, $labelColor, $logo)

    Check warning

    Code scanning / Phpcs (reported by Codacy)

    Missing doc comment for function generateBadgeUrl() Warning

    Missing doc comment for function generateBadgeUrl()
    src/ShieldsIo.php Fixed Show fixed Hide fixed
    Copy link
    Contributor

    Infisical secrets check: ✅ No secrets leaked!

    Scan results:

    11:01PM INF scanning for exposed secrets...
    11:01PM INF 150 commits scanned.
    11:01PM INF scan completed in 412ms
    11:01PM INF no leaks found
    
    

    Copy link

    sonarcloud bot commented Aug 28, 2024

    @guibranco guibranco merged commit 1f8a079 into main Aug 28, 2024
    28 checks passed
    @guibranco guibranco deleted the guibranco-patch-1 branch August 28, 2024 23:02
    $queryString = array();
    if (isset($style) && !empty($style)) {
    $queryString["style"] = $style;
    if (isset($component) && (empty($component) === false || $component === 0)) {

    Check warning

    Code scanning / Phpcs (reported by Codacy)

    Implicit true comparisons prohibited; use === TRUE instead Warning

    Implicit true comparisons prohibited; use === TRUE instead
    $queryString["labelColor"] = $labelColor;
    private function addQueryString($component, $key, &$queryString)
    {
    if (isset($component) && empty($component) === false) {

    Check warning

    Code scanning / Phpcs (reported by Codacy)

    Implicit true comparisons prohibited; use === TRUE instead Warning

    Implicit true comparisons prohibited; use === TRUE instead
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    ☑️ auto-merge Auto-merge enabled by gstraccini-bot enhancement New feature or request Review effort [1-5]: 2 size/M
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant