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

feat: build test and sonarqube issues fix. #111

Merged
merged 4 commits into from
Oct 16, 2024
Merged

feat: build test and sonarqube issues fix. #111

merged 4 commits into from
Oct 16, 2024

Conversation

azhard4int
Copy link
Contributor

@azhard4int azhard4int commented Oct 16, 2024

PR Type

Bug fix, Enhancement


Description

  • Reduced retry interval for sending attempts from 500ms to 200ms in client.ts.
  • Improved HTML tag removal with a non-greedy regex in autocapture.ts.
  • Added a custom trim function for environments without native support in form-tracking.ts.
  • Enhanced generateRandom function to use cryptographic randomness in common.ts.
  • Updated domain extraction regex to be non-backtracking in cookie.ts.
  • Refactored generateId to use the new generateRandom function in helpers.ts.
  • Updated Dockerfile path and build context in the CI workflow.
  • Changed paths for nginx configuration files in the Dockerfile.

Changes walkthrough 📝

Relevant files
Enhancement
client.ts
Reduce retry interval for sending attempts                             

packages/javascript-sdk/src/core/client.ts

  • Reduced retry interval from 500ms to 200ms.
+1/-1     
form-tracking.ts
Add custom trim function for compatibility                             

packages/javascript-sdk/src/tracking/form-tracking.ts

  • Implemented custom trim function for environments without native
    support.
  • +24/-1   
    common.ts
    Use cryptographic randomness in generateRandom function   

    packages/javascript-sdk/src/utils/common.ts

  • Enhanced generateRandom function to use cryptographic randomness.
  • +6/-2     
    helpers.ts
    Refactor generateId to use cryptographic randomness           

    packages/javascript-sdk/src/utils/helpers.ts

    • Refactored generateId to use the new generateRandom function.
    +2/-1     
    Bug fix
    autocapture.ts
    Improve HTML tag removal with non-greedy regex                     

    packages/javascript-sdk/src/tracking/autocapture.ts

    • Updated regex for HTML tag removal to be non-greedy.
    +1/-1     
    cookie.ts
    Improve domain extraction with non-backtracking regex       

    packages/javascript-sdk/src/utils/cookie.ts

    • Updated domain extraction regex to be non-backtracking.
    +4/-1     
    Configuration changes
    cd-develop.yml
    Update Dockerfile path and context in CI workflow               

    .github/workflows/cd-develop.yml

    • Updated Dockerfile path and build context for CI workflow.
    +2/-2     
    Dockerfile
    Update nginx configuration file paths                                       

    packages/javascript-sdk/docker/Dockerfile

    • Changed paths for nginx configuration files.
    +2/-2     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @azhard4int azhard4int merged commit e6da0cc into develop Oct 16, 2024
    0 of 2 checks passed
    @github-actions github-actions bot added enhancement New feature or request Bug fix labels Oct 16, 2024
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🏅 Score: 85
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Performance Issue
    The custom trim implementation might be inefficient for large strings due to its use of indexOf in a loop. Consider optimizing this method or using a more efficient approach.

    Hardcoded Value
    The retry interval is hardcoded, which might not be flexible for different environments or use cases. Consider making this configurable via the settings.

    Code feedback:
    relevant filepackages/javascript-sdk/src/core/client.ts
    suggestion      

    Consider making the retry interval configurable through this.config.retryInterval to enhance flexibility. [important]

    relevant line200 // Reduced interval to .2 second

    relevant filepackages/javascript-sdk/src/tracking/autocapture.ts
    suggestion      

    Ensure the regex for HTML tag removal handles all edge cases or consider using a library for sanitization to prevent XSS attacks. [important]

    relevant linetext = text.replace(/<[^>]*?>/g, '');

    relevant filepackages/javascript-sdk/src/tracking/form-tracking.ts
    suggestion      

    Optimize the custom trim function to avoid using indexOf in a loop, which can be inefficient for large strings. Consider using a regex-based trim or a more efficient loop mechanism. [important]

    relevant linewhile (start <= end && ws.indexOf(value[start]) > -1) {

    relevant filepackages/javascript-sdk/src/utils/common.ts
    suggestion      

    Use crypto.getRandomValues directly in generateRandom to avoid unnecessary array transformations, which can improve performance. [medium]

    relevant linereturn Array.from(array, (byte) => byte.toString(36).padStart(2, '0'))

    relevant filepackages/javascript-sdk/src/utils/cookie.ts
    suggestion      

    Verify the new regex for domain extraction against various edge cases to ensure it does not break existing functionality. [medium]

    relevant lineconst DOMAIN_MATCH_REGEX = /(?:[a-z0-9

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Bug fix enhancement New feature or request
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants