8000 src: check empty before accessing string by zcbenz · Pull Request #51665 · nodejs/node · GitHub
  • [go: up one dir, main page]
    Skip to content

    src: check empty before accessing string #51665

    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.

    Sign up for GitHub

    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

    Jump to bottom
    Merged
    nodejs-github-bot merged 1 commit into from
    Feb 16, 2024
    Merged

    src: check empty before accessing string #51665

    nodejs-github-bot merged 1 commit into from
    Feb 16, 2024

    Conversation

    @zcbenz
    Copy link
    Contributor
    @zcbenz zcbenz commented Feb 5, 2024

    Fix an assertion when running dotnev tests with GN build:
    assertion !empty() failed: string::front(): string is empty

    which was caused by calling value.front() without verifying the value is not empty.

    @nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Feb 5, 2024
    @zcbenz zcbenz force-pushed the fix-str-front-assert branch from 22199ff to 7da4605 Compare February 5, 2024 01:32
    @zcbenz zcbenz marked this pull request as draft February 5, 2024 01:39
    @zcbenz
    src: check empty before accessing string
    b208e65 
    Fix an assertion when running dotnev tests with GN build:
assertion !empty() failed: string::front(): string is empty

which was caused by calling value.front() without verifying the value is
not empty.
    @zcbenz zcbenz force-pushed the fix-str-front-assert branch from 7da4605 to b208e65 Compare February 5, 2024 01:46
    @zcbenz zcbenz marked this pull request as ready for review February 5, 2024 01:48
    anonrig
    anonrig approved these changes Feb 5, 2024
    View reviewed changes
    Copy link
    Member
    @anonrig anonrig left a comment

    Choose a reason for hiding this comment

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

    Nice catch

    @anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 5, 2024
    @zcbenz
    Copy link
    Contributor Author
    zcbenz commented Feb 13, 2024

    Can this be merged? It is currently breaking GN build.

    @Flarna Flarna added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Feb 13, 2024
    @marco-ippolito
    Copy link
    Member
    marco-ippolito commented Feb 13, 2024

    @zcbenz CI is currently locked for the security release

    @Flarna Flarna mentioned this pull request Feb 14, 2024
    Closed
    @victorgomes
    Copy link
    victorgomes commented Feb 14, 2024

    Another duplicate: #51757

    @github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 14, 2024
    @nodejs-github-bot

    This comment was marked as outdated.

    Sign in to view
    @nodejs-github-bot

    This comment was marked as outdated.

    Sign in to view
    @nodejs-github-bot

    This comment was marked as outdated.

    Sign in to view
    @nodejs-github-bot
    Copy link
    Collaborator
    nodejs-github-bot commented Feb 16, 2024

    CI: https://ci.nodejs.org/job/node-test-pull-request/57128/

    @Flarna Flarna added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 16, 2024
    @nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 16, 2024
    @nodejs-github-bot nodejs-github-bot merged commit c682fbf into nodejs:main Feb 16, 2024
    @nodejs-github-bot
    Copy link
    Collaborator
    nodejs-github-bot commented Feb 16, 2024

    Landed in c682fbf

    targos pushed a commit that referenced this pull request Feb 19, 2024
    @zcbenz @targos
    src: check empty before accessing string
    2a46dc7 
    Fix an assertion when running dotnev tests with GN build:
assertion !empty() failed: string::front(): string is empty

which was caused by calling value.front() without verifying the value is
not empty.

PR-URL: #51665
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
    @marco-ippolito marco-ippolito mentioned this pull request Mar 1, 2024
    Merged
    rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Mar 20, 2024
    @zcbenz @rdw-msft
    src: check empty before accessing string
    8d3b1de 
    Fix an assertion when running dotnev tests with GN build:
assertion !empty() failed: string::front(): string is empty

which was caused by calling value.front() without verifying the value is
not empty.

PR-URL: nodejs#51665
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
    richardlau pushed a commit that referenced this pull request Mar 25, 2024
    @zcbenz @richardlau
    src: check empty before accessing string
    603e7de 
    Fix an assertion when running dotnev tests with GN build:
assertion !empty() failed: string::front(): string is empty

which was caused by calling value.front() without verifying the value is
not empty.

PR-URL: #51665
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
    richardlau pushed a commit that referenced this pull request Mar 25, 2024
    @zcbenz @richardlau
    src: check empty before accessing string
    3875fa1 
    Fix an assertion when running dotnev tests with GN build:
assertion !empty() failed: string::front(): string is empty

which was caused by calling value.front() without verifying the value is
not empty.

PR-URL: #51665
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
    @richardlau richardlau mentioned this pull request Mar 25, 2024
    Merged
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @lpinca lpinca lpinca approved these changes

    @anonrig anonrig anonrig approved these changes

    @Flarna Flarna Flarna approved these changes

    @marco-ippolito marco-ippolito marco-ippolito approved these changes

    Assignees

    No one assigned

    Labels

    c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    7 participants

    @zcbenz @marco-ippolito @victorgomes @nodejs-github-bot @lpinca @anonrig @Flarna
    0