Fix nil pointer panic when moved block has invalid addresses#38235
Fix nil pointer panic when moved block has invalid addresses#38235MunemHashmi wants to merge 2 commits intohashicorp:mainfrom
Conversation
|
|
|
Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement Learn more about why HashiCorp requires a CLA and what the CLA includes Have you signed the CLA already but the status is still pending? Recheck it. |
When a moved block contains invalid addresses (e.g. quoted strings or unqualified names), the parsed MoveEndpoint fields are nil. Previously decodeMovedBlock returned the incomplete Moved struct, causing a nil pointer dereference downstream. Return nil early from decodeMovedBlock when diagnostics contain errors, preventing incomplete Moved structs from entering the module config. Add a defensive nil check in findMoveStatements as well. Fixes hashicorp#34041 Fixes hashicorp#34162
MunemHashmi
force-pushed
the
fix/moved-block-nil-pointer-panic-34041
branch
from
654fc34 to
9e53d61
Compare
|
Thanks for this submission! If you sign the Contributor License Agreement, I will raise this at the next triage meeting. Thanks! |
| if mc.From == nil || mc.To == nil { | ||
| // Invalid addresses should've been caught during original | ||
| // configuration decoding, in the configs package. | ||
| continue |
There was a problem hiding this comment.
Unclear if this is the correct thing to do, versus surface some sort of error. (Relaying a comment from the triage meeting.)
There was a problem hiding this comment.
Good point — I've removed this guard. The primary fix in decodeMovedBlock returns nil early when diagnostics contain errors, so nil From/To should never reach this code path. I've posted a detailed explanation of the approach on #34041 for discussion.
|
Hi @MunemHashmi, the feedback from triage is that it is unclear what should happen when one of these values is nil. Most likely some sort of error needs to be surfaced. Unfortunately this would take some research and investigation that we will not have time to do in the near future. You can leave this PR open or if you would rather leave a comment on the open issue with your proposed solution, so that it can be discussed before implementation (please see our contribution guidelines, https://github.com/hashicorp/terraform/blob/main/.github/CONTRIBUTING.md#proposing-a-change). Thanks! |
The primary fix in decodeMovedBlock prevents nil endpoints from reaching this code path, making the guard unnecessary.
2 participants
When a
movedblock contains invalid addresses (e.g. quoted strings like"module.foo"or unqualified names likebar), thefromandtofields in the parsedMoveEndpointare nil. Previously,decodeMovedBlockstill returned the incompleteMovedstruct, which caused a nil pointer dereference downstream when code accessed theFromorTofields.This change makes
decodeMovedBlockreturnnilearly when diagnostics contain errors, preventing incompleteMovedstructs from being added to the module configuration. A defensive nil check is also added infindMoveStatementsfor robustness.Fixes #34041
Fixes #34162
Target Release
1.15.x
Rollback Plan
Changes to Security Controls
No changes to security controls.
CHANGELOG entry