test: add regression tests for #27468 (invalid URI segfaults) #27477

pull theStack wants to merge 1 commits into bitcoin:master from theStack:202304-test-add_regression_for_27468 changing 1 files +3 −2

theStack commented at 4:45 PM on April 17, 2023: contributor

Prior to PR #27468 (commit 11422cc5720c8d73a87600de8fe8abb156db80dc) all call-sites of GetQueryParameter(...) in the REST module could trigger a crash. Add missing test cases for all possible code-paths as a regression test, as a foundation for possible follow-up fixes (which aim to resolve this issue in a more general and robust way).

test: add regression tests for #27468 (invalid URI segfaults)

Prior to PR #27468 (commit 11422cc5720c8d73a87600de8fe8abb156db80dc) all
call-sites of `GetQueryParameter(...)` in the REST module could trigger
a crash. Add missing test cases for all possible code-paths as a
regression test, as a foundation for possible follow-up fixes (which aim
to resolve this issue in a more general and robust way).

6a77d290da

DrahtBot commented at 4:45 PM on April 17, 2023: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK stickies-v, vasild

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label Tests on Apr 17, 2023
fanquake requested review from stickies-v on Apr 17, 2023
fanquake requested review from vasild on Apr 17, 2023
stickies-v approved
stickies-v commented at 5:12 PM on April 17, 2023: contributor

ACK 6a77d290da589bd5620585def5bfc019e242e189

The URI character validation/throwing logic is to be moved away from the query parameter logic (which would make it independent from these 3 endpoints), but this seems like a good addition for the follow-up to build on.
vasild approved
vasild commented at 7:40 AM on April 18, 2023: contributor

ACK 6a77d290da589bd5620585def5bfc019e242e189
fanquake referenced this in commit 467fa89438 on Apr 18, 2023
fanquake commented at 9:03 AM on April 18, 2023: member

This has been merged.
fanquake closed this on Apr 18, 2023
theStack deleted the branch on Apr 18, 2023
sidhujag referenced this in commit 21e773bf32 on Apr 18, 2023
bitcoin locked this on Apr 17, 2024

Type	Reviewers
ACK	stickies-v, vasild

Contributors

Labels

Linked (view graph)

#27468 bugfix: rest: avoid segfault for invalid URI

test: add regression tests for #27468 (invalid URI segfaults) #27477

Reviews