Skip to content

Fix KeyError in verify_payment_link_signature on incomplete payload#333

Open
gottostartsomewhere wants to merge 1 commit into
razorpay:masterfrom
gottostartsomewhere:fix/payment-link-signature-missing-key
Open

Fix KeyError in verify_payment_link_signature on incomplete payload#333
gottostartsomewhere wants to merge 1 commit into
razorpay:masterfrom
gottostartsomewhere:fix/payment-link-signature-missing-key

Conversation

@gottostartsomewhere

Copy link
Copy Markdown

Problem

Utility.verify_payment_link_signature is written to return False for an incomplete payload (it has an explicit else: return False branch). However, its guard only checks three keys:

if 'razorpay_payment_id' in parameters.keys() and 'payment_link_reference_id' in parameters.keys() and 'payment_link_status' in parameters.keys():

…while the body also reads parameters['payment_link_id'] and parameters['razorpay_signature']. So a payload that is missing payment_link_id (or razorpay_signature) but has the three guarded keys raises KeyError instead of returning False.

Reproduce

u.verify_payment_link_signature({
    'razorpay_payment_id': 'pay_123',
    'payment_link_reference_id': 'ref_1',
    'payment_link_status': 'paid',
    'razorpay_signature': 'deadbeef',
    # no 'payment_link_id'
})
# KeyError: 'payment_link_id'

Fix

Guard all required keys so an incomplete payload returns False as intended, rather than raising. Behaviour for complete payloads is unchanged.

Tests

Added test_verify_payment_link_signature_missing_key, which asserts a payload missing payment_link_id returns False. It fails on master (KeyError) and passes with this change. Full tests/test_client_utility.py suite passes (7 tests).

verify_payment_link_signature guarded only razorpay_payment_id,
payment_link_reference_id and payment_link_status, but also reads
payment_link_id and razorpay_signature. A payload missing either of the
latter raised KeyError instead of the documented graceful False return.

Guard all required keys so incomplete payloads return False. Add a
regression test.
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.

1 participant