Fix incorrect signature hash during verify return url #16
Conversation
When parsing a number string to a number, it can be divided twice. One of these divisions occurs before verifying the hash, causing the verification to fail. #15
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe updates primarily improve the documentation and fix a significant bug. The Changes
Assessment against linked issues
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Reviewer's Guide by SourceryThis pull request addresses a bug where the signature verification failed due to an incorrect amount calculation. The fix involves removing the division operation on vnp_Amount before verifying the signature. Additionally, the 'Buy Me A Coffee' button in the README files has been updated to use a new image source and styling. An English comment was also added for the vnp_Amount field in the type definition. File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @lehuygiang28 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟡 Documentation: 2 issues found
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Files selected for processing (4)
- README.md (1 hunks)
- README_en-US.md (1 hunks)
- src/types/return-from-vnpay.type.ts (1 hunks)
- src/vnpay.ts (1 hunks)
Additional context used
Path-based instructions (4)
README_en-US.md (1)
Pattern
**/*.md: Review the Markdown files to ensure they are well-structured follow Docusaurus's best practices, easy to read, and contain accurate and up-to-date information. Check for proper formatting, grammar, and spelling. Ensure that the content is relevant and engaging for the target audience. Highlight any inconsistencies or errors and suggest improvements to enhance the overall quality of the documentation.README.md (1)
Pattern
**/*.md: Review the Markdown files to ensure they are well-structured follow Docusaurus's best practices, easy to read, and contain accurate and up-to-date information. Check for proper formatting, grammar, and spelling. Ensure that the content is relevant and engaging for the target audience. Highlight any inconsistencies or errors and suggest improvements to enhance the overall quality of the documentation.src/types/return-from-vnpay.type.ts (1)
Pattern
**/*.ts: Review the TypeScript code of the NPM package to ensure it adheres to the principles of awesome and clean code. This includes proper use of functions, classes, modules. Also, ensure the code follows best practices for package development, such as clear and concise documentation, proper error handling, and efficient data structures and algorithms. The code should also conform to the ESLint rules configured in the codebase. Highlight any deviations and suggest appropriate corrections. Using design patterns that promote code reusability and readability is encouraged.src/vnpay.ts (1)
Pattern
**/*.ts: Review the TypeScript code of the NPM package to ensure it adheres to the principles of awesome and clean code. This includes proper use of functions, classes, modules. Also, ensure the code follows best practices for package development, such as clear and concise documentation, proper error handling, and efficient data structures and algorithms. The code should also conform to the ESLint rules configured in the codebase. Highlight any deviations and suggest appropriate corrections. Using design patterns that promote code reusability and readability is encouraged.
Markdownlint
README_en-US.md
80-80: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
80-80: Punctuation: '.'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
82-82: Element: a
Inline HTML(MD033, no-inline-html)
83-83: Element: img
Inline HTML(MD033, no-inline-html)
83-83: null
Images should have alternate text (alt text)(MD045, no-alt-text)
README.md
79-79: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
79-79: Punctuation: '.'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
81-81: Element: a
Inline HTML(MD033, no-inline-html)
82-82: Element: img
Inline HTML(MD033, no-inline-html)
82-82: null
Images should have alternate text (alt text)(MD045, no-alt-text)
LanguageTool
README.md
[misspelling] ~79-~79: Possible spelling mistakes found.
Context: ...Hỗ trợThư viện VNPay là một dự án mã nguồn mở và miễn phí. Nếu bạn thấy nó h...
(EN_MULTITOKEN_SPELLING_TWO)
Additional comments not posted (2)
src/types/return-from-vnpay.type.ts (1)
7-7: LGTM!The addition of the English comment enhances code clarity and documentation.
src/vnpay.ts (1)
261-261: LGTM!The removal of the division operation on
vnp_Amountensures that the signature verification matches correctly, addressing the bug mentioned in the PR objectives.
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Files selected for processing (2)
- README.md (3 hunks)
- README_en-US.md (3 hunks)
Additional context used
Path-based instructions (2)
README_en-US.md (1)
Pattern
**/*.md: Review the Markdown files to ensure they are well-structured follow Docusaurus's best practices, easy to read, and contain accurate and up-to-date information. Check for proper formatting, grammar, and spelling. Ensure that the content is relevant and engaging for the target audience. Highlight any inconsistencies or errors and suggest improvements to enhance the overall quality of the documentation.README.md (1)
Pattern
**/*.md: Review the Markdown files to ensure they are well-structured follow Docusaurus's best practices, easy to read, and contain accurate and up-to-date information. Check for proper formatting, grammar, and spelling. Ensure that the content is relevant and engaging for the target audience. Highlight any inconsistencies or errors and suggest improvements to enhance the overall quality of the documentation.
Markdownlint
README_en-US.md
43-43: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
82-82: Element: a
Inline HTML(MD033, no-inline-html)
83-83: Element: img
Inline HTML(MD033, no-inline-html)
README.md
43-43: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
81-81: Element: a
Inline HTML(MD033, no-inline-html)
82-82: Element: img
Inline HTML(MD033, no-inline-html)
LanguageTool
README.md
[misspelling] ~79-~79: Possible spelling mistakes found.
Context: ... Hỗ trợThư viện VNPay là một dự án mã nguồn mở và miễn phí. Nếu bạn thấy nó h...
(EN_MULTITOKEN_SPELLING_TWO)
Fix verify failed due to amount issue
This pull request fixes the issue by removing the division operation before verifying the signature.
fixes #15
Type of Change
Please check the type of change your PR introduces:
Testing
Does this introduce a breaking change?
If yes, please describe the impact and migration path for existing applications:
Related Issues or References
#15
Summary by Sourcery
This pull request fixes an issue with the signature verification by removing an unnecessary division operation on the amount. It also updates the 'Buy Me A Coffee' button in the documentation and adds an English translation for a comment in the type definition file.
Summary by CodeRabbit
Documentation
README.mdandREADME_en-US.mdwith a more sophisticated button link.Bug Fixes
vnp_Amountproperty calculation in theVNPayclass by removing unnecessary division by 100.Chores
ReturnQueryFromVNPaytype.