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

🐛 Add Escaped Double Quotes to comment #82

Merged

Conversation

keunhyung-chung
Copy link
Contributor

@keunhyung-chung keunhyung-chung commented Aug 30, 2024

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

Even if the PR size is over l_max_size, this actions fails to comment to the PR with the following message:

image


This is because #50 changed the sanitiation logic of arguments:

image


For example, before #50

$ a="--message_if_xl=\"This PR exceeds the recommended size of 1000 lines (additions + deletions). Please make sure you are NOT addressing multiple issues with one PR and split the PR if necessary.\""
$ echo "$a" | tr -d '\n'| sed "s/'//g"| sed "s/’//g"
--message_if_xl="This PR exceeds the recommended size of 1000 lines (additions + deletions). Please make sure you are NOT addressing multiple issues with one PR and split the PR if necessary."

the argument contains ".

However, after #50

$ echo "$a" | tr '\n' ' ' | xargs echo | sed "s/'//g"| sed "s/’//g"
--message_if_xl=This PR exceeds the recommended size of 1000 lines (additions + deletions). Please make sure you are NOT addressing multiple issues with one PR and split the PR if necessary.

the argument loose ".

How to test

Without Quotes

  • Before this PR
  • Expected failure
$ curl -L \
  -X POST \
  -H "Accept: application/vnd.github+json" \
  -H "Authorization: Bearer $GITHUB_TOKEN" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  https://api.github.com/repos/CodelyTV/pr-size-labeler/issues/82/comments \
  -d "{\"body\":hello}"
{
  "message": "Problems parsing JSON",
  "documentation_url": "https://docs.github.com/rest/issues/comments#create-an-issue-comment",
  "status": "400"
}

With Quotes

  • After this PR
$ curl -L \
  -X POST \
  -H "Accept: application/vnd.github+json" \
  -H "Authorization: Bearer $GITHUB_TOKEN" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  https://api.github.com/repos/CodelyTV/pr-size-labeler/issues/82/comments \
  -d "{\"body\":\"hello\"}"
{
  "url": "https://api.github.com/repos/CodelyTV/pr-size-labeler/issues/comments/2320573142",
  "html_url": "https://github.com/CodelyTV/pr-size-labeler/pull/82#issuecomment-2320573142",
  "issue_url": "https://api.github.com/repos/CodelyTV/pr-size-labeler/issues/82",
  "id": 2320573142,
  "node_id": "IC_kwDODoQbjM6KUSLW",
  "user": {
    "login": "keunhyung-chung",
    "id": 12574208,
    "node_id": "MDQ6VXNlcjEyNTc0MjA4",
    "avatar_url": "https://avatars.githubusercontent.com/u/12574208?u=3e6202a7d076a402f4de01c29f9f8aff06b3a84b&v=4",
    "gravatar_id": "",
    "url": "https://api.github.com/users/keunhyung-chung",
    "html_url": "https://github.com/keunhyung-chung",
    "followers_url": "https://api.github.com/users/keunhyung-chung/followers",
    "following_url": "https://api.github.com/users/keunhyung-chung/following{/other_user}",
    "gists_url": "https://api.github.com/users/keunhyung-chung/gists{/gist_id}",
    "starred_url": "https://api.github.com/users/keunhyung-chung/starred{/owner}{/repo}",
    "subscriptions_url": "https://api.github.com/users/keunhyung-chung/subscriptions",
    "organizations_url": "https://api.github.com/users/keunhyung-chung/orgs",
    "repos_url": "https://api.github.com/users/keunhyung-chung/repos",
    "events_url": "https://api.github.com/users/keunhyung-chung/events{/privacy}",
    "received_events_url": "https://api.github.com/users/keunhyung-chung/received_events",
    "type": "User",
    "site_admin": false
  },
  "created_at": "2024-08-30T09:05:07Z",
  "updated_at": "2024-08-30T09:05:07Z",
  "author_association": "NONE",
  "body": "hello",
  "reactions": {
    "url": "https://api.github.com/repos/CodelyTV/pr-size-labeler/issues/comments/2320573142/reactions",
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
  },
  "performed_via_github_app": null
}

@keunhyung-chung keunhyung-chung changed the title Add double quotes to comment Add Double Quotes to comment Aug 30, 2024
@keunhyung-chung keunhyung-chung changed the title Add Double Quotes to comment Add Escaped Double Quotes to comment Aug 30, 2024
@keunhyung-chung keunhyung-chung changed the title Add Escaped Double Quotes to comment 🐛 Add Escaped Double Quotes to comment Aug 30, 2024
@keunhyung-chung
Copy link
Contributor Author

hello

@keunhyung-chung keunhyung-chung marked this pull request as ready for review August 30, 2024 09:09
Copy link
Member

@JavierCane JavierCane left a comment

Choose a reason for hiding this comment

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

Good one. Thanks @keunhyung-chung!

@JavierCane JavierCane merged commit c7a55a0 into CodelyTV:main Sep 2, 2024
2 checks passed
@keunhyung-chung keunhyung-chung deleted the fix/github-comment/body/json branch September 9, 2024 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants